Skip to content

Commit

Permalink
Polish the password recovery page
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sandhose committed Jan 7, 2025
1 parent 2ebb59b commit ab7be10
Show file tree
Hide file tree
Showing 13 changed files with 715 additions and 57 deletions.
5 changes: 3 additions & 2 deletions crates/handlers/src/graphql/model/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand Down Expand Up @@ -26,7 +26,7 @@ pub use self::{
oauth::{OAuth2Client, OAuth2Session},
site_config::{SiteConfig, SITE_CONFIG_ID},
upstream_oauth::{UpstreamOAuth2Link, UpstreamOAuth2Provider},
users::{AppSession, User, UserEmail},
users::{AppSession, User, UserEmail, UserRecoveryTicket},
viewer::{Anonymous, Viewer, ViewerSession},
};

Expand All @@ -42,6 +42,7 @@ pub enum CreationEvent {
CompatSession(Box<CompatSession>),
BrowserSession(Box<BrowserSession>),
UserEmail(Box<UserEmail>),
UserRecoveryTicket(Box<UserRecoveryTicket>),
UpstreamOAuth2Provider(Box<UpstreamOAuth2Provider>),
UpstreamOAuth2Link(Box<UpstreamOAuth2Link>),
OAuth2Session(Box<OAuth2Session>),
Expand Down
7 changes: 6 additions & 1 deletion crates/handlers/src/graphql/model/node.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -12,6 +12,7 @@ use ulid::Ulid;
use super::{
Anonymous, Authentication, BrowserSession, CompatSession, CompatSsoLogin, OAuth2Client,
OAuth2Session, SiteConfig, UpstreamOAuth2Link, UpstreamOAuth2Provider, User, UserEmail,
UserRecoveryTicket,
};

#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Hash)]
Expand All @@ -26,6 +27,7 @@ pub enum NodeType {
UpstreamOAuth2Link,
User,
UserEmail,
UserRecoveryTicket,
}

#[derive(Debug, Error)]
Expand All @@ -50,6 +52,7 @@ impl NodeType {
NodeType::UpstreamOAuth2Link => "upstream_oauth2_link",
NodeType::User => "user",
NodeType::UserEmail => "user_email",
NodeType::UserRecoveryTicket => "user_recovery_ticket",
}
}

Expand All @@ -65,6 +68,7 @@ impl NodeType {
"upstream_oauth2_link" => Some(NodeType::UpstreamOAuth2Link),
"user" => Some(NodeType::User),
"user_email" => Some(NodeType::UserEmail),
"user_recovery_ticket" => Some(NodeType::UserRecoveryTicket),
_ => None,
}
}
Expand Down Expand Up @@ -120,4 +124,5 @@ pub enum Node {
UpstreamOAuth2Link(Box<UpstreamOAuth2Link>),
User(Box<User>),
UserEmail(Box<UserEmail>),
UserRecoveryTicket(Box<UserRecoveryTicket>),
}
100 changes: 99 additions & 1 deletion crates/handlers/src/graphql/model/users.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand Down Expand Up @@ -765,3 +765,101 @@ pub enum UserEmailState {
/// The email address has been confirmed.
Confirmed,
}

/// A recovery ticket
#[derive(Description)]
pub struct UserRecoveryTicket(pub mas_data_model::UserRecoveryTicket);

/// The status of a recovery ticket
#[derive(Enum, Copy, Clone, Eq, PartialEq)]
pub enum UserRecoveryTicketStatus {
/// The ticket is valid
Valid,

/// The ticket has expired
Expired,

/// The ticket has been consumed
Consumed,
}

