Remove the old password change page (#2874)

crates/handlers/src/lib.rs

@@ -354,7 +354,7 @@ where
         .route(
             mas_router::ChangePasswordDiscovery::route(),
             get(|State(url_builder): State<UrlBuilder>| async move {
-                url_builder.redirect(&mas_router::AccountPassword)
+                url_builder.redirect(&mas_router::AccountPasswordChange)
             }),
         )
         .route(mas_router::Index::route(), get(self::views::index::get))
@@ -371,10 +371,6 @@ where
             mas_router::Register::route(),
             get(self::views::register::get).post(self::views::register::post),
         )
-        .route(
-            mas_router::AccountPassword::route(),
-            get(self::views::account::password::get).post(self::views::account::password::post),
-        )
         .route(
             mas_router::AccountVerifyEmail::route(),
             get(self::views::account::emails::verify::get)

crates/handlers/src/views/account/mod.rs

@@ -13,4 +13,3 @@
 // limitations under the License.
 
 pub mod emails;
-pub mod password;

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

@@ -1,198 +0,0 @@
-// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-use anyhow::Context;
-use axum::{
-    extract::{Form, State},
-    http::StatusCode,
-    response::{Html, IntoResponse, Response},
-};
-use mas_axum_utils::{
-    cookies::CookieJar,
-    csrf::{CsrfExt, ProtectedForm},
-    FancyError, SessionInfoExt,
-};
-use mas_data_model::{BrowserSession, SiteConfig};
-use mas_i18n::DataLocale;
-use mas_policy::Policy;
-use mas_router::UrlBuilder;
-use mas_storage::{
-    user::{BrowserSessionRepository, UserPasswordRepository},
-    BoxClock, BoxRepository, BoxRng, Clock,
-};
-use mas_templates::{EmptyContext, TemplateContext, Templates};
-use rand::Rng;
-use serde::Deserialize;
-use zeroize::Zeroizing;
-
-use crate::{passwords::PasswordManager, BoundActivityTracker, PreferredLanguage};
-
-#[derive(Deserialize)]
-pub struct ChangeForm {
-    current_password: String,
-    new_password: String,
-    new_password_confirm: String,
-}
-
-#[tracing::instrument(name = "handlers.views.account_password.get", skip_all, err)]
-pub(crate) async fn get(
-    mut rng: BoxRng,
-    clock: BoxClock,
-    PreferredLanguage(locale): PreferredLanguage,
-    State(templates): State<Templates>,
-    State(site_config): State<SiteConfig>,
-    activity_tracker: BoundActivityTracker,
-    State(url_builder): State<UrlBuilder>,
-    mut repo: BoxRepository,
-    cookie_jar: CookieJar,
-) -> Result<Response, FancyError> {
-    // If the password manager is disabled, we can go back to the account page.
-    if !site_config.password_change_allowed {
-        return Ok(url_builder
-            .redirect(&mas_router::Account::default())
-            .into_response());
-    }
-
-    let (session_info, cookie_jar) = cookie_jar.session_info();
-
-    let maybe_session = session_info.load_session(&mut repo).await?;
-
-    if let Some(session) = maybe_session {
-        activity_tracker
-            .record_browser_session(&clock, &session)
-            .await;
-
-        render(&mut rng, &clock, locale, templates, session, cookie_jar).await
-    } else {
-        let login = mas_router::Login::and_then(mas_router::PostAuthAction::ChangePassword);
-        Ok((cookie_jar, url_builder.redirect(&login)).into_response())
-    }
-}
-
-async fn render(
-    rng: impl Rng + Send,
-    clock: &impl Clock,
-    locale: DataLocale,
-    templates: Templates,
-    session: BrowserSession,
-    cookie_jar: CookieJar,
-) -> Result<Response, FancyError> {
-    let (csrf_token, cookie_jar) = cookie_jar.csrf_token(clock, rng);
-
-    let ctx = EmptyContext
-        .with_session(session)
-        .with_csrf(csrf_token.form_value())
-        .with_language(locale);
-
-    let content = templates.render_account_password(&ctx)?;
-
-    Ok((cookie_jar, Html(content)).into_response())
-}
-
-#[tracing::instrument(name = "handlers.views.account_password.post", skip_all, err)]
-pub(crate) async fn post(
-    mut rng: BoxRng,
-    clock: BoxClock,
-    PreferredLanguage(locale): PreferredLanguage,
-    State(password_manager): State<PasswordManager>,
-    State(site_config): State<SiteConfig>,
-    State(templates): State<Templates>,
-    activity_tracker: BoundActivityTracker,
-    State(url_builder): State<UrlBuilder>,
-    mut policy: Policy,
-    mut repo: BoxRepository,
-    cookie_jar: CookieJar,
-    Form(form): Form<ProtectedForm<ChangeForm>>,
-) -> Result<Response, FancyError> {
-    if !site_config.password_change_allowed {
-        // XXX: do something better here
-        return Ok(StatusCode::METHOD_NOT_ALLOWED.into_response());
-    }
-
-    let form = cookie_jar.verify_form(&clock, form)?;
-
-    let (session_info, cookie_jar) = cookie_jar.session_info();
-
-    let maybe_session = session_info.load_session(&mut repo).await?;
-
-    let Some(session) = maybe_session else {
-        let login = mas_router::Login::and_then(mas_router::PostAuthAction::ChangePassword);
-        return Ok((cookie_jar, url_builder.redirect(&login)).into_response());
-    };
-
-    let user_password = repo
-        .user_password()
-        .active(&session.user)
-        .await?
-        .context("user has no password")?;
-
-    let res = policy.evaluate_password(&form.new_password).await?;
-
-    // TODO: display nice form errors
-    if !res.valid() {
-        return Err(anyhow::anyhow!("Password policy violation: {res}").into());
-    }
-
-    let password = Zeroizing::new(form.current_password.into_bytes());
-    let new_password = Zeroizing::new(form.new_password.into_bytes());
-    let new_password_confirm = Zeroizing::new(form.new_password_confirm.into_bytes());
-
-    password_manager
-        .verify(
-            user_password.version,
-            password,
-            user_password.hashed_password,
-        )
-        .await?;
-
-    // TODO: display nice form errors
-    if new_password != new_password_confirm {
-        return Err(anyhow::anyhow!("Password mismatch").into());
-    }
-
-    let (version, hashed_password) = password_manager.hash(&mut rng, new_password).await?;
-    let user_password = repo
-        .user_password()
-        .add(
-            &mut rng,
-            &clock,
-            &session.user,
-            version,
-            hashed_password,
-            None,
-        )
-        .await?;
-
-    repo.browser_session()
-        .authenticate_with_password(&mut rng, &clock, &session, &user_password)
-        .await?;
-
-    activity_tracker
-        .record_browser_session(&clock, &session)
-        .await;
-
-    let reply = render(
-        &mut rng,
-        &clock,
-        locale,
-        templates.clone(),
-        session,
-        cookie_jar,
-    )
-    .await?;
-
-    repo.save().await?;
-
-    Ok(reply)
-}

crates/router/src/endpoints.rs

@@ -77,7 +77,7 @@ impl PostAuthAction {
             Self::ContinueCompatSsoLogin { id } => {
                 url_builder.redirect(&CompatLoginSsoComplete::new(*id, None))
             }
-            Self::ChangePassword => url_builder.redirect(&AccountPassword),
+            Self::ChangePassword => url_builder.redirect(&AccountPasswordChange),
             Self::LinkUpstream { id } => url_builder.redirect(&UpstreamOAuth2Link::new(*id)),
             Self::ManageAccount { action } => url_builder.redirect(&Account {
                 action: action.clone(),
@@ -506,12 +506,15 @@ impl SimpleRoute for AccountWildcard {
     const PATH: &'static str = "/account/*rest";
 }
 
-/// `GET|POST /change-password`
+/// `GET /account/password/change`
+///
+/// Handled by the React frontend; this struct definition is purely for
+/// redirects.
 #[derive(Default, Debug, Clone)]
-pub struct AccountPassword;
+pub struct AccountPasswordChange;
 
-impl SimpleRoute for AccountPassword {
+impl SimpleRoute for AccountPasswordChange {
-    const PATH: &'static str = "/change-password";
+    const PATH: &'static str = "/account/password/change";
 }
 
 /// `GET /authorize/:grant_id`

crates/templates/src/lib.rs

@@ -341,9 +341,6 @@ register_templates! {
     /// Render the home page
     pub fn render_index(WithLanguage<WithCsrf<WithOptionalSession<IndexContext>>>) { "pages/index.html" }
 
-    /// Render the password change page
-    pub fn render_account_password(WithLanguage<WithCsrf<WithSession<EmptyContext>>>) { "pages/account/password.html" }
-
     /// Render the email verification page
     pub fn render_account_verify_email(WithLanguage<WithCsrf<WithSession<EmailVerificationPageContext>>>) { "pages/account/emails/verify.html" }
 
@@ -404,7 +401,6 @@ impl Templates {
         check::render_policy_violation(self, now, rng)?;
         check::render_sso_login(self, now, rng)?;
         check::render_index(self, now, rng)?;
-        check::render_account_password(self, now, rng)?;
         check::render_account_add_email(self, now, rng)?;
         check::render_account_verify_email(self, now, rng)?;
         check::render_reauth(self, now, rng)?;

templates/pages/account/password.html

@@ -1,49 +0,0 @@
-{#
-Copyright 2022 The Matrix.org Foundation C.I.C.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-#}
-
-{% extends "base.html" %}
-
-{% block content %}
-  <header class="page-heading">
-    <div class="icon">
-      {{ icon.lock() }}
-    </div>
-
-    <div class="header">
-      <h1 class="title">{{ _("mas.change_password.heading") }}</h1>
-      <p class="text">{{ _("mas.change_password.description") }}</p>
-    </div>
-  </header>
-
-  <form class="cpd-form-root" method="POST">
-    <input type="hidden" name="csrf" value="{{ csrf_token }}" />
-
-    {% call(f) field.field(label=_("mas.change_password.current"), name="current_password") %}
-      <input {{ field.attributes(f) }} class="cpd-text-control" type="password" autocomplete="current-password" required />
-    {% endcall %}
-
-    {% call(f) field.field(label=_("mas.change_password.new"), name="new_password") %}
-      <input {{ field.attributes(f) }} class="cpd-text-control" type="password" autocomplete="new-password" required />
-    {% endcall %}
-
-    {% call(f) field.field(label=_("mas.change_password.confirm"), name="new_password_confirm") %}
-      <input {{ field.attributes(f) }} class="cpd-text-control" type="password" autocomplete="new-password" required />
-    {% endcall %}
-
-    {{ button.button(text=_("mas.change_password.change"), type="submit") }}
-  </form>
-{% endblock content %}
-

translations/en.json

@@ -118,31 +118,24 @@
     "change_password": {
       "change": "Change password",
       "@change": {
-        "context": "pages/account/password.html:46:26-57",
         "description": "Button to change the user's password"
       },
       "confirm": "Confirm password",
       "@confirm": {
-        "context": "pages/account/password.html:42:33-65",
         "description": "Confirmation field for the new password"
       },
       "current": "Current password",
       "@current": {
-        "context": "pages/account/password.html:34:33-65",
         "description": "Field for the user's current password"
       },
       "description": "This will change the password on your account.",
-      "@description": {
+      "@description": {},
-        "context": "pages/account/password.html:27:25-61"
-      },
       "heading": "Change my password",
       "@heading": {
-        "context": "pages/account/password.html:26:27-59",
         "description": "Heading on the change password page"
       },
       "new": "New password",
       "@new": {
-        "context": "pages/account/password.html:38:33-61",
         "description": "Field for the user's new password"
       }
     },