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} /> );