#[Object(use_type_description)]
impl UserRecoveryTicket {
/// ID of the object.
pub async fn id(&self) -> ID {
NodeType::UserRecoveryTicket.id(self.0.id)
}

/// When the object was created.
pub async fn created_at(&self) -> DateTime<Utc> {
self.0.created_at
}

/// The status of the ticket
pub async fn status(
&self,
context: &Context<'_>,
) -> Result<UserRecoveryTicketStatus, async_graphql::Error> {
let state = context.state();
let clock = state.clock();
let mut repo = state.repository().await?;

// Lookup the session associated with the ticket
let session = repo
.user_recovery()
.lookup_session(self.0.user_recovery_session_id)
.await?
.context("Failed to lookup session")?;
repo.cancel().await?;

if session.consumed_at.is_some() {
return Ok(UserRecoveryTicketStatus::Consumed);
}

if self.0.expires_at < clock.now() {
return Ok(UserRecoveryTicketStatus::Expired);
}

Ok(UserRecoveryTicketStatus::Valid)
}

/// The username associated with this ticket
pub async fn username(&self, ctx: &Context<'_>) -> Result<String, async_graphql::Error> {
// We could expose the UserEmail, then the User, but this is unauthenticated, so
// we don't want to risk leaking too many objects. Instead, we just give the
// username as a property of the UserRecoveryTicket
let state = ctx.state();
let mut repo = state.repository().await?;
let user_email = repo
.user_email()
.lookup(self.0.user_email_id)
.await?
.context("Failed to lookup user email")?;

let user = repo
.user()
.lookup(user_email.user_id)
.await?
.context("Failed to lookup user")?;
repo.cancel().await?;

Ok(user.username)
}

/// The email address associated with this ticket
pub async fn email(&self, ctx: &Context<'_>) -> Result<String, async_graphql::Error> {
// We could expose the UserEmail directly, but this is unauthenticated, so we
// don't want to risk leaking too many objects. Instead, we just give
// the email as a property of the UserRecoveryTicket
let state = ctx.state();
let mut repo = state.repository().await?;
let user_email = repo
.user_email()
.lookup(self.0.user_email_id)
.await?
.context("Failed to lookup user email")?;
repo.cancel().await?;

Ok(user_email.email)
}
}
110 changes: 108 additions & 2 deletions crates/handlers/src/graphql/mutations/user.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2023, 2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -7,10 +7,15 @@
use anyhow::Context as _;
use async_graphql::{Context, Description, Enum, InputObject, Object, ID};
use mas_storage::{
queue::{DeactivateUserJob, ProvisionUserJob, QueueJobRepositoryExt as _},
queue::{
DeactivateUserJob, ProvisionUserJob, QueueJobRepositoryExt as _,
SendAccountRecoveryEmailsJob,
},
user::UserRepository,
};
use tracing::{info, warn};
use ulid::Ulid;
use url::Url;
use zeroize::Zeroizing;

use crate::graphql::{
Expand Down Expand Up @@ -323,6 +328,61 @@ impl SetPasswordPayload {
}
}

/// The input for the `resendRecoveryEmail` mutation.
#[derive(InputObject)]
pub struct ResendRecoveryEmailInput {
/// The recovery ticket to use.
ticket: String,
}

/// The return type for the `resendRecoveryEmail` mutation.
#[derive(Description)]
pub enum ResendRecoveryEmailPayload {
NoSuchRecoveryTicket,
RateLimited,
Sent { recovery_session_id: Ulid },
}

/// The status of the `resendRecoveryEmail` mutation.
#[derive(Enum, Copy, Clone, Eq, PartialEq)]
pub enum ResendRecoveryEmailStatus {
/// The recovery ticket was not found.
NoSuchRecoveryTicket,

/// The rate limit was exceeded.
RateLimited,

/// The recovery email was sent.
Sent,
}

#[Object(use_type_description)]
impl ResendRecoveryEmailPayload {
/// Status of the operation
async fn status(&self) -> ResendRecoveryEmailStatus {
match self {
Self::NoSuchRecoveryTicket => ResendRecoveryEmailStatus::NoSuchRecoveryTicket,
Self::RateLimited => ResendRecoveryEmailStatus::RateLimited,
Self::Sent { .. } => ResendRecoveryEmailStatus::Sent,
}
}

/// URL to continue the recovery process
async fn progress_url(&self, context: &Context<'_>) -> Option<Url> {
let state = context.state();
let url_builder = state.url_builder();
match self {
Self::NoSuchRecoveryTicket | Self::RateLimited => None,
Self::Sent {
recovery_session_id,
} => {
let route = mas_router::AccountRecoveryProgress::new(*recovery_session_id);
Some(url_builder.absolute_url_for(&route))
}
}
}
}

