From 68aac69cbe95cc91c61334863494d2d45d69e6d1 Mon Sep 17 00:00:00 2001 From: Devon Hudson Date: Fri, 2 Oct 2026 22:34:57 +0000 Subject: [PATCH] Explain an invalid invite link when password registration is disabled With password registration disabled, opening an invite link whose token is unknown, revoked, expired or used redirected to the upstream provider or to `/login`, so the invitee never learned that the link was the problem. The page now stays, with the "invite invalid" alert above the provider buttons and the sign-in link. A valid token which isn't passwordless can't register there, so it still redirects. Posting a token which has become invalid re-renders the page with only the token's error, instead of validating the rest of the form, which would tell whether a username exists even with registration closed. The page submits a token only when it resolved, and the re-rendered form drops it, so that the next submission is an ordinary registration rather than the same error. A blank token is no token, on the link and on the form alike, and spaces around one are ignored. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CZdGaj9bJcn5QeeUw8YXoF --- crates/handlers/src/views/register/mod.rs | 266 +++++++++++++++++- .../handlers/src/views/register/password.rs | 18 +- .../components/RegisterForm/RegisterForm.tsx | 31 +- frontend/src/entrypoints/register.test.tsx | 123 ++++++++ frontend/src/entrypoints/register.tsx | 1 + 5 files changed, 402 insertions(+), 37 deletions(-) create mode 100644 frontend/src/entrypoints/register.test.tsx diff --git a/crates/handlers/src/views/register/mod.rs b/crates/handlers/src/views/register/mod.rs index 14827e1a8..21bcf011f 100644 --- a/crates/handlers/src/views/register/mod.rs +++ b/crates/handlers/src/views/register/mod.rs @@ -26,8 +26,8 @@ use mas_storage::{ user::UserRegistrationTokenRepository, }; use mas_templates::{ - FormState, InviteContext, RegisterContext, RegisterFormField, TemplateContext, Templates, - ToFormState, + FieldError, FormState, InviteContext, RegisterContext, RegisterFormField, TemplateContext, + Templates, ToFormState, }; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -89,8 +89,13 @@ pub(crate) async fn get( } // Resolve the invite code the link carried, so that the form can show what - // it was issued for - let registration_token = match &query.token { + // it was issued for. A blank code is no code. + let token = query + .token + .as_deref() + .map(str::trim) + .filter(|token| !token.is_empty()); + let registration_token = match token { Some(token) => repo .user_registration_token() .find_by_token(token) @@ -101,13 +106,15 @@ pub(crate) async fn get( let passwordless_invite = registration_token .as_ref() .is_some_and(|token| token.passwordless); + let token_invalid = token.is_some() && registration_token.is_none(); let providers = repo.upstream_oauth_provider().all_enabled().await?; // Without a password form there is nothing to show beyond the provider // buttons, which isn't worth a page for one provider or none. A - // passwordless invite is a way to register on its own, so it keeps the page - if !site_config.password_registration_enabled && !passwordless_invite { + // passwordless invite is a way to register on its own, so it keeps the + // page. The page also reports an invalid invite. + if !site_config.password_registration_enabled && !passwordless_invite && !token_invalid { if providers.len() == 1 { let provider = providers.into_iter().next().unwrap(); @@ -132,11 +139,12 @@ pub(crate) async fn get( query.action.post_auth_action.as_ref(), ); - // The code travels back on submission through a hidden field, whether it - // resolved or not - if let Some(token) = query.token { + // A code which resolved travels back on submission through a hidden field + if let Some(token) = token { let mut form = FormState::default(); - form.set_value(RegisterFormField::Token, Some(token)); + if registration_token.is_some() { + form.set_value(RegisterFormField::Token, Some(token.to_owned())); + } ctx = ctx .with_form_state(form) .with_invite(InviteContext::new(registration_token.as_ref())); @@ -279,15 +287,43 @@ pub(crate) async fn post( // Resolve the invite code now: it decides whether a password is needed at // all, and a passwordless one is a way to register on its own - let registration_token = if form.token.is_empty() { + let token = form.token.trim(); + let registration_token = if token.is_empty() { None } else { repo.user_registration_token() - .find_by_token(&form.token) + .find_by_token(token) .await? .filter(|token| token.is_valid(clock.now())) }; - let token_invalid = !form.token.is_empty() && registration_token.is_none(); + + // Only the code's error is reported: checking the rest of the form would + // tell whether a username exists, even with registration closed. The code + // isn't submitted again, so the next submission is an ordinary + // registration. + if !token.is_empty() && registration_token.is_none() { + let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); + let mut form_state = form + .to_form_state() + .with_error_on_field(RegisterFormField::Token, FieldError::Invalid); + form_state.set_value(RegisterFormField::Token, None); + + let content = self::password::render( + locale, + form_state, + &query.action, + csrf_token, + &mut repo, + &templates, + &url_builder, + site_config.captcha.clone(), + Some(InviteContext::new(None)), + ) + .await?; + + return Ok((cookie_jar, Html(content)).into_response()); + } + let passwordless_invite = registration_token .as_ref() .is_some_and(|token| token.passwordless); @@ -316,7 +352,6 @@ pub(crate) async fn post( cookie_jar, form, registration_token, - token_invalid, ) .await } @@ -713,7 +748,7 @@ mod tests { serde_json::json!("invite_alice") ); - // A code the server doesn't know only says so + // A code the server doesn't know only says so, and isn't submitted let request = Request::get("/register?token=nope").empty(); let response = state.request(request).await; response.assert_status(StatusCode::OK); @@ -721,6 +756,10 @@ mod tests { json_attribute(response.body(), "data-invite"), serde_json::json!({ "valid": false }) ); + assert_eq!( + json_attribute(response.body(), "data-form")["fields"]["token"]["value"], + serde_json::Value::Null + ); } /// A passwordless invite is a way to register on its own, so the page keeps @@ -767,6 +806,203 @@ mod tests { response.assert_header_value(LOCATION, "/login"); } + /// With password registration disabled, an invalid invite link keeps the + /// page to say so, whatever the providers. A valid code which isn't + /// passwordless can't register there, so it doesn't. + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_get_with_invalid_invite_without_password_registration(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + password_login_enabled: false, + password_registration_enabled: false, + ..test_site_config() + }, + ) + .await + .unwrap(); + let revoked = add_registration_token(&state, "revoked", None, None, true).await; + let expired = add_registration_token(&state, "expired", None, None, true).await; + let used = add_registration_token(&state, "used", None, None, true).await; + add_registration_token(&state, "not_passwordless", None, None, false).await; + + let mut repo = state.repository().await.unwrap(); + repo.user_registration_token() + .revoke(&state.clock, revoked) + .await + .unwrap(); + repo.user_registration_token() + .set_expiry(expired, Some(state.clock.now())) + .await + .unwrap(); + let used = repo + .user_registration_token() + .set_usage_limit(used, Some(1)) + .await + .unwrap(); + repo.user_registration_token() + .use_token(&state.clock, used) + .await + .unwrap(); + repo.save().await.unwrap(); + + for providers in 0..=2 { + if providers > 0 { + provider(&state).await; + } + + for token in ["unknown", "revoked", "expired", "used"] { + let request = Request::get(format!("/register?token={token}")).empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + assert_eq!( + json_attribute(response.body(), "data-invite"), + serde_json::json!({ "valid": false }) + ); + } + + let request = Request::get("/register?token=not_passwordless").empty(); + let response = state.request(request).await; + match providers { + 0 => { + response.assert_status(StatusCode::SEE_OTHER); + response.assert_header_value(LOCATION, "/login"); + } + 1 => response.assert_status(StatusCode::SEE_OTHER), + _ => response.assert_status(StatusCode::OK), + } + } + } + + /// A blank invite code is no code, and spaces around one are ignored + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_blank_and_padded_tokens(pool: PgPool) { + setup(); + let state = TestState::from_pool_with_site_config( + pool, + SiteConfig { + password_login_enabled: false, + password_registration_enabled: false, + ..test_site_config() + }, + ) + .await + .unwrap(); + let cookies = CookieHelper::new(); + + add_registration_token( + &state, + "invite_alice", + Some("alice"), + Some("alice@example.com"), + true, + ) + .await; + + for token in ["", "%20%20"] { + let request = Request::get(format!("/register?token={token}")).empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::SEE_OTHER); + response.assert_header_value(LOCATION, "/login"); + } + + let request = Request::get("/register?token=%20invite_alice%20").empty(); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + assert_eq!( + json_attribute(response.body(), "data-invite")["valid"], + serde_json::json!(true) + ); + assert_eq!( + json_attribute(response.body(), "data-form")["fields"]["token"]["value"], + serde_json::json!("invite_alice") + ); + + let csrf_token = mint_csrf_token(&state, &cookies); + for (token, status) in [ + (" ", StatusCode::METHOD_NOT_ALLOWED), + (" invite_alice ", StatusCode::SEE_OTHER), + ] { + let request = + cookies.with_cookies(Request::post("/register").form(serde_json::json!({ + "csrf": csrf_token, + "token": token, + "accept_terms": "on", + }))); + let response = state.request(request).await; + response.assert_status(status); + } + } + + /// Posting an invite code which has become invalid reports only the code, + /// so the form can't tell whether a username exists, even with password + /// registration disabled + #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] + async fn test_post_with_used_invite(pool: PgPool) { + setup(); + let state = TestState::from_pool(pool.clone()).await.unwrap(); + + let used = add_registration_token(&state, "used", None, None, true).await; + let mut repo = state.repository().await.unwrap(); + repo.user() + .add(&mut state.rng(), &state.clock, "alice".to_owned()) + .await + .unwrap(); + let used = repo + .user_registration_token() + .set_usage_limit(used, Some(1)) + .await + .unwrap(); + repo.user_registration_token() + .use_token(&state.clock, used) + .await + .unwrap(); + repo.save().await.unwrap(); + + for password_registration_enabled in [true, false] { + let state = TestState::from_pool_with_site_config( + pool.clone(), + SiteConfig { + password_registration_enabled, + ..test_site_config() + }, + ) + .await + .unwrap(); + let cookies = CookieHelper::new(); + + let csrf_token = mint_csrf_token(&state, &cookies); + let request = + cookies.with_cookies(Request::post("/register").form(serde_json::json!({ + "csrf": csrf_token, + "username": "alice", + "email": "alice@example.com", + "password": "hunter2", + "password_confirm": "hunter2", + "token": "used", + }))); + let response = state.request(request).await; + response.assert_status(StatusCode::OK); + assert_eq!( + json_attribute(response.body(), "data-invite"), + serde_json::json!({ "valid": false }) + ); + + let form = json_attribute(response.body(), "data-form"); + assert_eq!(form["errors"], serde_json::json!([])); + for (field, state) in form["fields"].as_object().unwrap() { + let errors = if field == "token" { + serde_json::json!([{"kind": "invalid"}]) + } else { + serde_json::json!([]) + }; + assert_eq!(state["errors"], errors, "errors on {field}"); + } + assert_eq!(form["fields"]["token"]["value"], serde_json::Value::Null); + } + } + /// Without password registration, the page is just the provider buttons #[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")] async fn test_get_password_disabled_with_providers(pool: PgPool) { diff --git a/crates/handlers/src/views/register/password.rs b/crates/handlers/src/views/register/password.rs index 1320001d7..458911acf 100644 --- a/crates/handlers/src/views/register/password.rs +++ b/crates/handlers/src/views/register/password.rs @@ -62,7 +62,6 @@ pub(super) async fn register( cookie_jar: CookieJar, form: RegisterForm, registration_token: Option, - token_invalid: bool, ) -> Result { let ip_address = activity_tracker.ip(); @@ -122,10 +121,6 @@ pub(super) async fn register( state.add_error_on_form(FormError::Captcha); } - if token_invalid { - state.add_error_on_field(RegisterFormField::Token, FieldError::Invalid); - } - // An invite code may only be used to register the identity it was // issued for if pinned_username.is_some_and(|pinned| pinned != username) { @@ -169,10 +164,8 @@ pub(super) async fn register( } } - // A passwordless invite code waives the password entirely. If we - // couldn't resolve the code we don't know whether one is needed, so - // don't pile up password errors on top of the code error either - if !passwordless && !token_invalid { + // A passwordless invite code waives the password entirely + if !passwordless { if form.password.is_empty() { state.add_error_on_field(RegisterFormField::Password, FieldError::Required); } @@ -279,8 +272,9 @@ pub(super) async fn register( if !state.is_valid() { // Re-render with what the invite code resolved to, so the form keeps // the shape the user submitted it in - let invite = - (!form.token.is_empty()).then(|| InviteContext::new(registration_token.as_ref())); + let invite = registration_token + .as_ref() + .map(|token| InviteContext::new(Some(token))); let content = render( locale, @@ -384,7 +378,7 @@ pub(super) async fn register( /// Render the registration page again, with the errors the form collected #[expect(clippy::too_many_arguments)] -async fn render( +pub(super) async fn render( locale: DataLocale, form_state: FormState, action: &OptionalPostAuthAction, diff --git a/frontend/src/components/RegisterForm/RegisterForm.tsx b/frontend/src/components/RegisterForm/RegisterForm.tsx index e922adf13..321991532 100644 --- a/frontend/src/components/RegisterForm/RegisterForm.tsx +++ b/frontend/src/components/RegisterForm/RegisterForm.tsx @@ -58,6 +58,12 @@ const ProviderButtons: React.FC<{ providers: Provider[] }> = ({ ); }; +const ErrorAlert: React.FC<{ children: React.ReactNode }> = ({ children }) => ( +
+ {children} +
+); + const LoginLink: React.FC<{ href: string }> = ({ href }) => { const { t } = useTranslation(); return ( @@ -202,9 +208,7 @@ export const PasswordRegisterForm: React.FC<{ )} {inviteInvalid && ( -
- {t("frontend.register.invite_invalid")} -
+ {t("frontend.register.invite_invalid")} )} {formErrors.map((error, index) => ( @@ -360,10 +364,17 @@ export const ProvidersOnlyForm: React.FC<{ csrfToken: string; providers: Provider[]; loginLink: string; -}> = ({ csrfToken, providers, loginLink }) => ( - - - - - -); + inviteInvalid: boolean; +}> = ({ csrfToken, providers, loginLink, inviteInvalid }) => { + const { t } = useTranslation(); + return ( + + + {inviteInvalid && ( + {t("frontend.register.invite_invalid")} + )} + + + + ); +}; diff --git a/frontend/src/entrypoints/register.test.tsx b/frontend/src/entrypoints/register.test.tsx new file mode 100644 index 000000000..e07915568 --- /dev/null +++ b/frontend/src/entrypoints/register.test.tsx @@ -0,0 +1,123 @@ +// Copyright 2026 Element Creations Ltd. +// +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +// Please see LICENSE files in the repository root for full details. + +// @vitest-environment happy-dom + +import { act, screen } from "@testing-library/react"; +import type { Root } from "react-dom/client"; +import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; + +/** The roots the entry point mounted, to unmount after each test */ +const roots = vi.hoisted((): Root[] => []); + +vi.mock("react-dom/client", async (importOriginal) => { + const client = await importOriginal(); + return { + ...client, + createRoot: (...args: Parameters): Root => { + const root = client.createRoot(...args); + roots.push(root); + return root; + }, + }; +}); + +// The scorer loads its dictionaries lazily, which can outlive the test file +vi.mock("../utils/password_complexity", () => ({ + estimatePasswordComplexity: async () => ({ + score: 0, + scoreText: "", + improvementsText: [], + }), +})); + +const PROVIDERS = [ + { name: "First", brand: null, id: "01J00000000000000000000001" }, + { name: "Second", brand: null, id: "01J00000000000000000000002" }, +]; + +/** Mount the island on a node carrying the given data, as the page does */ +const mountRegister = async ({ + passwordRegistration = true, + providers = [], + invite, + form = { errors: [], fields: {} }, +}: { + passwordRegistration?: boolean; + providers?: typeof PROVIDERS; + invite?: Record; + form?: Record; +} = {}): Promise => { + const el = document.createElement("div"); + el.id = "register-form"; + el.dataset.csrfToken = "csrf"; + el.dataset.graphqlEndpoint = "/graphql"; + el.dataset.branding = JSON.stringify({ server_name: "example.com" }); + el.dataset.features = JSON.stringify({ + password_registration: passwordRegistration, + password_registration_email_required: true, + minimum_password_complexity: 1, + }); + el.dataset.form = JSON.stringify(form); + el.dataset.providers = JSON.stringify(providers); + el.dataset.loginLink = "/login"; + if (invite) el.dataset.invite = JSON.stringify(invite); + document.body.append(el); + + // The entry point mounts the island when it is imported + vi.resetModules(); + await import("./register"); + await screen.findByRole("link", { name: "Already have an account?" }); +}; + +const unmount = (): void => { + act(() => { + for (const root of roots.splice(0)) root.unmount(); + }); + document.body.innerHTML = ""; +}; + +describe("register island", () => { + // The first import of the entry point is slow, so it happens outside the + // tests' timeout + beforeAll(async () => { + await mountRegister(); + unmount(); + }); + + afterEach(unmount); + + describe("with an invalid invite", () => { + it.each([ + 0, 1, 2, + ])("says so without password registration, with %i providers", async (count) => { + await mountRegister({ + passwordRegistration: false, + providers: PROVIDERS.slice(0, count), + invite: { valid: false }, + }); + + expect(screen.getByRole("alert")).toHaveTextContent( + "This invite link is invalid or has expired", + ); + expect( + screen.queryAllByRole("button", { name: /^Continue with/ }), + ).toHaveLength(count); + }); + + it("says so above the password form", async () => { + await mountRegister({ invite: { valid: false } }); + + const alert = screen.getByRole("alert"); + expect(alert).toHaveTextContent( + "This invite link is invalid or has expired", + ); + expect( + alert.compareDocumentPosition(screen.getByLabelText("Username")) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); + }); +}); diff --git a/frontend/src/entrypoints/register.tsx b/frontend/src/entrypoints/register.tsx index d39e84f6f..1ef7e0928 100644 --- a/frontend/src/entrypoints/register.tsx +++ b/frontend/src/entrypoints/register.tsx @@ -106,6 +106,7 @@ const RegisterPage: React.FC<{ data: Data }> = ({ data }) => csrfToken={data.csrfToken} providers={data.providers} loginLink={data.loginLink} + inviteInvalid={data.invite?.valid === false} /> );