Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polish the password recovery flow and other small design tweaks #3780

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions crates/cli/src/commands/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ impl Options {
site_config.clone(),
password_manager.clone(),
url_builder.clone(),
limiter.clone(),
);

let state = {
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/server.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
22 changes: 18 additions & 4 deletions crates/handlers/src/graphql/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 @@ -53,7 +53,10 @@ use self::{
mutations::Mutation,
query::Query,
};
use crate::{impl_from_error_for_route, passwords::PasswordManager, BoundActivityTracker};
use crate::{
impl_from_error_for_route, passwords::PasswordManager, BoundActivityTracker, Limiter,
RequesterFingerprint,
};

#[cfg(test)]
mod tests;
Expand All @@ -72,6 +75,7 @@ struct GraphQLState {
site_config: SiteConfig,
password_manager: PasswordManager,
url_builder: UrlBuilder,
limiter: Limiter,
}

#[async_trait]
Expand Down Expand Up @@ -104,6 +108,10 @@ impl state::State for GraphQLState {
&self.url_builder
}

fn limiter(&self) -> &Limiter {
&self.limiter
}

fn clock(&self) -> BoxClock {
let clock = SystemClock::default();
Box::new(clock)
Expand All @@ -126,6 +134,7 @@ pub fn schema(
site_config: SiteConfig,
password_manager: PasswordManager,
url_builder: UrlBuilder,
limiter: Limiter,
) -> Schema {
let state = GraphQLState {
pool: pool.clone(),
Expand All @@ -134,6 +143,7 @@ pub fn schema(
site_config,
password_manager,
url_builder,
limiter,
};
let state: BoxState = Box::new(state);

Expand Down Expand Up @@ -303,6 +313,7 @@ pub async fn post(
cookie_jar: CookieJar,
content_type: Option<TypedHeader<ContentType>>,
authorization: Option<TypedHeader<Authorization<Bearer>>>,
requester_fingerprint: RequesterFingerprint,
body: Body,
) -> Result<impl IntoResponse, RouteError> {
let body = body.into_data_stream();
Expand All @@ -329,6 +340,7 @@ pub async fn post(
MultipartOptions::default(),
)
.await?
.data(requester_fingerprint)
.data(requester); // XXX: this should probably return another error response?

let span = span_for_graphql_request(&request);
Expand All @@ -355,6 +367,7 @@ pub async fn get(
activity_tracker: BoundActivityTracker,
cookie_jar: CookieJar,
authorization: Option<TypedHeader<Authorization<Bearer>>>,
requester_fingerprint: RequesterFingerprint,
RawQuery(query): RawQuery,
) -> Result<impl IntoResponse, FancyError> {
let token = authorization
Expand All @@ -371,8 +384,9 @@ pub async fn get(
)
.await?;

let request =
async_graphql::http::parse_query_string(&query.unwrap_or_default())?.data(requester);
let request = async_graphql::http::parse_query_string(&query.unwrap_or_default())?
.data(requester)
.data(requester_fingerprint);

let span = span_for_graphql_request(&request);
let response = schema.execute(request).instrument(span).await;
Expand Down
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)
}
}
Loading
Loading