Move the finishing of registration to a dedicated view

crates/handlers/src/lib.rs

@@ -383,6 +383,10 @@ where
             get(self::views::register::steps::verify_email::get)
                 .post(self::views::register::steps::verify_email::post),
         )
+        .route(
+            mas_router::RegisterFinish::route(),
+            get(self::views::register::steps::finish::get),
+        )
         .route(
             mas_router::AccountRecoveryStart::route(),
             get(self::views::recovery::start::get).post(self::views::recovery::start::post),

crates/handlers/src/views/register/password.rs

@@ -362,7 +362,7 @@ pub(crate) async fn post(
     repo.save().await?;
 
     Ok(url_builder
-        .redirect(&mas_router::RegisterVerifyEmail::new(registration.id))
+        .redirect(&mas_router::RegisterFinish::new(registration.id))
         .into_response())
 }
 
@@ -479,12 +479,12 @@ mod tests {
         response.assert_status(StatusCode::SEE_OTHER);
         let location = response.headers().get(LOCATION).unwrap();
 
-        // The handler redirects with the ID as the last portion of the path
+        // The handler redirects with the ID as the second to last portion of the path
         let id = location
             .to_str()
             .unwrap()
             .rsplit('/')
-            .next()
+            .nth(1)
             .unwrap()
             .parse()
             .unwrap();

crates/handlers/src/views/register/steps/finish.rs

@@ -0,0 +1,174 @@
+// Copyright 2025 New Vector Ltd.
+//
+// SPDX-License-Identifier: AGPL-3.0-only
+// Please see LICENSE in the repository root for full details.
+
+use anyhow::Context as _;
+use axum::{
+    extract::{Path, State},
+    response::IntoResponse,
+};
+use axum_extra::TypedHeader;
+use mas_axum_utils::{cookies::CookieJar, FancyError, SessionInfoExt as _};
+use mas_data_model::UserAgent;
+use mas_router::{PostAuthAction, UrlBuilder};
+use mas_storage::{
+    queue::{ProvisionUserJob, QueueJobRepositoryExt as _},
+    user::UserEmailFilter,
+    BoxClock, BoxRepository, BoxRng,
+};
+use ulid::Ulid;
+
+use crate::{views::shared::OptionalPostAuthAction, BoundActivityTracker};
+
+#[tracing::instrument(
+    name = "handlers.views.register.steps.finish.get",
+    fields(user_registration.id = %id),
+    skip_all,
+    err,
+)]
+pub(crate) async fn get(
+    mut rng: BoxRng,
+    clock: BoxClock,
+    mut repo: BoxRepository,
+    activity_tracker: BoundActivityTracker,
+    user_agent: Option<TypedHeader<headers::UserAgent>>,
+    State(url_builder): State<UrlBuilder>,
+    cookie_jar: CookieJar,
+    Path(id): Path<Ulid>,
+) -> Result<impl IntoResponse, FancyError> {
+    let user_agent = user_agent.map(|ua| UserAgent::parse(ua.as_str().to_owned()));
+    let registration = repo
+        .user_registration()
+        .lookup(id)
+        .await?
+        .context("User registration not found")?;
+
+    // If the registration is completed, we can go to the registration destination
+    // XXX: this might not be the right thing to do? Maybe an error page would be
+    // better?
+    if registration.completed_at.is_some() {
+        let post_auth_action: Option<PostAuthAction> = registration
+            .post_auth_action
+            .map(serde_json::from_value)
+            .transpose()?;
+
+        return Ok((
+            cookie_jar,
+            OptionalPostAuthAction::from(post_auth_action).go_next(&url_builder),
+        ));
+    }
+
+    // Let's perform last minute checks on the registration, especially to avoid
+    // race conditions where multiple users register with the same username or email
+    // address
+
+    if repo.user().exists(&registration.username).await? {
+        return Err(FancyError::from(anyhow::anyhow!(
+            "Username is already taken"
+        )));
+    }
+
+    // TODO: query the homeserver
+
+    // For now, we require an email address on the registration, but this might
+    // change in the future
+    let email_authentication_id = registration
+        .email_authentication_id
+        .context("No email authentication started for this registration")?;
+    let email_authentication = repo
+        .user_email()
+        .lookup_authentication(email_authentication_id)
+        .await?
+        .context("Could not load the email authentication")?;
+
+    // Check that the email authentication has been completed
+    if email_authentication.completed_at.is_none() {
+        return Ok((
+            cookie_jar,
+            url_builder.redirect(&mas_router::RegisterVerifyEmail::new(id)),
+        ));
+    }
+
+    // Check that the email address isn't already used
+    if repo
+        .user_email()
+        .count(UserEmailFilter::new().for_email(&email_authentication.email))
+        .await?
+        > 0
+    {
+        return Err(FancyError::from(anyhow::anyhow!(
+            "Email address is already used"
+        )));
+    }
+
+    // Everuthing is good, let's complete the registration
+    let registration = repo
+        .user_registration()
+        .complete(&clock, registration)
+        .await?;
+
+    // Now we can start the user creation
+    let user = repo
+        .user()
+        .add(&mut rng, &clock, registration.username)
+        .await?;
+    // Also create a browser session which will log the user in
+    let user_session = repo
+        .browser_session()
+        .add(&mut rng, &clock, &user, user_agent)
+        .await?;
+
+    repo.user_email()
+        .add(&mut rng, &clock, &user, email_authentication.email)
+        .await?;
+
+    if let Some(password) = registration.password {
+        let user_password = repo
+            .user_password()
+            .add(
+                &mut rng,
+                &clock,
+                &user,
+                password.version,
+                password.hashed_password,
+                None,
+            )
+            .await?;
+
+        repo.browser_session()
+            .authenticate_with_password(&mut rng, &clock, &user_session, &user_password)
+            .await?;
+    }
+
+    if let Some(terms_url) = registration.terms_url {
+        repo.user_terms()
+            .accept_terms(&mut rng, &clock, &user, terms_url)
+            .await?;
+    }
+
+    let mut job = ProvisionUserJob::new(&user);
+    if let Some(display_name) = registration.display_name {
+        job = job.set_display_name(display_name);
+    }
+    repo.queue_job().schedule_job(&mut rng, &clock, job).await?;
+
+    repo.save().await?;
+
+    activity_tracker
+        .record_browser_session(&clock, &user_session)
+        .await;
+
+    let post_auth_action: Option<PostAuthAction> = registration
+        .post_auth_action
+        .map(serde_json::from_value)
+        .transpose()?;
+
+    // Login the user with the session we just created
+    let cookie_jar = cookie_jar.set_session(&user_session);
+
+    return Ok((
+        cookie_jar,
+        OptionalPostAuthAction::from(post_auth_action).go_next(&url_builder),
+    ));
+}

