Skip to content

Commit 4ca76be

Browse files
committed
Polish the password recovery page
This includes: - show an error message if the recovery link is expired, with a button to resend the email - show an error message if the recovery link has already been used - include an invisible username field in the form, so that password managers can save the new password
1 parent 5cbb576 commit 4ca76be

File tree

13 files changed

+729
-57
lines changed

13 files changed

+729
-57
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 New Vector Ltd.
1+
// Copyright 2024, 2025 New Vector Ltd.
22
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
33
//
44
// SPDX-License-Identifier: AGPL-3.0-only
@@ -26,7 +26,7 @@ pub use self::{
2626
oauth::{OAuth2Client, OAuth2Session},
2727
site_config::{SiteConfig, SITE_CONFIG_ID},
2828
upstream_oauth::{UpstreamOAuth2Link, UpstreamOAuth2Provider},
29-
users::{AppSession, User, UserEmail},
29+
users::{AppSession, User, UserEmail, UserRecoveryTicket},
3030
viewer::{Anonymous, Viewer, ViewerSession},
3131
};
3232

@@ -42,6 +42,7 @@ pub enum CreationEvent {
4242
CompatSession(Box<CompatSession>),
4343
BrowserSession(Box<BrowserSession>),
4444
UserEmail(Box<UserEmail>),
45+
UserRecoveryTicket(Box<UserRecoveryTicket>),
4546
UpstreamOAuth2Provider(Box<UpstreamOAuth2Provider>),
4647
UpstreamOAuth2Link(Box<UpstreamOAuth2Link>),
4748
OAuth2Session(Box<OAuth2Session>),

crates/handlers/src/graphql/model/node.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 New Vector Ltd.
1+
// Copyright 2024, 2025 New Vector Ltd.
22
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
33
//
44
// SPDX-License-Identifier: AGPL-3.0-only
@@ -12,6 +12,7 @@ use ulid::Ulid;
1212
use super::{
1313
Anonymous, Authentication, BrowserSession, CompatSession, CompatSsoLogin, OAuth2Client,
1414
OAuth2Session, SiteConfig, UpstreamOAuth2Link, UpstreamOAuth2Provider, User, UserEmail,
15+
UserRecoveryTicket,
1516
};
1617

1718
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -26,6 +27,7 @@ pub enum NodeType {
2627
UpstreamOAuth2Link,
2728
User,
2829
UserEmail,
30+
UserRecoveryTicket,
2931
}
3032

3133
#[derive(Debug, Error)]
@@ -50,6 +52,7 @@ impl NodeType {
5052
NodeType::UpstreamOAuth2Link => "upstream_oauth2_link",
5153
NodeType::User => "user",
5254
NodeType::UserEmail => "user_email",
55+
NodeType::UserRecoveryTicket => "user_recovery_ticket",
5356
}
5457
}
5558

@@ -65,6 +68,7 @@ impl NodeType {
6568
"upstream_oauth2_link" => Some(NodeType::UpstreamOAuth2Link),
6669
"user" => Some(NodeType::User),
6770
"user_email" => Some(NodeType::UserEmail),
71+
"user_recovery_ticket" => Some(NodeType::UserRecoveryTicket),
6872
_ => None,
6973
}
7074
}
@@ -120,4 +124,5 @@ pub enum Node {
120124
UpstreamOAuth2Link(Box<UpstreamOAuth2Link>),
121125
User(Box<User>),
122126
UserEmail(Box<UserEmail>),
127+
UserRecoveryTicket(Box<UserRecoveryTicket>),
123128
}