fn valid_username_character(c: char) -> bool {
c.is_ascii_lowercase()
|| c.is_ascii_digit()
Expand Down Expand Up @@ -760,4 +820,50 @@ impl UserMutations {
status: SetPasswordStatus::Allowed,
})
}

/// Resend a user recovery email
pub async fn resend_recovery_email(
&self,
ctx: &Context<'_>,
input: ResendRecoveryEmailInput,
) -> Result<ResendRecoveryEmailPayload, async_graphql::Error> {
let state = ctx.state();
let requester_fingerprint = ctx.requester_fingerprint();
let clock = state.clock();
let mut rng = state.rng();
let limiter = state.limiter();
let mut repo = state.repository().await?;

let Some(recovery_ticket) = repo.user_recovery().find_ticket(&input.ticket).await? else {
return Ok(ResendRecoveryEmailPayload::NoSuchRecoveryTicket);
};

let recovery_session = repo
.user_recovery()
.lookup_session(recovery_ticket.user_recovery_session_id)
.await?
.context("Could not load recovery session")?;

if let Err(e) =
limiter.check_account_recovery(requester_fingerprint, &recovery_session.email)
{
tracing::warn!(error = &e as &dyn std::error::Error);
return Ok(ResendRecoveryEmailPayload::RateLimited);
}

// Schedule a new batch of emails
repo.queue_job()
.schedule_job(
&mut rng,
&clock,
SendAccountRecoveryEmailsJob::new(&recovery_session),
)
.await?;

repo.save().await?;

Ok(ResendRecoveryEmailPayload::Sent {
recovery_session_id: recovery_session.id,
})
}
}
22 changes: 19 additions & 3 deletions crates/handlers/src/graphql/query/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2023, 2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand All @@ -9,7 +9,7 @@ use async_graphql::{Context, MergedObject, Object, ID};
use crate::graphql::{
model::{
Anonymous, BrowserSession, CompatSession, Node, NodeType, OAuth2Client, OAuth2Session,
SiteConfig, User, UserEmail,
SiteConfig, User, UserEmail, UserRecoveryTicket,
},
state::ContextExt,
};
Expand Down Expand Up @@ -182,6 +182,20 @@ impl BaseQuery {
Ok(Some(UserEmail(user_email)))
}

/// Fetch a user recovery ticket.
async fn user_recovery_ticket(
&self,
ctx: &Context<'_>,
ticket: String,
) -> Result<Option<UserRecoveryTicket>, async_graphql::Error> {
let state = ctx.state();
let mut repo = state.repository().await?;
let ticket = repo.user_recovery().find_ticket(&ticket).await?;
repo.cancel().await?;

Ok(ticket.map(UserRecoveryTicket))
}

/// Fetches an object given its ID.
async fn node(&self, ctx: &Context<'_>, id: ID) -> Result<Option<Node>, async_graphql::Error> {
// Special case for the anonymous user
Expand All @@ -199,7 +213,9 @@ impl BaseQuery {

let ret = match node_type {
// TODO
NodeType::Authentication | NodeType::CompatSsoLogin => None,
NodeType::Authentication | NodeType::CompatSsoLogin | NodeType::UserRecoveryTicket => {
None
}

NodeType::UpstreamOAuth2Provider => UpstreamOAuthQuery
.upstream_oauth2_provider(ctx, id)
Expand Down
4 changes: 3 additions & 1 deletion crates/handlers/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2024, 2025 New Vector Ltd.
// Copyright 2021-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
Expand Down Expand Up @@ -118,6 +118,8 @@ where
BoxClock: FromRequestParts<S>,
Encrypter: FromRef<S>,
CookieJar: FromRequestParts<S>,
Limiter: FromRef<S>,
RequesterFingerprint: FromRequestParts<S>,
{
let mut router = Router::new()
.route(
Expand Down
Loading

0 comments on commit ab7be10

Please sign in to comment.