diff --git a/auth/server/README.md b/auth/server/README.md index bf7e6f6..9dd3385 100644 --- a/auth/server/README.md +++ b/auth/server/README.md @@ -230,8 +230,8 @@ impl mogh_auth_server::AuthImpl for AppAuthImpl { // = OIDC AUTH = // ============= - fn oidc_config(&self) -> &OidcConfig { - &core_config().oidc + fn oidc_config(&self) -> Option<&OidcConfig> { + Some(&core_config().oidc) } fn find_user_with_oidc_subject( @@ -286,6 +286,26 @@ impl mogh_auth_server::AuthImpl for AppAuthImpl { }) } + fn on_oidc_login( + &self, + user_id: String, + claims: mogh_auth_server::OidcClaims, + ) -> mogh_auth_server::DynFuture> { + Box::pin(async move { + let groups = claims + .claim("groups") + .and_then(|groups| groups.as_array()) + .into_iter() + .flatten() + .filter_map(|group| group.as_str().map(ToString::to_string)) + .collect::>(); + + sync_user_groups_from_oidc(user_id, groups) + .await + .map_err(Into::into) + }) + } + // =============== // = GITHUB AUTH = // =============== @@ -539,4 +559,4 @@ impl mogh_server::session::SessionConfig for MemorySessionConfig { axum::Router::new() .nest("/auth", mogh_auth_server::api::router::()) .layer(mogh_server::session::memory_session_layer(MemorySessionConfig)) -``` \ No newline at end of file +``` diff --git a/auth/server/src/api/oidc.rs b/auth/server/src/api/oidc.rs index b5b2207..aeffa97 100644 --- a/auth/server/src/api/oidc.rs +++ b/auth/server/src/api/oidc.rs @@ -230,6 +230,9 @@ pub async fn oidc_callback( &nonce, ) .await?; + let claims = provider + .get_verified_claims(config, &subject, &token, &nonce) + .await?; let user = auth.find_user_with_oidc_subject(subject.clone()).await?; @@ -237,6 +240,9 @@ pub async fn oidc_callback( let user_id_or_two_factor = match user { // Log in existing user Some(user) => { + auth + .on_oidc_login(user.id().to_string(), claims.clone()) + .await?; get_user_id_or_two_factor(&auth, &session, &user).await? } // Sign up user @@ -250,8 +256,7 @@ pub async fn oidc_callback( ); } - let mut username = - provider.get_username(&subject, &token, &nonce).await; + let mut username = provider.get_username_from_claims(&claims); // Modify username if it already exists if auth @@ -271,6 +276,13 @@ pub async fn oidc_callback( ) .await?; + auth + .on_oidc_signup(user_id.clone(), claims.clone()) + .await?; + auth + .on_oidc_login(user_id.clone(), claims.clone()) + .await?; + info!(user_id, username, "New user registration (OIDC)"); session.insert_authenticated_user_id(&user_id).await?; @@ -308,7 +320,7 @@ async fn link_oidc_callback( .context("OIDC login is not set up") .status_code(StatusCode::BAD_REQUEST)?; - let (subject, _) = provider + let (subject, token) = provider .validate_extract_subject_and_token( config, (state, csrf_token), @@ -317,6 +329,9 @@ async fn link_oidc_callback( &nonce, ) .await?; + let claims = provider + .get_verified_claims(config, &subject, &token, &nonce) + .await?; // Ensure there are no other existing users with this login linked. if let Some(existing_user) = @@ -324,6 +339,9 @@ async fn link_oidc_callback( { if existing_user.id() == user_id { // Link is already complete, this is a no-op + auth + .on_oidc_link(user_id.clone(), claims.clone()) + .await?; return Ok(Redirect::to(auth.post_link_redirect())); } else { return Err( @@ -333,6 +351,9 @@ async fn link_oidc_callback( } auth.link_oidc_login(user_id.clone(), subject).await?; + auth + .on_oidc_link(user_id.clone(), claims.clone()) + .await?; info!(user_id, "OIDC login linked"); diff --git a/auth/server/src/lib.rs b/auth/server/src/lib.rs index 7f57729..026ad89 100644 --- a/auth/server/src/lib.rs +++ b/auth/server/src/lib.rs @@ -29,6 +29,8 @@ use crate::{ }, }; +pub use provider::oidc::OidcClaims; + pub mod request_ip { pub use mogh_request_ip::*; } @@ -347,6 +349,36 @@ pub trait AuthImpl: Send + Sync + 'static { }) } + /// Runs after a verified OIDC login has been mapped to an app user id. + /// This runs for both existing users and newly-created users. If the user + /// has 2FA enabled, this hook runs before the second-factor challenge is + /// completed, after the OIDC identity itself has been validated. + fn on_oidc_login( + &self, + _user_id: String, + _claims: OidcClaims, + ) -> DynFuture> { + Box::pin(async { Ok(()) }) + } + + /// Runs after an OIDC user is created, before the login redirect returns. + fn on_oidc_signup( + &self, + _user_id: String, + _claims: OidcClaims, + ) -> DynFuture> { + Box::pin(async { Ok(()) }) + } + + /// Runs after an OIDC login is linked to an existing app user. + fn on_oidc_link( + &self, + _user_id: String, + _claims: OidcClaims, + ) -> DynFuture> { + Box::pin(async { Ok(()) }) + } + // ============== // = NAMED AUTH = // ============== diff --git a/auth/server/src/provider/oidc.rs b/auth/server/src/provider/oidc.rs index 0fddf30..e1c1c20 100644 --- a/auth/server/src/provider/oidc.rs +++ b/auth/server/src/provider/oidc.rs @@ -12,11 +12,12 @@ use openidconnect::{ EndpointMaybeSet, EndpointNotSet, EndpointSet, IdTokenFields, IssuerUrl, Nonce, OAuth2TokenResponse, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, Scope, StandardErrorResponse, - StandardTokenResponse, TokenResponse as _, + StandardTokenResponse, TokenResponse as _, UserInfoClaims, core::*, reqwest::{self, Url}, }; use serde::{Deserialize, Serialize}; +use serde_json::{Map, Value}; use tracing::{debug, error}; pub use openidconnect::SubjectIdentifier; @@ -30,9 +31,61 @@ pub struct UsernameAdditionalClaims { impl AdditionalClaims for UsernameAdditionalClaims {} +/// Additional OIDC claims captured for verified ID token and userinfo data. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct OidcAdditionalClaims { + /// Some providers use 'username' rather than 'preferred_username'. + pub username: Option, + /// Provider-specific claims, excluding standard OIDC claims. + #[serde(flatten)] + pub custom_claims: Map, +} + +impl AdditionalClaims for OidcAdditionalClaims {} + +/// Verified OIDC claims from the ID token and, when available, userinfo. +/// +/// `id_token_claims` contains the claims from the validated ID token. +/// `userinfo_claims` contains claims returned by the userinfo endpoint only +/// when that response was available and verified for the same subject. +/// `claims` is a merged view where userinfo values override ID token values. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +pub struct OidcClaims { + /// The validated OIDC subject from the ID token. + pub subject: String, + /// Convenience standard claims. Userinfo values take precedence when + /// available; otherwise the ID token value is used. + pub email: Option, + pub preferred_username: Option, + pub name: Option, + /// Merged ID token and userinfo claims. Userinfo values take precedence. + pub claims: Map, + /// Claims from the validated ID token. + pub id_token_claims: Map, + /// Claims from the userinfo endpoint, when available and verified for the + /// same subject. + pub userinfo_claims: Option>, +} + +impl OidcClaims { + /// Read a claim from the merged claims view. Userinfo claims take + /// precedence over ID token claims when both sources contain the key. + pub fn claim(&self, key: &str) -> Option<&Value> { + self.claims.get(key) + } + + pub fn id_token_claim(&self, key: &str) -> Option<&Value> { + self.id_token_claims.get(key) + } + + pub fn userinfo_claim(&self, key: &str) -> Option<&Value> { + self.userinfo_claims.as_ref()?.get(key) + } +} + pub type TokenResponse = StandardTokenResponse< IdTokenFields< - UsernameAdditionalClaims, + OidcAdditionalClaims, EmptyExtraTokenFields, CoreGenderClaim, CoreJweContentEncryptionAlgorithm, @@ -71,7 +124,7 @@ fn reqwest(app_user_agent: &str) -> &'static reqwest::Client { } pub type InnerOidcProvider = Client< - UsernameAdditionalClaims, + OidcAdditionalClaims, CoreAuthDisplay, CoreGenderClaim, CoreJweContentEncryptionAlgorithm, @@ -275,6 +328,229 @@ impl OidcProvider { Ok((claims.subject().clone(), token_response)) } + /// Collect verified claims from the ID token and the userinfo endpoint. + /// + /// This must only be called after the normal authorization-code exchange. + /// ID token claims are verified with the same nonce and audience handling as + /// `validate_extract_subject_and_token`. Userinfo is optional; if the + /// provider has no endpoint or the request fails, only ID token claims are + /// returned. + pub async fn get_verified_claims( + &self, + config: &OidcConfig, + subject: &SubjectIdentifier, + token: &TokenResponse, + nonce: &Nonce, + ) -> anyhow::Result { + let id_token = token + .id_token() + .context("OIDC Server did not return an ID token")?; + + let verifier = self.client.id_token_verifier(); + let additional_audiences = &config.additional_audiences; + let verifier = if additional_audiences.is_empty() { + verifier + } else { + verifier.set_other_audience_verifier_fn(|aud| { + additional_audiences.contains(aud) + }) + }; + + let id_claims = id_token + .claims(&verifier, nonce) + .context("Failed to verify token claims")?; + let id_token_claims = claims_to_map(id_claims)?; + + let userinfo = self.get_userinfo(subject, token).await; + let userinfo_claims = userinfo + .as_ref() + .map(claims_to_map) + .transpose()?; + + Ok(OidcClaims::new( + id_claims.subject().to_string(), + userinfo + .as_ref() + .and_then(|claims| { + claims.email().map(|email| email.as_str().to_string()) + }) + .or_else(|| { + id_claims.email().map(|email| email.as_str().to_string()) + }), + userinfo + .as_ref() + .and_then(|claims| { + claims + .preferred_username() + .map(|username| username.as_str().to_string()) + }) + .or_else(|| { + id_claims + .preferred_username() + .map(|username| username.as_str().to_string()) + }), + userinfo + .as_ref() + .and_then(|claims| { + claims + .name() + .and_then(|name| name.get(None)) + .map(|name| name.as_str().to_string()) + }) + .or_else(|| { + id_claims + .name() + .and_then(|name| name.get(None)) + .map(|name| name.as_str().to_string()) + }), + id_token_claims, + userinfo_claims, + )) + } + + async fn get_userinfo( + &self, + subject: &SubjectIdentifier, + token: &TokenResponse, + ) -> Option> + { + self + .client + .user_info(token.access_token().clone(), Some(subject.clone())) + .ok()? + .request_async::( + reqwest(self.app_user_agent), + ) + .await + .inspect(|user_info| debug!("OIDC USER INFO: {user_info:?}")) + .ok() + } + + pub fn get_username_from_claims(&self, claims: &OidcClaims) -> String { + if self.use_full_email { + return self.get_username_from_claims_prioritize_email(claims); + } + + // Priority 1: preferred_username from id_token. + if let Some(username) = + string_claim(&claims.id_token_claims, "preferred_username") + { + return username; + } + + // Priority 2: preferred_username from userinfo. + if let Some(username) = claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "preferred_username")) + { + return username; + } + + // Priority 3: username additional claim from id claims, then userinfo. + if let Some(username) = string_claim(&claims.id_token_claims, "username") + .or_else(|| { + claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "username")) + }) + { + return username; + } + + // Priority 4: name from id claims, then userinfo. + if let Some(username) = string_claim(&claims.id_token_claims, "name") + .or_else(|| { + claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "name")) + }) + { + return username; + } + + // Priority 5: username part of email from id claims, then userinfo. + if let Some(email) = string_claim(&claims.id_token_claims, "email") + .or_else(|| { + claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "email")) + }) + { + let username = email + .split_once('@') + .map(|(username, _)| username) + .unwrap_or(email.as_str()) + .to_string(); + return username; + } + + // Priority 6 (fallback): use the subject if no others available. + claims.subject.clone() + } + + fn get_username_from_claims_prioritize_email( + &self, + claims: &OidcClaims, + ) -> String { + // Priority 1: email from id_token. + if let Some(email) = string_claim(&claims.id_token_claims, "email") { + return email; + } + + // Priority 2: email from userinfo. + if let Some(email) = claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "email")) + { + return email; + } + + // Priority 3: preferred_username from id claims, then userinfo. + if let Some(username) = + string_claim(&claims.id_token_claims, "preferred_username") + .or_else(|| { + claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "preferred_username")) + }) + { + return username; + } + + // Priority 4: username additional claim from id claims, then userinfo. + if let Some(username) = string_claim(&claims.id_token_claims, "username") + .or_else(|| { + claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "username")) + }) + { + return username; + } + + // Priority 5: name from id claims, then userinfo. + if let Some(username) = string_claim(&claims.id_token_claims, "name") + .or_else(|| { + claims + .userinfo_claims + .as_ref() + .and_then(|claims| string_claim(claims, "name")) + }) + { + return username; + } + + // Priority 6 (fallback): use the subject if no others available. + claims.subject.clone() + } + pub async fn get_username( &self, subject: &SubjectIdentifier, @@ -461,3 +737,83 @@ impl OidcProvider { subject.to_string() } } + +impl OidcClaims { + fn new( + subject: String, + email: Option, + preferred_username: Option, + name: Option, + id_token_claims: Map, + userinfo_claims: Option>, + ) -> Self { + let mut claims = id_token_claims.clone(); + if let Some(userinfo_claims) = userinfo_claims.as_ref() { + claims.extend(userinfo_claims.clone()); + } + + Self { + subject, + email, + preferred_username, + name, + claims, + id_token_claims, + userinfo_claims, + } + } +} + +fn claims_to_map( + claims: &T, +) -> anyhow::Result> { + match serde_json::to_value(claims)? { + Value::Object(map) => Ok(map), + _ => Ok(Map::new()), + } +} + +fn string_claim(claims: &Map, key: &str) -> Option { + claims.get(key)?.as_str().map(ToString::to_string) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn merged_claim_lookup_prefers_userinfo_claims() { + let mut id_token_claims = Map::new(); + id_token_claims + .insert("groups".to_string(), json!(["id-token-group"])); + id_token_claims.insert("roles".to_string(), json!(["admin"])); + + let mut userinfo_claims = Map::new(); + userinfo_claims + .insert("groups".to_string(), json!(["userinfo-group"])); + + let claims = OidcClaims::new( + "subject-1".to_string(), + Some("user@example.com".to_string()), + Some("user".to_string()), + Some("User Example".to_string()), + id_token_claims, + Some(userinfo_claims), + ); + + assert_eq!( + claims.claim("groups"), + Some(&json!(["userinfo-group"])) + ); + assert_eq!(claims.claim("roles"), Some(&json!(["admin"]))); + assert_eq!( + claims.id_token_claim("groups"), + Some(&json!(["id-token-group"])) + ); + assert_eq!( + claims.userinfo_claim("groups"), + Some(&json!(["userinfo-group"])) + ); + } +}