crates/handlers/src/views/register/steps/mod.rs

@@ -3,4 +3,5 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 // Please see LICENSE in the repository root for full details.
 
+pub(crate) mod finish;
 pub(crate) mod verify_email;

crates/handlers/src/views/register/steps/verify_email.rs

@@ -8,19 +8,13 @@ use axum::{
     extract::{Form, Path, State},
     response::{Html, IntoResponse, Response},
 };
-use axum_extra::TypedHeader;
 use mas_axum_utils::{
     cookies::CookieJar,
     csrf::{CsrfExt, ProtectedForm},
-    FancyError, SessionInfoExt,
+    FancyError,
 };
-use mas_data_model::UserAgent;
 use mas_router::{PostAuthAction, UrlBuilder};
-use mas_storage::{
-    queue::{ProvisionUserJob, QueueJobRepositoryExt as _},
-    user::UserEmailRepository,
-    BoxClock, BoxRepository, BoxRng, RepositoryAccess,
-};
+use mas_storage::{user::UserEmailRepository, BoxClock, BoxRepository, BoxRng, RepositoryAccess};
 use mas_templates::{
     FieldError, RegisterStepsVerifyEmailContext, RegisterStepsVerifyEmailFormField,
     TemplateContext, Templates, ToFormState,
@@ -28,7 +22,7 @@ use mas_templates::{
 use serde::{Deserialize, Serialize};
 use ulid::Ulid;
 
-use crate::{views::shared::OptionalPostAuthAction, BoundActivityTracker, PreferredLanguage};
+use crate::{views::shared::OptionalPostAuthAction, PreferredLanguage};
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CodeForm {
@@ -72,8 +66,12 @@ pub(crate) async fn get(
             .map(serde_json::from_value)
             .transpose()?;
 
-        return Ok(OptionalPostAuthAction::from(post_auth_action)
-            .go_next(&url_builder)
+        return Ok((
+            cookie_jar,
+            OptionalPostAuthAction::from(post_auth_action)
+                .go_next(&url_builder)
+                .into_response(),
+        )
             .into_response());
     }
 
@@ -115,13 +113,10 @@ pub(crate) async fn post(
     State(templates): State<Templates>,
     mut repo: BoxRepository,
     cookie_jar: CookieJar,
-    user_agent: Option<TypedHeader<headers::UserAgent>>,
     State(url_builder): State<UrlBuilder>,
-    activity_tracker: BoundActivityTracker,
     Path(id): Path<Ulid>,
     Form(form): Form<ProtectedForm<CodeForm>>,
 ) -> Result<Response, FancyError> {
-    let user_agent = user_agent.map(|ua| UserAgent::parse(ua.as_str().to_owned()));
     let form = cookie_jar.verify_form(&clock, form)?;
 
     let registration = repo
@@ -139,8 +134,10 @@ pub(crate) async fn post(
             .map(serde_json::from_value)
             .transpose()?;
 
-        return Ok(OptionalPostAuthAction::from(post_auth_action)
-            .go_next(&url_builder)
+        return Ok((
+            cookie_jar,
+            OptionalPostAuthAction::from(post_auth_action).go_next(&url_builder),
+        )
             .into_response());
     }
 
@@ -180,74 +177,12 @@ pub(crate) async fn post(
         return Ok((cookie_jar, Html(content)).into_response());
     };
 
-    let email_authentication = repo
-        .user_email()
-        .complete_authentication(&clock, email_authentication, &code)
-        .await?;
-
-    let registration = repo
-        .user_registration()
-        .complete(&clock, registration)
-        .await?;
-
-    // XXX: this should move somewhere else, and it doesn't check for uniqueness
-    let user = repo
-        .user()
-        .add(&mut rng, &clock, registration.username)
-        .await?;
-    let user_session = repo
-        .browser_session()
-        .add(&mut rng, &clock, &user, user_agent)
-        .await?;
-
     repo.user_email()
-        .add(&mut rng, &clock, &user, email_authentication.email)
-        .await?;
-
-    if let Some(password) = registration.password {
-        let user_password = repo
-            .user_password()
-            .add(
-                &mut rng,
-                &clock,
-                &user,
-                password.version,
-                password.hashed_password,
-                None,
-            )
-            .await?;
-
-        repo.browser_session()
-            .authenticate_with_password(&mut rng, &clock, &user_session, &user_password)
-            .await?;
-    }
-
-    if let Some(terms_url) = registration.terms_url {
-        repo.user_terms()
-            .accept_terms(&mut rng, &clock, &user, terms_url)
-            .await?;
-    }
-
-    repo.queue_job()
-        .schedule_job(&mut rng, &clock, ProvisionUserJob::new(&user))
+        .complete_authentication(&clock, email_authentication, &code)
         .await?;
 
     repo.save().await?;
 
-    activity_tracker
-        .record_browser_session(&clock, &user_session)
-        .await;
-
-    let post_auth_action: Option<PostAuthAction> = registration
-        .post_auth_action
-        .map(serde_json::from_value)
-        .transpose()?;
-
-    let cookie_jar = cookie_jar.set_session(&user_session);
-
-    return Ok((
-        cookie_jar,
-        OptionalPostAuthAction::from(post_auth_action).go_next(&url_builder),
-    )
-        .into_response());
+    let destination = mas_router::RegisterFinish::new(registration.id);
+    return Ok((cookie_jar, url_builder.redirect(&destination)).into_response());
 }

crates/router/src/endpoints.rs

@@ -444,7 +444,7 @@ impl From<Option<PostAuthAction>> for PasswordRegister {
     }
 }
 
-/// `GET|POST /register/steps/verify-email/:id`
+/// `GET|POST /register/steps/:id/verify-email`
 #[derive(Debug, Clone)]
 pub struct RegisterVerifyEmail {
     id: Ulid,
@@ -460,11 +460,35 @@ impl RegisterVerifyEmail {
 impl Route for RegisterVerifyEmail {
     type Query = ();
     fn route() -> &'static str {
-        "/register/steps/verify-email/:id"
+        "/register/steps/:id/verify-email"
     }
 
     fn path(&self) -> std::borrow::Cow<'static, str> {
-        format!("/register/steps/verify-email/{}", self.id).into()
+        format!("/register/steps/{}/verify-email", self.id).into()
+    }
+}
+
+/// `GET /register/steps/:id/finish`
+#[derive(Debug, Clone)]
+pub struct RegisterFinish {
+    id: Ulid,
+}
+
+impl RegisterFinish {
+    #[must_use]
+    pub const fn new(id: Ulid) -> Self {
+        Self { id }
+    }
+}
+
+impl Route for RegisterFinish {
+    type Query = ();
+    fn route() -> &'static str {
+        "/register/steps/:id/finish"
+    }
+
+    fn path(&self) -> std::borrow::Cow<'static, str> {
+        format!("/register/steps/{}/finish", self.id).into()
     }
 }