diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index 4652c839..5efb6caa 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -354,7 +354,7 @@ where .route( mas_router::ChangePasswordDiscovery::route(), get(|State(url_builder): State| 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) diff --git a/crates/handlers/src/views/account/mod.rs b/crates/handlers/src/views/account/mod.rs index 3c9090ae..b349e19c 100644 --- a/crates/handlers/src/views/account/mod.rs +++ b/crates/handlers/src/views/account/mod.rs @@ -13,4 +13,3 @@ // limitations under the License. pub mod emails; -pub mod password; diff --git a/crates/handlers/src/views/account/password.rs b/crates/handlers/src/views/account/password.rs deleted file mode 100644 index 086cf304..00000000 --- a/crates/handlers/src/views/account/password.rs +++ /dev/null @@ -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, - State(site_config): State, - activity_tracker: BoundActivityTracker, - State(url_builder): State, - mut repo: BoxRepository, - cookie_jar: CookieJar, -) -> Result { - // 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 { - 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, - State(site_config): State, - State(templates): State, - activity_tracker: BoundActivityTracker, - State(url_builder): State, - mut policy: Policy, - mut repo: BoxRepository, - cookie_jar: CookieJar, - Form(form): Form>, -) -> Result { - 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) -} diff --git a/crates/router/src/endpoints.rs b/crates/router/src/endpoints.rs index 17527b48..cc619c9f 100644 --- a/crates/router/src/endpoints.rs +++ b/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 { - const PATH: &'static str = "/change-password"; +impl SimpleRoute for AccountPasswordChange { + const PATH: &'static str = "/account/password/change"; } /// `GET /authorize/:grant_id` diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index f71ee5c8..ecbdc15c 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -341,9 +341,6 @@ register_templates! { /// Render the home page pub fn render_index(WithLanguage>>) { "pages/index.html" } - /// Render the password change page - pub fn render_account_password(WithLanguage>>) { "pages/account/password.html" } - /// Render the email verification page pub fn render_account_verify_email(WithLanguage>>) { "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)?; diff --git a/templates/pages/account/password.html b/templates/pages/account/password.html deleted file mode 100644 index d3d840d4..00000000 --- a/templates/pages/account/password.html +++ /dev/null @@ -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 %} -
-
- {{ icon.lock() }} -
- -
-

{{ _("mas.change_password.heading") }}

-

{{ _("mas.change_password.description") }}

-
-
- -
- - - {% call(f) field.field(label=_("mas.change_password.current"), name="current_password") %} - - {% endcall %} - - {% call(f) field.field(label=_("mas.change_password.new"), name="new_password") %} - - {% endcall %} - - {% call(f) field.field(label=_("mas.change_password.confirm"), name="new_password_confirm") %} - - {% endcall %} - - {{ button.button(text=_("mas.change_password.change"), type="submit") }} -
-{% endblock content %} - diff --git a/translations/en.json b/translations/en.json index b20f6f02..428958b4 100644 --- a/translations/en.json +++ b/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": { - "context": "pages/account/password.html:27:25-61" - }, + "@description": {}, "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" } },