From b8c7a2dae78008464d2b0528b280e06c5111a1d4 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 12 Mar 2025 11:50:44 +0100 Subject: [PATCH 01/10] Move the `verify_password_if_needed` helper up one level --- crates/handlers/src/graphql/mutations/mod.rs | 58 ++++++++++++++++ .../src/graphql/mutations/user_email.rs | 66 ++----------------- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/crates/handlers/src/graphql/mutations/mod.rs b/crates/handlers/src/graphql/mutations/mod.rs index dbc56a518..66bb5766c 100644 --- a/crates/handlers/src/graphql/mutations/mod.rs +++ b/crates/handlers/src/graphql/mutations/mod.rs @@ -11,7 +11,14 @@ mod oauth2_session; mod user; mod user_email; +use anyhow::Context as _; use async_graphql::MergedObject; +use mas_data_model::SiteConfig; +use mas_storage::BoxRepository; +use zeroize::Zeroizing; + +use super::Requester; +use crate::passwords::PasswordManager; /// The mutations root of the GraphQL interface. #[derive(Default, MergedObject)] @@ -30,3 +37,54 @@ impl Mutation { Self::default() } } + +/// Check the password if neeed +/// +/// Returns true if password verification is not needed, or if the password is +/// correct. Returns false if the password is incorrect or missing. +async fn verify_password_if_needed( + requester: &Requester, + config: &SiteConfig, + password_manager: &PasswordManager, + password: Option, + user: &mas_data_model::User, + repo: &mut BoxRepository, +) -> Result { + // If the requester is admin, they don't need to provide a password + if requester.is_admin() { + return Ok(true); + } + + // If password login is disabled, assume we don't want the user to reauth + if !config.password_login_enabled { + return Ok(true); + } + + // Else we need to check if the user has a password + let Some(user_password) = repo + .user_password() + .active(user) + .await + .context("Failed to load user password")? + else { + // User has no password, so we don't need to verify the password + return Ok(true); + }; + + let Some(password) = password else { + // There is a password on the user, but not provided in the input + return Ok(false); + }; + + let password = Zeroizing::new(password.into_bytes()); + + let res = password_manager + .verify( + user_password.version, + password, + user_password.hashed_password, + ) + .await; + + Ok(res.is_ok()) +} diff --git a/crates/handlers/src/graphql/mutations/user_email.rs b/crates/handlers/src/graphql/mutations/user_email.rs index 958b18be7..6f24f1ed4 100644 --- a/crates/handlers/src/graphql/mutations/user_email.rs +++ b/crates/handlers/src/graphql/mutations/user_email.rs @@ -6,75 +6,19 @@ use anyhow::Context as _; use async_graphql::{Context, Description, Enum, ID, InputObject, Object}; -use mas_data_model::SiteConfig; use mas_i18n::DataLocale; use mas_storage::{ - BoxRepository, RepositoryAccess, + RepositoryAccess, queue::{ProvisionUserJob, QueueJobRepositoryExt as _, SendEmailAuthenticationCodeJob}, user::{UserEmailFilter, UserEmailRepository, UserRepository}, }; -use zeroize::Zeroizing; -use crate::{ - graphql::{ - Requester, - model::{NodeType, User, UserEmail, UserEmailAuthentication}, - state::ContextExt, - }, - passwords::PasswordManager, +use super::verify_password_if_needed; +use crate::graphql::{ + model::{NodeType, User, UserEmail, UserEmailAuthentication}, + state::ContextExt, }; -/// Check the password if neeed -/// -/// Returns true if password verification is not needed, or if the password is -/// correct. Returns false if the password is incorrect or missing. -async fn verify_password_if_needed( - requester: &Requester, - config: &SiteConfig, - password_manager: &PasswordManager, - password: Option, - user: &mas_data_model::User, - repo: &mut BoxRepository, -) -> Result { - // If the requester is admin, they don't need to provide a password - if requester.is_admin() { - return Ok(true); - } - - // If password login is disabled, assume we don't want the user to reauth - if !config.password_login_enabled { - return Ok(true); - } - - // Else we need to check if the user has a password - let Some(user_password) = repo - .user_password() - .active(user) - .await - .context("Failed to load user password")? - else { - // User has no password, so we don't need to verify the password - return Ok(true); - }; - - let Some(password) = password else { - // There is a password on the user, but not provided in the input - return Ok(false); - }; - - let password = Zeroizing::new(password.into_bytes()); - - let res = password_manager - .verify( - user_password.version, - password, - user_password.hashed_password, - ) - .await; - - Ok(res.is_ok()) -} - #[derive(Default)] pub struct UserEmailMutations { _private: (), From 19f1091e5641cc255db401ac57d850df279f18c2 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 12 Mar 2025 12:11:34 +0100 Subject: [PATCH 02/10] Config option to allow account self-deactivation --- crates/cli/src/util.rs | 1 + crates/config/src/sections/account.rs | 7 +++++++ crates/data-model/src/site_config.rs | 3 +++ crates/handlers/src/graphql/model/site_config.rs | 4 ++++ crates/handlers/src/test_utils.rs | 1 + docs/config.schema.json | 4 ++++ frontend/schema.graphql | 4 ++++ frontend/src/gql/graphql.ts | 2 ++ 8 files changed, 26 insertions(+) diff --git a/crates/cli/src/util.rs b/crates/cli/src/util.rs index 27c23eeb5..e8b037e45 100644 --- a/crates/cli/src/util.rs +++ b/crates/cli/src/util.rs @@ -207,6 +207,7 @@ pub fn site_config_from_config( && account_config.password_change_allowed, account_recovery_allowed: password_config.enabled() && account_config.password_recovery_enabled, + account_deactivation_allowed: account_config.account_deactivation_allowed, captcha, minimum_password_complexity: password_config.minimum_complexity(), session_expiration, diff --git a/crates/config/src/sections/account.rs b/crates/config/src/sections/account.rs index 7be79e357..d621dbf4d 100644 --- a/crates/config/src/sections/account.rs +++ b/crates/config/src/sections/account.rs @@ -61,6 +61,11 @@ pub struct AccountConfig { /// This has no effect if password login is disabled. #[serde(default = "default_false", skip_serializing_if = "is_default_false")] pub password_recovery_enabled: bool, + + /// Whether users are allowed to delete their own account. Defaults to + /// `false`. + #[serde(default = "default_false", skip_serializing_if = "is_default_false")] + pub account_deactivation_allowed: bool, } impl Default for AccountConfig { @@ -71,6 +76,7 @@ impl Default for AccountConfig { password_registration_enabled: default_false(), password_change_allowed: default_true(), password_recovery_enabled: default_false(), + account_deactivation_allowed: default_false(), } } } @@ -83,6 +89,7 @@ impl AccountConfig { && is_default_true(&self.displayname_change_allowed) && is_default_true(&self.password_change_allowed) && is_default_false(&self.password_recovery_enabled) + && is_default_false(&self.account_deactivation_allowed) } } diff --git a/crates/data-model/src/site_config.rs b/crates/data-model/src/site_config.rs index 0e09f8a31..4688c0f11 100644 --- a/crates/data-model/src/site_config.rs +++ b/crates/data-model/src/site_config.rs @@ -76,6 +76,9 @@ pub struct SiteConfig { /// Whether users can recover their account via email. pub account_recovery_allowed: bool, + /// Whether users can delete their own account. + pub account_deactivation_allowed: bool, + /// Captcha configuration pub captcha: Option, diff --git a/crates/handlers/src/graphql/model/site_config.rs b/crates/handlers/src/graphql/model/site_config.rs index dc2cae188..598c0aabc 100644 --- a/crates/handlers/src/graphql/model/site_config.rs +++ b/crates/handlers/src/graphql/model/site_config.rs @@ -46,6 +46,9 @@ pub struct SiteConfig { /// Whether passwords are enabled and users can register using a password. password_registration_enabled: bool, + /// Whether users can delete their own account. + account_deactivation_allowed: bool, + /// Minimum password complexity, from 0 to 4, in terms of a zxcvbn score. /// The exact scorer (including dictionaries and other data tables) /// in use is . @@ -93,6 +96,7 @@ impl SiteConfig { password_login_enabled: data_model.password_login_enabled, password_change_allowed: data_model.password_change_allowed, password_registration_enabled: data_model.password_registration_enabled, + account_deactivation_allowed: data_model.account_deactivation_allowed, minimum_password_complexity: data_model.minimum_password_complexity, } } diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index 60da629e2..1cb6f29cd 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -137,6 +137,7 @@ pub fn test_site_config() -> SiteConfig { displayname_change_allowed: true, password_change_allowed: true, account_recovery_allowed: true, + account_deactivation_allowed: true, captcha: None, minimum_password_complexity: 1, session_expiration: None, diff --git a/docs/config.schema.json b/docs/config.schema.json index a998f08fd..93bb96b91 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -2463,6 +2463,10 @@ "password_recovery_enabled": { "description": "Whether email-based password recovery is enabled. Defaults to `false`.\n\nThis has no effect if password login is disabled.", "type": "boolean" + }, + "account_deactivation_allowed": { + "description": "Whether users are allowed to delete their own account. Defaults to `false`.", + "type": "boolean" } } }, diff --git a/frontend/schema.graphql b/frontend/schema.graphql index eeb9b44e4..14775ba2d 100644 --- a/frontend/schema.graphql +++ b/frontend/schema.graphql @@ -1599,6 +1599,10 @@ type SiteConfig implements Node { """ passwordRegistrationEnabled: Boolean! """ + Whether users can delete their own account. + """ + accountDeactivationAllowed: Boolean! + """ Minimum password complexity, from 0 to 4, in terms of a zxcvbn score. The exact scorer (including dictionaries and other data tables) in use is . diff --git a/frontend/src/gql/graphql.ts b/frontend/src/gql/graphql.ts index 5910ff98e..c76a8d3c8 100644 --- a/frontend/src/gql/graphql.ts +++ b/frontend/src/gql/graphql.ts @@ -1161,6 +1161,8 @@ export type SetPrimaryEmailStatus = export type SiteConfig = Node & { __typename?: 'SiteConfig'; + /** Whether users can delete their own account. */ + accountDeactivationAllowed: Scalars['Boolean']['output']; /** The configuration of CAPTCHA provider. */ captchaConfig?: Maybe; /** Whether users can change their display name. */ From 1f03d6da0348d497c0fa942dda8e641d22cfe947 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 12 Mar 2025 11:46:25 +0100 Subject: [PATCH 03/10] GraphQL mutation to deactivate a user --- crates/handlers/src/graphql/mutations/user.rs | 109 ++++++++++++++++++ frontend/schema.graphql | 46 ++++++++ frontend/src/gql/graphql.ts | 36 ++++++ 3 files changed, 191 insertions(+) diff --git a/crates/handlers/src/graphql/mutations/user.rs b/crates/handlers/src/graphql/mutations/user.rs index bb50d1b04..b5a0902e9 100644 --- a/crates/handlers/src/graphql/mutations/user.rs +++ b/crates/handlers/src/graphql/mutations/user.rs @@ -18,6 +18,7 @@ use ulid::Ulid; use url::Url; use zeroize::Zeroizing; +use super::verify_password_if_needed; use crate::graphql::{ UserId, model::{NodeType, User}, @@ -383,6 +384,54 @@ impl ResendRecoveryEmailPayload { } } +/// The input for the `deactivateUser` mutation. +#[derive(InputObject)] +pub struct DeactivateUserInput { + /// Whether to ask the homeserver to GDPR-erase the user + hs_erase: bool, + + /// The password of the user to deactivate. + password: Option, +} + +/// The payload for the `deactivateUser` mutation. +#[derive(Description)] +pub enum DeactivateUserPayload { + /// The user was deactivated. + Deactivated(mas_data_model::User), + + /// The password was wrong or missing. + IncorrectPassword, +} + +/// The status of the `deactivateUser` mutation. +#[derive(Enum, Copy, Clone, Eq, PartialEq)] +pub enum DeactivateUserStatus { + /// The user was deactivated. + Deactivated, + + /// The password was wrong. + IncorrectPassword, +} + +#[Object(use_type_description)] +impl DeactivateUserPayload { + /// Status of the operation + async fn status(&self) -> DeactivateUserStatus { + match self { + Self::Deactivated(_) => DeactivateUserStatus::Deactivated, + Self::IncorrectPassword => DeactivateUserStatus::IncorrectPassword, + } + } + + async fn user(&self) -> Option { + match self { + Self::Deactivated(user) => Some(User(user.clone())), + Self::IncorrectPassword => None, + } + } +} + fn valid_username_character(c: char) -> bool { c.is_ascii_lowercase() || c.is_ascii_digit() @@ -868,4 +917,64 @@ impl UserMutations { recovery_session_id: recovery_session.id, }) } + + /// Deactivate the current user account + /// + /// If the user has a password, it *must* be supplied in the `password` + /// field. + async fn deactivate_user( + &self, + ctx: &Context<'_>, + input: DeactivateUserInput, + ) -> Result { + let state = ctx.state(); + let mut rng = state.rng(); + let clock = state.clock(); + let requester = ctx.requester(); + let site_config = state.site_config(); + + // Only allow calling this if the requester is a browser session + let Some(browser_session) = requester.browser_session() else { + return Err(async_graphql::Error::new("Unauthorized")); + }; + + if !site_config.account_deactivation_allowed { + return Err(async_graphql::Error::new( + "Account deactivation is not allowed on this server", + )); + } + + let mut repo = state.repository().await?; + if !verify_password_if_needed( + requester, + site_config, + &state.password_manager(), + input.password, + &browser_session.user, + &mut repo, + ) + .await? + { + return Ok(DeactivateUserPayload::IncorrectPassword); + } + + // Deactivate the user right away + let user = repo + .user() + .deactivate(&state.clock(), browser_session.user.clone()) + .await?; + + // and then schedule a job to deactivate it fully + repo.queue_job() + .schedule_job( + &mut rng, + &clock, + DeactivateUserJob::new(&user, input.hs_erase), + ) + .await?; + + repo.save().await?; + + Ok(DeactivateUserPayload::Deactivated(user)) + } } diff --git a/frontend/schema.graphql b/frontend/schema.graphql index 14775ba2d..05a330d14 100644 --- a/frontend/schema.graphql +++ b/frontend/schema.graphql @@ -601,6 +601,45 @@ The input/output is a string in RFC3339 format. """ scalar DateTime +""" +The input for the `deactivateUser` mutation. +""" +input DeactivateUserInput { + """ + Whether to ask the homeserver to GDPR-erase the user + """ + hsErase: Boolean! + """ + The password of the user to deactivate. + """ + password: String +} + +""" +The payload for the `deactivateUser` mutation. +""" +type DeactivateUserPayload { + """ + Status of the operation + """ + status: DeactivateUserStatus! + user: User +} + +""" +The status of the `deactivateUser` mutation. +""" +enum DeactivateUserStatus { + """ + The user was deactivated. + """ + DEACTIVATED + """ + The password was wrong. + """ + INCORRECT_PASSWORD +} + """ The type of a user agent """ @@ -876,6 +915,13 @@ type Mutation { input: ResendRecoveryEmailInput! ): ResendRecoveryEmailPayload! """ + Deactivate the current user account + + If the user has a password, it *must* be supplied in the `password` + field. + """ + deactivateUser(input: DeactivateUserInput!): DeactivateUserPayload! + """ Create a new arbitrary OAuth 2.0 Session. Only available for administrators. diff --git a/frontend/src/gql/graphql.ts b/frontend/src/gql/graphql.ts index c76a8d3c8..882bba82b 100644 --- a/frontend/src/gql/graphql.ts +++ b/frontend/src/gql/graphql.ts @@ -389,6 +389,29 @@ export type DateFilter = { before?: InputMaybe; }; +/** The input for the `deactivateUser` mutation. */ +export type DeactivateUserInput = { + /** Whether to ask the homeserver to GDPR-erase the user */ + hsErase: Scalars['Boolean']['input']; + /** The password of the user to deactivate. */ + password?: InputMaybe; +}; + +/** The payload for the `deactivateUser` mutation. */ +export type DeactivateUserPayload = { + __typename?: 'DeactivateUserPayload'; + /** Status of the operation */ + status: DeactivateUserStatus; + user?: Maybe; +}; + +/** The status of the `deactivateUser` mutation. */ +export type DeactivateUserStatus = + /** The user was deactivated. */ + | 'DEACTIVATED' + /** The password was wrong. */ + | 'INCORRECT_PASSWORD'; + /** The type of a user agent */ export type DeviceType = /** A mobile phone. Can also sometimes be a tablet. */ @@ -519,6 +542,13 @@ export type Mutation = { * Only available for administrators. */ createOauth2Session: CreateOAuth2SessionPayload; + /** + * Deactivate the current user account + * + * If the user has a password, it *must* be supplied in the `password` + * field. + */ + deactivateUser: DeactivateUserPayload; endBrowserSession: EndBrowserSessionPayload; endCompatSession: EndCompatSessionPayload; endOauth2Session: EndOAuth2SessionPayload; @@ -596,6 +626,12 @@ export type MutationCreateOauth2SessionArgs = { }; +/** The mutations root of the GraphQL interface. */ +export type MutationDeactivateUserArgs = { + input: DeactivateUserInput; +}; + + /** The mutations root of the GraphQL interface. */ export type MutationEndBrowserSessionArgs = { input: EndBrowserSessionInput; From 14643417c39df2251bbe97f1157c1461bb5bd25b Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 12 Mar 2025 14:22:38 +0100 Subject: [PATCH 04/10] Use our own implementation of the Separator component The one in compound-web is weirdly implemented and doesn't use the right semantic tokens --- .../OAuth2ClientDetail.test.tsx.snap | 11 ++-- .../components/Separator/Separator.module.css | 13 ++++ .../src/components/Separator/Separator.tsx | 32 +++++++++ frontend/src/components/Separator/index.tsx | 6 ++ .../components/SessionDetail/SessionInfo.tsx | 12 ++-- .../CompatSessionDetail.test.tsx.snap | 66 +++++++++---------- .../OAuth2SessionDetail.test.tsx.snap | 44 ++++++------- frontend/src/routes/_account.index.lazy.tsx | 5 +- .../routes/_account.sessions.index.lazy.tsx | 5 +- .../src/routes/password.change.index.lazy.tsx | 3 +- .../account/__snapshots__/index.test.tsx.snap | 17 ++--- 11 files changed, 128 insertions(+), 86 deletions(-) create mode 100644 frontend/src/components/Separator/Separator.module.css create mode 100644 frontend/src/components/Separator/Separator.tsx create mode 100644 frontend/src/components/Separator/index.tsx diff --git a/frontend/src/components/Client/__snapshots__/OAuth2ClientDetail.test.tsx.snap b/frontend/src/components/Client/__snapshots__/OAuth2ClientDetail.test.tsx.snap index 1acf56602..2558267f6 100644 --- a/frontend/src/components/Client/__snapshots__/OAuth2ClientDetail.test.tsx.snap +++ b/frontend/src/components/Client/__snapshots__/OAuth2ClientDetail.test.tsx.snap @@ -21,13 +21,12 @@ exports[` > renders client details 1`] = ` class="_typography_6v6n8_153 _font-heading-sm-semibold_6v6n8_93" > Client info -