Better feedback when changing passwords (#5153)

crates/handlers/src/graphql/mutations/mod.rs

@@ -84,7 +84,7 @@ async fn verify_password_if_needed(
             password,
             user_password.hashed_password,
         )
-        .await;
+        .await?;
 
-    Ok(res.is_ok())
+    Ok(res.is_success())
 }

crates/handlers/src/graphql/mutations/user.rs

@@ -737,13 +737,14 @@ impl UserMutations {
                 ));
             };
 
-            if let Err(_err) = password_manager
+            if !password_manager
                 .verify(
                     active_password.version,
                     Zeroizing::new(current_password_attempt),
                     active_password.hashed_password,
                 )
-                .await
+                .await?
+                .is_success()
             {
                 return Ok(SetPasswordPayload {
                     status: SetPasswordStatus::WrongPassword,

crates/handlers/src/graphql/tests.rs

@@ -6,6 +6,7 @@
 
 use axum::http::Request;
 use hyper::StatusCode;
+use mas_axum_utils::SessionInfoExt;
 use mas_data_model::{AccessToken, Client, TokenType, User};
 use mas_matrix::{HomeserverConnection, ProvisionRequest};
 use mas_router::SimpleRoute;
@@ -19,11 +20,9 @@ use oauth2_types::{
     scope::{OPENID, Scope, ScopeToken},
 };
 use sqlx::PgPool;
+use zeroize::Zeroizing;
 
-use crate::{
-    test_utils,
-    test_utils::{RequestBuilderExt, ResponseExt, TestState, setup},
-};
+use crate::test_utils::{self, CookieHelper, RequestBuilderExt, ResponseExt, TestState, setup};
 
 async fn create_test_client(state: &TestState) -> Client {
     let mut repo = state.repository().await.unwrap();
@@ -781,3 +780,301 @@ async fn test_add_user(pool: PgPool) {
         })
     );
 }
+
+/// Test the setPassword mutation where the current password provided is
+/// wrong.
+#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
+async fn test_set_password_rejected_wrong_password(pool: PgPool) {
+    setup();
+    let state = TestState::from_pool(pool).await.unwrap();
+
+    let mut rng = state.rng();
+    let mut repo = state.repository().await.unwrap();
+    let user = repo
+        .user()
+        .add(&mut rng, &state.clock, "alice".to_owned())
+        .await
+        .unwrap();
+    let password = Zeroizing::new("current.password.123".to_owned());
+    let (version, hashed_password) = state
+        .password_manager
+        .hash(&mut rng, password)
+        .await
+        .unwrap();
+
+    repo.user_password()
+        .add(
+            &mut rng,
+            &state.clock,
+            &user,
+            version,
+            hashed_password,
+            None,
+        )
+        .await
+        .unwrap();
+    let browser_session = repo
+        .browser_session()
+        .add(&mut rng, &state.clock, &user, None)
+        .await
+        .unwrap();
+    repo.save().await.unwrap();
+
+    let cookie_jar = state.cookie_jar();
+    let cookie_jar = cookie_jar.set_session(&browser_session);
+
+    let user_id = user.id;
+
+    let request = Request::post("/graphql").json(serde_json::json!({
+        "query": format!(r#"
+                mutation {{
+                    setPassword(input: {{
+                        userId: "user:{user_id}",
+                        currentPassword: "wrong.password.123",
+                        newPassword: "new.password.123"
+                    }}) {{
+                        status
+                    }}
+                }}
+            "#),
+    }));
+
+    let cookies = CookieHelper::new();
+    cookies.import(cookie_jar);
+    let request = cookies.with_cookies(request);
+
+    let response = state.request(request).await;
+    response.assert_status(StatusCode::OK);
+    let response: GraphQLResponse = response.json();
+    assert!(response.errors.is_empty(), "{:?}", response.errors);
+    assert_eq!(
+        response.data["setPassword"]["status"].as_str(),
+        Some("WRONG_PASSWORD"),
+        "{:?}",
+        response.data
+    );
+}
+
+/// Test the startEmailAuthentication mutation where the current password
+/// provided is invalid.
+#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
+async fn test_start_email_authentication_rejected_wrong_password(pool: PgPool) {
+    setup();
+    let state = TestState::from_pool(pool).await.unwrap();
+
+    let mut rng = state.rng();
+    let mut repo = state.repository().await.unwrap();
+    let user = repo
+        .user()
+        .add(&mut rng, &state.clock, "alice".to_owned())
+        .await
+        .unwrap();
+    let password = Zeroizing::new("current.password.123".to_owned());
+    let (version, hashed_password) = state
+        .password_manager
+        .hash(&mut rng, password)
+        .await
+        .unwrap();
+
+    repo.user_password()
+        .add(
+            &mut rng,
+            &state.clock,
+            &user,
+            version,
+            hashed_password,
+            None,
+        )
+        .await
+        .unwrap();
+    let browser_session = repo
+        .browser_session()
+        .add(&mut rng, &state.clock, &user, None)
+        .await
+        .unwrap();
+    repo.save().await.unwrap();
+
+    let cookie_jar = state.cookie_jar();
+    let cookie_jar = cookie_jar.set_session(&browser_session);
+
+    let request = Request::post("/graphql").json(serde_json::json!({
+        "query": r#"
+            mutation {
+                startEmailAuthentication(input: {
+                    email: "alice@example.org",
+                    password: "wrong.password.123"
+                }) {
+                    status
+                }
+            }
+        "#,
+    }));
+
+    let cookies = CookieHelper::new();
+    cookies.import(cookie_jar);
+    let request = cookies.with_cookies(request);
+
+    let response = state.request(request).await;
+    response.assert_status(StatusCode::OK);
+    let response: GraphQLResponse = response.json();
+    assert!(response.errors.is_empty(), "{:?}", response.errors);
+    assert_eq!(
+        response.data["startEmailAuthentication"]["status"].as_str(),
+        Some("INCORRECT_PASSWORD"),
+        "{:?}",
+        response.data
+    );
+}
+
+/// Test the removeEmail mutation where the current password
+/// provided is invalid.
+#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
+async fn test_remove_email_rejected_wrong_password(pool: PgPool) {
+    setup();
+    let state = TestState::from_pool(pool).await.unwrap();
+
+    let mut rng = state.rng();
+    let mut repo = state.repository().await.unwrap();
+    let user = repo
+        .user()
+        .add(&mut rng, &state.clock, "alice".to_owned())
+        .await
+        .unwrap();
+    let password = Zeroizing::new("current.password.123".to_owned());
+    let (version, hashed_password) = state
+        .password_manager
+        .hash(&mut rng, password)
+        .await
+        .unwrap();
+
+    repo.user_password()
+        .add(
+            &mut rng,
+            &state.clock,
+            &user,
+            version,
+            hashed_password,
+            None,
+        )
+        .await
+        .unwrap();
+    let user_email_id = repo
+        .user_email()
+        .add(
+            &mut rng,
+            &state.clock,
+            &user,
+            "alice@example.org".to_owned(),
+        )
+        .await
+        .unwrap()
+        .id;
+    let browser_session = repo
+        .browser_session()
+        .add(&mut rng, &state.clock, &user, None)
+        .await
+        .unwrap();
+    repo.save().await.unwrap();
+
+    let cookie_jar = state.cookie_jar();
+    let cookie_jar = cookie_jar.set_session(&browser_session);
+
+    let request = Request::post("/graphql").json(serde_json::json!({
+        "query": format!(r#"
+            mutation {{
+                removeEmail(input: {{
+                    userEmailId: "user_email:{user_email_id}",
+                    password: "wrong.password.123"
+                }}) {{
+                    status
+                }}
+            }}
+        "#),
+    }));
+
+    let cookies = CookieHelper::new();
+    cookies.import(cookie_jar);
+    let request = cookies.with_cookies(request);
+
+    let response = state.request(request).await;
+    response.assert_status(StatusCode::OK);
+    let response: GraphQLResponse = response.json();
+    assert!(response.errors.is_empty(), "{:?}", response.errors);
+    assert_eq!(
+        response.data["removeEmail"]["status"].as_str(),
+        Some("INCORRECT_PASSWORD"),
+        "{:?}",
+        response.data
+    );
+}
+
+/// Test the deactivateUser mutation where the current password
+/// provided is invalid.
+#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
+async fn test_deactivate_user_rejected_wrong_password(pool: PgPool) {
+    setup();
+    let state = TestState::from_pool(pool).await.unwrap();
+
+    let mut rng = state.rng();
+    let mut repo = state.repository().await.unwrap();
+    let user = repo
+        .user()
+        .add(&mut rng, &state.clock, "alice".to_owned())
+        .await
+        .unwrap();
+    let password = Zeroizing::new("current.password.123".to_owned());
+    let (version, hashed_password) = state
+        .password_manager
+        .hash(&mut rng, password)
+        .await
+        .unwrap();
+
+    repo.user_password()
+        .add(
+            &mut rng,
+            &state.clock,
+            &user,
+            version,
+            hashed_password,
+            None,
+        )
+        .await
+        .unwrap();
+    let browser_session = repo
+        .browser_session()
+        .add(&mut rng, &state.clock, &user, None)
+        .await
+        .unwrap();
+    repo.save().await.unwrap();
+
+    let cookie_jar = state.cookie_jar();
+    let cookie_jar = cookie_jar.set_session(&browser_session);
+
+    let request = Request::post("/graphql").json(serde_json::json!({
+        "query": r#"
+            mutation {
+                deactivateUser(input: {
+                    hsErase: true,
+                    password: "wrong.password.123"
+                }) {
+                    status
+                }
+            }
+        "#,
+    }));
+
+    let cookies = CookieHelper::new();
+    cookies.import(cookie_jar);
+    let request = cookies.with_cookies(request);
+
+    let response = state.request(request).await;
+    response.assert_status(StatusCode::OK);
+    let response: GraphQLResponse = response.json();
+    assert!(response.errors.is_empty(), "{:?}", response.errors);
+    assert_eq!(
+        response.data["deactivateUser"]["status"].as_str(),
+        Some("INCORRECT_PASSWORD"),
+        "{:?}",
+        response.data
+    );
+}

crates/handlers/src/passwords.rs

@@ -49,6 +49,11 @@ impl<T> PasswordVerificationResult<T> {
             Self::Failure => PasswordVerificationResult::Failure,
         }
     }
+
+    #[must_use]
+    pub fn is_success(&self) -> bool {
+        matches!(self, Self::Success(_))
+    }
 }
 
 impl From<bool> for PasswordVerificationResult<()> {