Skip to content

Commit bce99ed

Browse files
sandhosesandhose
authored
Better feedback when changing passwords (#5153)
2 parents 341ac89 + e221a37 commit bce99ed

File tree

4 files changed

+311
-8
lines changed

4 files changed

+311
-8
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ async fn verify_password_if_needed(
8484
password,
8585
user_password.hashed_password,
8686
)
87-
.await;
87+
.await?;
8888

89-
Ok(res.is_ok())
89+
Ok(res.is_success())
9090
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,13 +737,14 @@ impl UserMutations {
737737
));
738738
};
739739

740-
if let Err(_err) = password_manager
740+
if !password_manager
741741
.verify(
742742
active_password.version,
743743
Zeroizing::new(current_password_attempt),
744744
active_password.hashed_password,
745745
)
746-
.await
746+
.await?
747+
.is_success()
747748
{
748749
return Ok(SetPasswordPayload {
749750
status: SetPasswordStatus::WrongPassword,

‎crates/handlers/src/graphql/tests.rs‎

Lines changed: 301 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use axum::http::Request;
88
use hyper::StatusCode;
9+
use mas_axum_utils::SessionInfoExt;
910
use mas_data_model::{AccessToken, Client, TokenType, User};
1011
use mas_matrix::{HomeserverConnection, ProvisionRequest};
1112
use mas_router::SimpleRoute;
@@ -19,11 +20,9 @@ use oauth2_types::{
1920
scope::{OPENID, Scope, ScopeToken},
2021
};
2122
use sqlx::PgPool;
23+
use zeroize::Zeroizing;
2224

23-
use crate::{
24-
test_utils,
25-
test_utils::{RequestBuilderExt, ResponseExt, TestState, setup},
26-
};
25+
use crate::test_utils::{self, CookieHelper, RequestBuilderExt, ResponseExt, TestState, setup};
2726

2827
async fn create_test_client(state: &TestState) -> Client {
2928
let mut repo = state.repository().await.unwrap();
@@ -781,3 +780,301 @@ async fn test_add_user(pool: PgPool) {
781780
})
782781
);
783782
}
783+
784+
/// Test the setPassword mutation where the current password provided is
785+
/// wrong.
786+
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
787+
async fn test_set_password_rejected_wrong_password(pool: PgPool) {
788+
setup();
789+
let state = TestState::from_pool(pool).await.unwrap();
790+
791+
let mut rng = state.rng();
792+
let mut repo = state.repository().await.unwrap();
793+
let user = repo
794+
.user()
795+
.add(&mut rng, &state.clock, "alice".to_owned())
796+
.await
797+
.unwrap();
798+
let password = Zeroizing::new("current.password.123".to_owned());
799+
let (version, hashed_password) = state
800+
.password_manager
801+
.hash(&mut rng, password)
802+
.await
803+
.unwrap();
804+
805+
repo.user_password()
806+
.add(
807+
&mut rng,
808+
&state.clock,
809+
&user,
810+
version,
811+
hashed_password,
812+
None,
813+
)
814+
.await
815+
.unwrap();
816+
let browser_session = repo
817+
.browser_session()
818+
.add(&mut rng, &state.clock, &user, None)
819+
.await
820+
.unwrap();
821+
repo.save().await.unwrap();
822+
823+
let cookie_jar = state.cookie_jar();
824+
let cookie_jar = cookie_jar.set_session(&browser_session);
825+
826+
let user_id = user.id;
827+
828+
let request = Request::post("/graphql").json(serde_json::json!({
829+
"query": format!(r#"
830+
mutation {{
831+
setPassword(input: {{
832+
userId: "user:{user_id}",
833+
currentPassword: "wrong.password.123",
834+
newPassword: "new.password.123"
835+
}}) {{
836+
status
837+
}}
838+
}}
839+
"#),
840+
}));
841+
842+
let cookies = CookieHelper::new();
843+
cookies.import(cookie_jar);
844+
let request = cookies.with_cookies(request);
845+
846+
let response = state.request(request).await;
847+
response.assert_status(StatusCode::OK);
848+
let response: GraphQLResponse = response.json();
849+
assert!(response.errors.is_empty(), "{:?}", response.errors);
850+
assert_eq!(
851+
response.data["setPassword"]["status"].as_str(),
852+
Some("WRONG_PASSWORD"),
853+
"{:?}",
854+
response.data
855+
);
856+
}
857+
858+
/// Test the startEmailAuthentication mutation where the current password
859+
/// provided is invalid.
860+
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
861+
async fn test_start_email_authentication_rejected_wrong_password(pool: PgPool) {
862+
setup();
863+
let state = TestState::from_pool(pool).await.unwrap();
864+
865+
let mut rng = state.rng();
866+
let mut repo = state.repository().await.unwrap();
867+
let user = repo
868+
.user()
869+
.add(&mut rng, &state.clock, "alice".to_owned())
870+
.await
871+
.unwrap();
872+
let password = Zeroizing::new("current.password.123".to_owned());
873+
let (version, hashed_password) = state
874+
.password_manager
875+
.hash(&mut rng, password)
876+
.await
877+
.unwrap();
878+
879+
repo.user_password()
880+
.add(
881+
&mut rng,
882+
&state.clock,
883+
&user,
884+
version,
885+
hashed_password,
886+
None,
887+
)
888+
.await
889+
.unwrap();
890+
let browser_session = repo
891+
.browser_session()
892+
.add(&mut rng, &state.clock, &user, None)
893+
.await
894+
.unwrap();
895+
repo.save().await.unwrap();
896+
897+
let cookie_jar = state.cookie_jar();
898+
let cookie_jar = cookie_jar.set_session(&browser_session);
899+
900+
let request = Request::post("/graphql").json(serde_json::json!({
901+
"query": r#"
902+
mutation {
903+
startEmailAuthentication(input: {
904+
email: "alice@example.org",
905+
password: "wrong.password.123"
906+
}) {
907+
status
908+
}
909+
}
910+
"#,
911+
}));
912+
913+
let cookies = CookieHelper::new();
914+
cookies.import(cookie_jar);
915+
let request = cookies.with_cookies(request);
916+
917+
let response = state.request(request).await;
918+
response.assert_status(StatusCode::OK);
919+
let response: GraphQLResponse = response.json();
920+
assert!(response.errors.is_empty(), "{:?}", response.errors);
921+
assert_eq!(
922+
response.data["startEmailAuthentication"]["status"].as_str(),
923+
Some("INCORRECT_PASSWORD"),
924+
"{:?}",
925+
response.data
926+
);
927+
}
928+
929+
/// Test the removeEmail mutation where the current password
930+
/// provided is invalid.
931+
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
932+
async fn test_remove_email_rejected_wrong_password(pool: PgPool) {
933+
setup();
934+
let state = TestState::from_pool(pool).await.unwrap();
935+
936+
let mut rng = state.rng();
937+
let mut repo = state.repository().await.unwrap();
938+
let user = repo
939+
.user()
940+
.add(&mut rng, &state.clock, "alice".to_owned())
941+
.await
942+
.unwrap();
943+
let password = Zeroizing::new("current.password.123".to_owned());
944+
let (version, hashed_password) = state
945+
.password_manager
946+
.hash(&mut rng, password)
947+
.await
948+
.unwrap();
949+
950+
repo.user_password()
951+
.add(
952+
&mut rng,
953+
&state.clock,
954+
&user,
955+
version,
956+
hashed_password,
957+
None,
958+
)
959+
.await
960+
.unwrap();
961+
let user_email_id = repo
962+
.user_email()
963+
.add(
964+
&mut rng,
965+
&state.clock,
966+
&user,
967+
"alice@example.org".to_owned(),
968+
)
969+
.await
970+
.unwrap()
971+
.id;
972+
let browser_session = repo
973+
.browser_session()
974+
.add(&mut rng, &state.clock, &user, None)
975+
.await
976+
.unwrap();
977+
repo.save().await.unwrap();
978+
979+
let cookie_jar = state.cookie_jar();
980+
let cookie_jar = cookie_jar.set_session(&browser_session);
981+
982+
let request = Request::post("/graphql").json(serde_json::json!({
983+
"query": format!(r#"
984+
mutation {{
985+
removeEmail(input: {{
986+
userEmailId: "user_email:{user_email_id}",
987+
password: "wrong.password.123"
988+
}}) {{
989+
status
990+
}}
991+
}}
992+
"#),
993+
}));
994+
995+
let cookies = CookieHelper::new();
996+
cookies.import(cookie_jar);
997+
let request = cookies.with_cookies(request);
998+
999+
let response = state.request(request).await;
1000+
response.assert_status(StatusCode::OK);
1001+
let response: GraphQLResponse = response.json();
1002+
assert!(response.errors.is_empty(), "{:?}", response.errors);
1003+
assert_eq!(
1004+
response.data["removeEmail"]["status"].as_str(),
1005+
Some("INCORRECT_PASSWORD"),
1006+
"{:?}",
1007+
response.data
1008+
);
1009+
}
1010+
1011+
/// Test the deactivateUser mutation where the current password
1012+
/// provided is invalid.
1013+
#[sqlx::test(migrator = "mas_storage_pg::MIGRATOR")]
1014+
async fn test_deactivate_user_rejected_wrong_password(pool: PgPool) {
1015+
setup();
1016+
let state = TestState::from_pool(pool).await.unwrap();
1017+
1018+
let mut rng = state.rng();
1019+
let mut repo = state.repository().await.unwrap();
1020+
let user = repo
1021+
.user()
1022+
.add(&mut rng, &state.clock, "alice".to_owned())
1023+
.await
1024+
.unwrap();
1025+
let password = Zeroizing::new("current.password.123".to_owned());
1026+
let (version, hashed_password) = state
1027+
.password_manager
1028+
.hash(&mut rng, password)
1029+
.await
1030+
.unwrap();
1031+
1032+
repo.user_password()
1033+
.add(
1034+
&mut rng,
1035+
&state.clock,
1036+
&user,
1037+
version,
1038+
hashed_password,
1039+
None,
1040+
)
1041+
.await
1042+
.unwrap();
1043+
let browser_session = repo
1044+
.browser_session()
1045+
.add(&mut rng, &state.clock, &user, None)
1046+
.await
1047+
.unwrap();
1048+
repo.save().await.unwrap();
1049+
1050+
let cookie_jar = state.cookie_jar();
1051+
let cookie_jar = cookie_jar.set_session(&browser_session);
1052+
1053+
let request = Request::post("/graphql").json(serde_json::json!({
1054+
"query": r#"
1055+
mutation {
1056+
deactivateUser(input: {
1057+
hsErase: true,
1058+
password: "wrong.password.123"
1059+
}) {
1060+
status
1061+
}
1062+
}
1063+
"#,
1064+
}));
1065+
1066+
let cookies = CookieHelper::new();
1067+
cookies.import(cookie_jar);
1068+
let request = cookies.with_cookies(request);
1069+
1070+
let response = state.request(request).await;
1071+
response.assert_status(StatusCode::OK);
1072+
let response: GraphQLResponse = response.json();
1073+
assert!(response.errors.is_empty(), "{:?}", response.errors);
1074+
assert_eq!(
1075+
response.data["deactivateUser"]["status"].as_str(),
1076+
Some("INCORRECT_PASSWORD"),
1077+
"{:?}",
1078+
response.data
1079+
);
1080+
}

‎crates/handlers/src/passwords.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ impl<T> PasswordVerificationResult<T> {
4949
Self::Failure => PasswordVerificationResult::Failure,
5050
}
5151
}
52+
53+
#[must_use]
54+
pub fn is_success(&self) -> bool {
55+
matches!(self, Self::Success(_))
56+
}
5257
}
5358

5459
impl From<bool> for PasswordVerificationResult<()> {

0 commit comments

Comments
 (0)