crates/handlers/src/graphql/model/users.rs

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 New Vector Ltd.
1+
// Copyright 2024, 2025 New Vector Ltd.
22
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
33
//
44
// SPDX-License-Identifier: AGPL-3.0-only
@@ -765,3 +765,101 @@ pub enum UserEmailState {
765765
/// The email address has been confirmed.
766766
Confirmed,
767767
}
768+
769+
/// A recovery ticket
770+
#[derive(Description)]
771+
pub struct UserRecoveryTicket(pub mas_data_model::UserRecoveryTicket);
772+
773+
/// The status of a recovery ticket
774+
#[derive(Enum, Copy, Clone, Eq, PartialEq)]
775+
pub enum UserRecoveryTicketStatus {
776+
/// The ticket is valid
777+
Valid,
778+
779+
/// The ticket has expired
780+
Expired,
781+
782+
/// The ticket has been consumed
783+
Consumed,
784+
}
785+
786+
#[Object(use_type_description)]
787+
impl UserRecoveryTicket {
788+
/// ID of the object.
789+
pub async fn id(&self) -> ID {
790+
NodeType::UserRecoveryTicket.id(self.0.id)
791+
}
792+
793+
/// When the object was created.
794+
pub async fn created_at(&self) -> DateTime<Utc> {
795+
self.0.created_at
796+
}
797+
798+
/// The status of the ticket
799+
pub async fn status(
800+
&self,
801+
context: &Context<'_>,
802+
) -> Result<UserRecoveryTicketStatus, async_graphql::Error> {
803+
let state = context.state();
804+
let clock = state.clock();
805+
let mut repo = state.repository().await?;
806+
807+
// Lookup the session associated with the ticket
808+
let session = repo
809+
.user_recovery()
810+
.lookup_session(self.0.user_recovery_session_id)
811+
.await?
812+
.context("Failed to lookup session")?;
813+
repo.cancel().await?;
814+
815+
if session.consumed_at.is_some() {
816+
return Ok(UserRecoveryTicketStatus::Consumed);
817+
}
818+
819+
if self.0.expires_at < clock.now() {
820+
return Ok(UserRecoveryTicketStatus::Expired);
821+
}
822+
823+
Ok(UserRecoveryTicketStatus::Valid)
824+
}
825+
826+
/// The username associated with this ticket
827+
pub async fn username(&self, ctx: &Context<'_>) -> Result<String, async_graphql::Error> {
828+
// We could expose the UserEmail, then the User, but this is unauthenticated, so
829+
// we don't want to risk leaking too many objects. Instead, we just give the
830+
// username as a property of the UserRecoveryTicket
831+
let state = ctx.state();
832+
let mut repo = state.repository().await?;
833+
let user_email = repo
834+
.user_email()
835+
.lookup(self.0.user_email_id)
836+
.await?
837+
.context("Failed to lookup user email")?;
838+
839+
let user = repo
840+
.user()
841+
.lookup(user_email.user_id)
842+
.await?
843+
.context("Failed to lookup user")?;
844+
repo.cancel().await?;
845+
846+
Ok(user.username)
847+
}
848+
849+
/// The email address associated with this ticket
850+
pub async fn email(&self, ctx: &Context<'_>) -> Result<String, async_graphql::Error> {
851+
// We could expose the UserEmail directly, but this is unauthenticated, so we
852+
// don't want to risk leaking too many objects. Instead, we just give
853+
// the email as a property of the UserRecoveryTicket
854+
let state = ctx.state();
855+
let mut repo = state.repository().await?;
856+
let user_email = repo
857+
.user_email()
858+
.lookup(self.0.user_email_id)
859+
.await?
860+
.context("Failed to lookup user email")?;
861+
repo.cancel().await?;
862+
863+
Ok(user_email.email)
864+
}
865+
}

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

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 New Vector Ltd.
1+
// Copyright 2024, 2025 New Vector Ltd.
22
// Copyright 2023, 2024 The Matrix.org Foundation C.I.C.
33
//
44
// SPDX-License-Identifier: AGPL-3.0-only
@@ -7,10 +7,15 @@
77
use anyhow::Context as _;
88
use async_graphql::{Context, Description, Enum, InputObject, Object, ID};
99
use mas_storage::{
10-
queue::{DeactivateUserJob, ProvisionUserJob, QueueJobRepositoryExt as _},
10+
queue::{
11+
DeactivateUserJob, ProvisionUserJob, QueueJobRepositoryExt as _,
12+
SendAccountRecoveryEmailsJob,
13+
},
1114
user::UserRepository,
1215
};
1316
use tracing::{info, warn};
17+
use ulid::Ulid;
18+
use url::Url;
1419
use zeroize::Zeroizing;
1520

