From 964af7cca120819c7841323e12a284460a2a9ffc Mon Sep 17 00:00:00 2001 From: mcalinghee Date: Thu, 15 May 2025 10:36:34 +0200 Subject: [PATCH 1/2] display email login_hint when login_with_email_allowed is activated --- Cargo.lock | 1 + crates/data-model/Cargo.toml | 2 + .../src/oauth2/authorization_grant.rs | 61 +++++++++++++++++-- crates/handlers/src/views/login.rs | 20 ++++-- 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6dd7c2c7e..f832ca245 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3274,6 +3274,7 @@ dependencies = [ "base64ct", "chrono", "crc", + "lettre", "mas-iana", "mas-jose", "oauth2-types", diff --git a/crates/data-model/Cargo.toml b/crates/data-model/Cargo.toml index 6474d46ae..2a152a296 100644 --- a/crates/data-model/Cargo.toml +++ b/crates/data-model/Cargo.toml @@ -33,3 +33,5 @@ ruma-common.workspace = true mas-iana.workspace = true mas-jose.workspace = true oauth2-types.workspace = true +# Emails +lettre.workspace = true diff --git a/crates/data-model/src/oauth2/authorization_grant.rs b/crates/data-model/src/oauth2/authorization_grant.rs index 383701bcb..b5a83c80a 100644 --- a/crates/data-model/src/oauth2/authorization_grant.rs +++ b/crates/data-model/src/oauth2/authorization_grant.rs @@ -4,6 +4,8 @@ // SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial // Please see LICENSE files in the repository root for full details. +use std::str::FromStr as _; + use chrono::{DateTime, Utc}; use mas_iana::oauth::PkceCodeChallengeMethod; use oauth2_types::{ @@ -142,6 +144,7 @@ impl AuthorizationGrantStage { pub enum LoginHint<'a> { MXID(&'a UserId), + EMAIL(lettre::Address), None, } @@ -173,7 +176,7 @@ impl std::ops::Deref for AuthorizationGrant { impl AuthorizationGrant { #[must_use] - pub fn parse_login_hint(&self, homeserver: &str) -> LoginHint { + pub fn parse_login_hint(&self, homeserver: &str, login_with_email_allowed: bool) -> LoginHint { let Some(login_hint) = &self.login_hint else { return LoginHint::None; }; @@ -197,6 +200,16 @@ impl AuthorizationGrant { LoginHint::MXID(mxid) } + "email" => { + if !login_with_email_allowed { + return LoginHint::None; + } + // Validate the email + let Ok(address) = lettre::Address::from_str(value) else { + return LoginHint::None; + }; + LoginHint::EMAIL(address) + } // Unknown hint type, treat as none _ => LoginHint::None, } @@ -288,7 +301,7 @@ mod tests { ..AuthorizationGrant::sample(now, &mut rng) }; - let hint = grant.parse_login_hint("example.com"); + let hint = grant.parse_login_hint("example.com", false); assert!(matches!(hint, LoginHint::None)); } @@ -306,11 +319,47 @@ mod tests { ..AuthorizationGrant::sample(now, &mut rng) }; - let hint = grant.parse_login_hint("example.com"); + let hint = grant.parse_login_hint("example.com", false); assert!(matches!(hint, LoginHint::MXID(mxid) if mxid.localpart() == "example-user")); } + #[test] + fn valid_login_hint_with_email() { + #[allow(clippy::disallowed_methods)] + let mut rng = thread_rng(); + + #[allow(clippy::disallowed_methods)] + let now = Utc::now(); + + let grant = AuthorizationGrant { + login_hint: Some(String::from("email:example@user")), + ..AuthorizationGrant::sample(now, &mut rng) + }; + + let hint = grant.parse_login_hint("example.com", true); + + assert!(matches!(hint, LoginHint::EMAIL(email) if email.to_string() == "example@user")); + } + + #[test] + fn valid_login_hint_with_email_when_login_with_email_not_allowed() { + #[allow(clippy::disallowed_methods)] + let mut rng = thread_rng(); + + #[allow(clippy::disallowed_methods)] + let now = Utc::now(); + + let grant = AuthorizationGrant { + login_hint: Some(String::from("email:example@user")), + ..AuthorizationGrant::sample(now, &mut rng) + }; + + let hint = grant.parse_login_hint("example.com", false); + + assert!(matches!(hint, LoginHint::None)); + } + #[test] fn invalid_login_hint() { #[allow(clippy::disallowed_methods)] @@ -324,7 +373,7 @@ mod tests { ..AuthorizationGrant::sample(now, &mut rng) }; - let hint = grant.parse_login_hint("example.com"); + let hint = grant.parse_login_hint("example.com", false); assert!(matches!(hint, LoginHint::None)); } @@ -342,7 +391,7 @@ mod tests { ..AuthorizationGrant::sample(now, &mut rng) }; - let hint = grant.parse_login_hint("example.com"); + let hint = grant.parse_login_hint("example.com", false); assert!(matches!(hint, LoginHint::None)); } @@ -360,7 +409,7 @@ mod tests { ..AuthorizationGrant::sample(now, &mut rng) }; - let hint = grant.parse_login_hint("example.com"); + let hint = grant.parse_login_hint("example.com", false); assert!(matches!(hint, LoginHint::None)); } diff --git a/crates/handlers/src/views/login.rs b/crates/handlers/src/views/login.rs index f684f32ad..8657cb8e0 100644 --- a/crates/handlers/src/views/login.rs +++ b/crates/handlers/src/views/login.rs @@ -123,6 +123,7 @@ pub(crate) async fn get( &mut rng, &templates, &homeserver, + &site_config, ) .await } @@ -177,6 +178,7 @@ pub(crate) async fn post( &mut rng, &templates, &homeserver, + &site_config, ) .await; } @@ -187,7 +189,7 @@ pub(crate) async fn post( .unwrap_or(&form.username); // First, lookup the user - let Some(user) = get_user_by_email_or_by_username(site_config, &mut repo, username).await? + let Some(user) = get_user_by_email_or_by_username(&site_config, &mut repo, username).await? else { let form_state = form_state.with_error_on_form(FormError::InvalidCredentials); PASSWORD_LOGIN_COUNTER.add(1, &[KeyValue::new(RESULT, "error")]); @@ -201,6 +203,7 @@ pub(crate) async fn post( &mut rng, &templates, &homeserver, + &site_config, ) .await; }; @@ -220,6 +223,7 @@ pub(crate) async fn post( &mut rng, &templates, &homeserver, + &site_config, ) .await; } @@ -240,6 +244,7 @@ pub(crate) async fn post( &mut rng, &templates, &homeserver, + &site_config, ) .await; }; @@ -283,6 +288,7 @@ pub(crate) async fn post( &mut rng, &templates, &homeserver, + &site_config, ) .await; } @@ -339,7 +345,7 @@ pub(crate) async fn post( } async fn get_user_by_email_or_by_username( - site_config: SiteConfig, + site_config: &SiteConfig, repo: &mut R, username_or_email: &str, ) -> Result, R::Error> { @@ -364,6 +370,7 @@ fn handle_login_hint( mut ctx: LoginContext, next: &PostAuthContext, homeserver: &dyn HomeserverConnection, + site_config: &SiteConfig, ) -> LoginContext { let form_state = ctx.form_state_mut(); @@ -373,8 +380,12 @@ fn handle_login_hint( } if let PostAuthContextInner::ContinueAuthorizationGrant { ref grant } = next.ctx { - let value = match grant.parse_login_hint(homeserver.homeserver()) { + let value = match grant.parse_login_hint( + homeserver.homeserver(), + site_config.login_with_email_allowed, + ) { LoginHint::MXID(mxid) => Some(mxid.localpart().to_owned()), + LoginHint::EMAIL(email) => Some(email.to_string()), LoginHint::None => None, }; form_state.set_value(LoginFormField::Username, value); @@ -393,6 +404,7 @@ async fn render( rng: impl Rng, templates: &Templates, homeserver: &dyn HomeserverConnection, + site_config: &SiteConfig, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(clock, rng); let providers = repo.upstream_oauth_provider().all_enabled().await?; @@ -406,7 +418,7 @@ async fn render( .await .map_err(InternalError::from_anyhow)?; let ctx = if let Some(next) = next { - let ctx = handle_login_hint(ctx, &next, homeserver); + let ctx = handle_login_hint(ctx, &next, homeserver, site_config); ctx.with_post_action(next) } else { ctx From d18fcaac8dee545a97537532713d1b3e1b1b600c Mon Sep 17 00:00:00 2001 From: mcalinghee Date: Thu, 5 Jun 2025 15:40:22 +0200 Subject: [PATCH 2/2] rename Login:EMAIL to Login::Email + remove use of email prefix --- crates/data-model/Cargo.toml | 4 +- .../src/oauth2/authorization_grant.rs | 37 +++++++++++-------- crates/handlers/src/views/login.rs | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/crates/data-model/Cargo.toml b/crates/data-model/Cargo.toml index 2a152a296..ad3a9848b 100644 --- a/crates/data-model/Cargo.toml +++ b/crates/data-model/Cargo.toml @@ -29,9 +29,9 @@ rand.workspace = true regex.workspace = true woothee.workspace = true ruma-common.workspace = true +lettre.workspace = true mas-iana.workspace = true mas-jose.workspace = true oauth2-types.workspace = true -# Emails -lettre.workspace = true + diff --git a/crates/data-model/src/oauth2/authorization_grant.rs b/crates/data-model/src/oauth2/authorization_grant.rs index b5a83c80a..8f8423623 100644 --- a/crates/data-model/src/oauth2/authorization_grant.rs +++ b/crates/data-model/src/oauth2/authorization_grant.rs @@ -144,7 +144,7 @@ impl AuthorizationGrantStage { pub enum LoginHint<'a> { MXID(&'a UserId), - EMAIL(lettre::Address), + Email(lettre::Address), None, } @@ -175,14 +175,31 @@ impl std::ops::Deref for AuthorizationGrant { } impl AuthorizationGrant { + /// Parse a `login_hint` + /// + /// Returns `LoginHint::MXID` for valid mxid 'mxid:@john.doe:example.com' + /// + /// Returns `LoginHint::Email` for valid email 'john.doe@example.com' if + /// email supports is enabled + /// + /// Otherwise returns `LoginHint::None` #[must_use] pub fn parse_login_hint(&self, homeserver: &str, login_with_email_allowed: bool) -> LoginHint { let Some(login_hint) = &self.login_hint else { return LoginHint::None; }; - // Return none if the format is incorrect let Some((prefix, value)) = login_hint.split_once(':') else { + // If email supports for login_hint is enabled + if login_with_email_allowed { + // Validate the email + let Ok(address) = lettre::Address::from_str(login_hint) else { + // Return none if the format is incorrect + return LoginHint::None; + }; + return LoginHint::Email(address); + } + // Unknown hint type, treat as none return LoginHint::None; }; @@ -200,16 +217,6 @@ impl AuthorizationGrant { LoginHint::MXID(mxid) } - "email" => { - if !login_with_email_allowed { - return LoginHint::None; - } - // Validate the email - let Ok(address) = lettre::Address::from_str(value) else { - return LoginHint::None; - }; - LoginHint::EMAIL(address) - } // Unknown hint type, treat as none _ => LoginHint::None, } @@ -333,13 +340,13 @@ mod tests { let now = Utc::now(); let grant = AuthorizationGrant { - login_hint: Some(String::from("email:example@user")), + login_hint: Some(String::from("example@user")), ..AuthorizationGrant::sample(now, &mut rng) }; let hint = grant.parse_login_hint("example.com", true); - assert!(matches!(hint, LoginHint::EMAIL(email) if email.to_string() == "example@user")); + assert!(matches!(hint, LoginHint::Email(email) if email.to_string() == "example@user")); } #[test] @@ -351,7 +358,7 @@ mod tests { let now = Utc::now(); let grant = AuthorizationGrant { - login_hint: Some(String::from("email:example@user")), + login_hint: Some(String::from("example@user")), ..AuthorizationGrant::sample(now, &mut rng) }; diff --git a/crates/handlers/src/views/login.rs b/crates/handlers/src/views/login.rs index 8657cb8e0..9c42b51aa 100644 --- a/crates/handlers/src/views/login.rs +++ b/crates/handlers/src/views/login.rs @@ -385,7 +385,7 @@ fn handle_login_hint( site_config.login_with_email_allowed, ) { LoginHint::MXID(mxid) => Some(mxid.localpart().to_owned()), - LoginHint::EMAIL(email) => Some(email.to_string()), + LoginHint::Email(email) => Some(email.to_string()), LoginHint::None => None, }; form_state.set_value(LoginFormField::Username, value);