1621
use crate::graphql::{
@@ -323,6 +328,61 @@ impl SetPasswordPayload {
323328
}
324329
}
325330

331+
/// The input for the `resendRecoveryEmail` mutation.
332+
#[derive(InputObject)]
333+
pub struct ResendRecoveryEmailInput {
334+
/// The recovery ticket to use.
335+
ticket: String,
336+
}
337+
338+
/// The return type for the `resendRecoveryEmail` mutation.
339+
#[derive(Description)]
340+
pub enum ResendRecoveryEmailPayload {
341+
NoSuchRecoveryTicket,
342+
RateLimited,
343+
Sent { recovery_session_id: Ulid },
344+
}
345+
346+
/// The status of the `resendRecoveryEmail` mutation.
347+
#[derive(Enum, Copy, Clone, Eq, PartialEq)]
348+
pub enum ResendRecoveryEmailStatus {
349+
/// The recovery ticket was not found.
350+
NoSuchRecoveryTicket,
351+
352+
/// The rate limit was exceeded.
353+
RateLimited,
354+
355+
/// The recovery email was sent.
356+
Sent,
357+
}
358+
359+
#[Object(use_type_description)]
360+
impl ResendRecoveryEmailPayload {
361+
/// Status of the operation
362+
async fn status(&self) -> ResendRecoveryEmailStatus {
363+
match self {
364+
Self::NoSuchRecoveryTicket => ResendRecoveryEmailStatus::NoSuchRecoveryTicket,
365+
Self::RateLimited => ResendRecoveryEmailStatus::RateLimited,
366+
Self::Sent { .. } => ResendRecoveryEmailStatus::Sent,
367+
}
368+
}
369+
370+
/// URL to continue the recovery process
371+
async fn progress_url(&self, context: &Context<'_>) -> Option<Url> {
372+
let state = context.state();
373+
let url_builder = state.url_builder();
374+
match self {
375+
Self::NoSuchRecoveryTicket | Self::RateLimited => None,
376+
Self::Sent {
377+
recovery_session_id,
378+
} => {
379+
let route = mas_router::AccountRecoveryProgress::new(*recovery_session_id);
380+
Some(url_builder.absolute_url_for(&route))
381+
}
382+
}
383+
}
384+
}
385+
326386
fn valid_username_character(c: char) -> bool {
327387
c.is_ascii_lowercase()
328388
|| c.is_ascii_digit()
@@ -760,4 +820,54 @@ impl UserMutations {
760820
status: SetPasswordStatus::Allowed,
761821
})
762822
}
823+
824+
/// Resend a user recovery email
825+
///
826+
/// This is used when a user opens a recovery link that has expired. In this
827+
/// case, we display a link for them to get a new recovery email, which
828+
/// calls this mutation.
829+
pub async fn resend_recovery_email(
830+
&self,
831+
ctx: &Context<'_>,
832+
input: ResendRecoveryEmailInput,
833+
) -> Result<ResendRecoveryEmailPayload, async_graphql::Error> {
834+
let state = ctx.state();
835+
let requester_fingerprint = ctx.requester_fingerprint();
836+
let clock = state.clock();
837+
let mut rng = state.rng();
838+
let limiter = state.limiter();
839+
let mut repo = state.repository().await?;
840+
841+
let Some(recovery_ticket) = repo.user_recovery().find_ticket(&input.ticket).await? else {
842+
return Ok(ResendRecoveryEmailPayload::NoSuchRecoveryTicket);
843+
};
844+
845+
let recovery_session = repo
846+
.user_recovery()
847+
.lookup_session(recovery_ticket.user_recovery_session_id)
848+
.await?
849+
.context("Could not load recovery session")?;
850+
851+
if let Err(e) =
852+
limiter.check_account_recovery(requester_fingerprint, &recovery_session.email)
853+
{
854+
tracing::warn!(error = &e as &dyn std::error::Error);
855+
return Ok(ResendRecoveryEmailPayload::RateLimited);
856+
}
857+
858+
// Schedule a new batch of emails
859+
repo.queue_job()
860+
.schedule_job(
861+
&mut rng,
862+
&clock,
863+
SendAccountRecoveryEmailsJob::new(&recovery_session),
864+
)
865+
.await?;
866+
867+
repo.save().await?;
868+
869+
Ok(ResendRecoveryEmailPayload::Sent {
870+
recovery_session_id: recovery_session.id,
871+
})
872+
}
763873
}

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 New Vector Ltd.
1+
// Copyright 2024, 2025 New Vector Ltd.
22
// Copyright 2023, 2024 The Matrix.org Foundation C.I.C.
33
//
44
// SPDX-License-Identifier: AGPL-3.0-only
@@ -9,7 +9,7 @@ use async_graphql::{Context, MergedObject, Object, ID};
99
use crate::graphql::{
1010
model::{
1111
Anonymous, BrowserSession, CompatSession, Node, NodeType, OAuth2Client, OAuth2Session,
12-
SiteConfig, User, UserEmail,
12+
SiteConfig, User, UserEmail, UserRecoveryTicket,
1313
},
1414
state::ContextExt,
1515
};
@@ -182,6 +182,20 @@ impl BaseQuery {
182182
Ok(Some(UserEmail(user_email)))
183183
}
184184

185+
/// Fetch a user recovery ticket.
186+
async fn user_recovery_ticket(
187+
&self,
188+
ctx: &Context<'_>,
189+
ticket: String,
190+
) -> Result<Option<UserRecoveryTicket>, async_graphql::Error> {
191+
let state = ctx.state();
192+
let mut repo = state.repository().await?;
193+
let ticket = repo.user_recovery().find_ticket(&ticket).await?;
194+
repo.cancel().await?;
195+
196+
Ok(ticket.map(UserRecoveryTicket))
197+
}
198+
185199
/// Fetches an object given its ID.
186200
async fn node(&self, ctx: &Context<'_>, id: ID) -> Result<Option<Node>, async_graphql::Error> {
187201
// Special case for the anonymous user
@@ -199,7 +213,9 @@ impl BaseQuery {
199213

200214
let ret = match node_type {
201215
// TODO
202-
NodeType::Authentication | NodeType::CompatSsoLogin => None,
216+
NodeType::Authentication | NodeType::CompatSsoLogin | NodeType::UserRecoveryTicket => {
217+
None
218+
}
203219

204220
NodeType::UpstreamOAuth2Provider => UpstreamOAuthQuery
205221
.upstream_oauth2_provider(ctx, id)

crates/handlers/src/lib.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2024 New Vector Ltd.
1+
// Copyright 2024, 2025 New Vector Ltd.
22
// Copyright 2021-2024 The Matrix.org Foundation C.I.C.
33
//
44
// SPDX-License-Identifier: AGPL-3.0-only
@@ -118,6 +118,8 @@ where
118118
BoxClock: FromRequestParts<S>,
119119
Encrypter: FromRef<S>,
120120
CookieJar: FromRequestParts<S>,
121+
Limiter: FromRef<S>,
122+
RequesterFingerprint: FromRequestParts<S>,
121123
{
122124
let mut router = Router::new()
123125
.route(

0 commit comments

Comments
 (0)