From 6ded39797796bc5a528e7164652cfda1e2bf94a6 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 7 Nov 2023 19:31:29 +0100 Subject: [PATCH] Use minijinja templates to map OIDC claims to user attributes --- Cargo.lock | 1 + crates/cli/src/commands/config.rs | 67 ++++----- crates/config/src/sections/upstream_oauth2.rs | 64 +++++++-- crates/data-model/src/lib.rs | 2 +- crates/data-model/src/upstream_oauth2/mod.rs | 3 +- .../src/upstream_oauth2/provider.rs | 12 ++ crates/handlers/Cargo.toml | 1 + .../handlers/src/upstream_oauth2/callback.rs | 31 ++++- crates/handlers/src/upstream_oauth2/link.rs | 127 ++++++++++++------ docs/config.schema.json | 108 +++++++++++---- 10 files changed, 298 insertions(+), 118 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d49de9f6..c88d45de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2972,6 +2972,7 @@ dependencies = [ "mas-storage-pg", "mas-templates", "mime", + "minijinja", "oauth2-types", "opentelemetry", "opentelemetry-semantic-conventions", diff --git a/crates/cli/src/commands/config.rs b/crates/cli/src/commands/config.rs index 6b071a12..f818f770 100644 --- a/crates/cli/src/commands/config.rs +++ b/crates/cli/src/commands/config.rs @@ -45,51 +45,36 @@ fn map_import_action( } } -fn map_import_preference( - config: &mas_config::UpstreamOAuth2ImportPreference, -) -> mas_data_model::UpstreamOAuthProviderImportPreference { - mas_data_model::UpstreamOAuthProviderImportPreference { - action: map_import_action(&config.action), - } -} - fn map_claims_imports( config: &mas_config::UpstreamOAuth2ClaimsImports, ) -> mas_data_model::UpstreamOAuthProviderClaimsImports { mas_data_model::UpstreamOAuthProviderClaimsImports { - localpart: config - .localpart - .as_ref() - .map(map_import_preference) - .unwrap_or_default(), - displayname: config - .displayname - .as_ref() - .map(map_import_preference) - .unwrap_or_default(), - email: config - .email - .as_ref() - .map(|c| mas_data_model::UpstreamOAuthProviderImportPreference { - action: map_import_action(&c.action), - }) - .unwrap_or_default(), - // XXX: this is a bit ugly - verify_email: config - .email - .as_ref() - .map(|c| match c.set_email_verification { - mas_config::UpstreamOAuth2SetEmailVerification::Always => { - mas_data_model::UpsreamOAuthProviderSetEmailVerification::Always - } - mas_config::UpstreamOAuth2SetEmailVerification::Never => { - mas_data_model::UpsreamOAuthProviderSetEmailVerification::Never - } - mas_config::UpstreamOAuth2SetEmailVerification::Import => { - mas_data_model::UpsreamOAuthProviderSetEmailVerification::Import - } - }) - .unwrap_or_default(), + subject: mas_data_model::UpstreamOAuthProviderSubjectPreference { + template: config.subject.template.clone(), + }, + localpart: mas_data_model::UpstreamOAuthProviderImportPreference { + action: map_import_action(&config.localpart.action), + template: config.localpart.template.clone(), + }, + displayname: mas_data_model::UpstreamOAuthProviderImportPreference { + action: map_import_action(&config.displayname.action), + template: config.displayname.template.clone(), + }, + email: mas_data_model::UpstreamOAuthProviderImportPreference { + action: map_import_action(&config.email.action), + template: config.email.template.clone(), + }, + verify_email: match config.email.set_email_verification { + mas_config::UpstreamOAuth2SetEmailVerification::Always => { + mas_data_model::UpsreamOAuthProviderSetEmailVerification::Always + } + mas_config::UpstreamOAuth2SetEmailVerification::Never => { + mas_data_model::UpsreamOAuthProviderSetEmailVerification::Never + } + mas_config::UpstreamOAuth2SetEmailVerification::Import => { + mas_data_model::UpsreamOAuthProviderSetEmailVerification::Import + } + }, } } diff --git a/crates/config/src/sections/upstream_oauth2.rs b/crates/config/src/sections/upstream_oauth2.rs index d0593c5b..1d2d3e24 100644 --- a/crates/config/src/sections/upstream_oauth2.rs +++ b/crates/config/src/sections/upstream_oauth2.rs @@ -96,10 +96,10 @@ pub enum ImportAction { Require, } -/// What should be done with a claim +/// What should be done with a attribute #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] pub struct ImportPreference { - /// How to handle the claim + /// How to handle the attribute #[serde(default)] pub action: ImportAction, } @@ -120,13 +120,57 @@ pub enum SetEmailVerification { Import, } -/// What should be done with the email claim +/// What should be done for the subject attribute +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] +pub struct SubjectImportPreference { + /// The Jinja2 template to use for the subject attribute + /// + /// If not provided, the default template is `{{ user.sub }}` + #[serde(default)] + pub template: Option, +} + +/// What should be done for the localpart attribute +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] +pub struct LocalpartImportPreference { + /// How to handle the attribute + #[serde(default)] + pub action: ImportAction, + + /// The Jinja2 template to use for the localpart attribute + /// + /// If not provided, the default template is `{{ user.preferred_username }}` + #[serde(default)] + pub template: Option, +} + +/// What should be done for the displayname attribute +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] +pub struct DisplaynameImportPreference { + /// How to handle the attribute + #[serde(default)] + pub action: ImportAction, + + /// The Jinja2 template to use for the displayname attribute + /// + /// If not provided, the default template is `{{ user.name }}` + #[serde(default)] + pub template: Option, +} + +/// What should be done with the email attribute #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] pub struct EmailImportPreference { /// How to handle the claim #[serde(default)] pub action: ImportAction, + /// The Jinja2 template to use for the email address attribute + /// + /// If not provided, the default template is `{{ user.email }}` + #[serde(default)] + pub template: Option, + /// Should the email address be marked as verified #[serde(default)] pub set_email_verification: SetEmailVerification, @@ -135,18 +179,22 @@ pub struct EmailImportPreference { /// How claims should be imported #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema)] pub struct ClaimsImports { - /// Import the localpart of the MXID based on the `preferred_username` claim + /// How to determine the subject of the user #[serde(default)] - pub localpart: Option, + pub subject: SubjectImportPreference, - /// Import the displayname of the user based on the `name` claim + /// Import the localpart of the MXID #[serde(default)] - pub displayname: Option, + pub localpart: LocalpartImportPreference, + + /// Import the displayname of the user. + #[serde(default)] + pub displayname: DisplaynameImportPreference, /// Import the email address of the user based on the `email` and /// `email_verified` claims #[serde(default)] - pub email: Option, + pub email: EmailImportPreference, } #[skip_serializing_none] diff --git a/crates/data-model/src/lib.rs b/crates/data-model/src/lib.rs index 0fd81733..325d1eed 100644 --- a/crates/data-model/src/lib.rs +++ b/crates/data-model/src/lib.rs @@ -49,7 +49,7 @@ pub use self::{ UpsreamOAuthProviderSetEmailVerification, UpstreamOAuthAuthorizationSession, UpstreamOAuthAuthorizationSessionState, UpstreamOAuthLink, UpstreamOAuthProvider, UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderImportAction, - UpstreamOAuthProviderImportPreference, + UpstreamOAuthProviderImportPreference, UpstreamOAuthProviderSubjectPreference, }, users::{ Authentication, AuthenticationMethod, BrowserSession, Password, User, UserEmail, diff --git a/crates/data-model/src/upstream_oauth2/mod.rs b/crates/data-model/src/upstream_oauth2/mod.rs index 0bd6779d..1e5d9f9b 100644 --- a/crates/data-model/src/upstream_oauth2/mod.rs +++ b/crates/data-model/src/upstream_oauth2/mod.rs @@ -22,7 +22,8 @@ pub use self::{ ClaimsImports as UpstreamOAuthProviderClaimsImports, ImportAction as UpstreamOAuthProviderImportAction, ImportPreference as UpstreamOAuthProviderImportPreference, - SetEmailVerification as UpsreamOAuthProviderSetEmailVerification, UpstreamOAuthProvider, + SetEmailVerification as UpsreamOAuthProviderSetEmailVerification, + SubjectPreference as UpstreamOAuthProviderSubjectPreference, UpstreamOAuthProvider, }, session::{UpstreamOAuthAuthorizationSession, UpstreamOAuthAuthorizationSessionState}, }; diff --git a/crates/data-model/src/upstream_oauth2/provider.rs b/crates/data-model/src/upstream_oauth2/provider.rs index 6aba113e..3e00ee7e 100644 --- a/crates/data-model/src/upstream_oauth2/provider.rs +++ b/crates/data-model/src/upstream_oauth2/provider.rs @@ -59,6 +59,9 @@ impl SetEmailVerification { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] pub struct ClaimsImports { + #[serde(default)] + pub subject: SubjectPreference, + #[serde(default)] pub localpart: ImportPreference, @@ -72,10 +75,19 @@ pub struct ClaimsImports { pub verify_email: SetEmailVerification, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct SubjectPreference { + #[serde(default)] + pub template: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] pub struct ImportPreference { #[serde(default)] pub action: ImportAction, + + #[serde(default)] + pub template: Option, } impl std::ops::Deref for ImportPreference { diff --git a/crates/handlers/Cargo.toml b/crates/handlers/Cargo.toml index 5d8b549f..db9d0b0f 100644 --- a/crates/handlers/Cargo.toml +++ b/crates/handlers/Cargo.toml @@ -57,6 +57,7 @@ psl = "2.1.4" time = "0.3.30" url.workspace = true mime = "0.3.17" +minijinja.workspace = true rand.workspace = true rand_chacha = "0.3.1" headers = "0.3.9" diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index 62dbe653..425809e5 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -20,7 +20,6 @@ use hyper::StatusCode; use mas_axum_utils::{ cookies::CookieJar, http_client_factory::HttpClientFactory, sentry::SentryEventID, }; -use mas_jose::claims::ClaimError; use mas_keystore::{Encrypter, Keystore}; use mas_oidc_client::requests::{ authorization_code::AuthorizationValidationData, jose::JwtVerificationData, @@ -83,8 +82,11 @@ pub(crate) enum RouteError { #[error("Missing ID token")] MissingIDToken, - #[error("Invalid ID token")] - InvalidIdToken(#[from] ClaimError), + #[error("Could not extract subject from ID token")] + ExtractSubject(#[source] minijinja::Error), + + #[error("Subject is empty")] + EmptySubject, #[error("Error from the provider: {error}")] ClientError { @@ -236,10 +238,27 @@ pub(crate) async fn get( ) .await?; - let (_header, mut id_token) = id_token.ok_or(RouteError::MissingIDToken)?.into_parts(); + let (_header, id_token) = id_token.ok_or(RouteError::MissingIDToken)?.into_parts(); - // Extract the subject from the id_token - let subject = mas_jose::claims::SUB.extract_required(&mut id_token)?; + let env = { + let mut env = minijinja::Environment::new(); + env.add_global("user", minijinja::Value::from_serializable(&id_token)); + env + }; + + let template = provider + .claims_imports + .subject + .template + .as_deref() + .unwrap_or("{{ user.sub }}"); + let subject = env + .render_str(template, ()) + .map_err(RouteError::ExtractSubject)?; + + if subject.is_empty() { + return Err(RouteError::EmptySubject); + } // Look for an existing link let maybe_link = repo diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index 6e430e0f..deccb2e2 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -24,7 +24,7 @@ use mas_axum_utils::{ sentry::SentryEventID, FancyError, SessionInfoExt, }; -use mas_data_model::{UpstreamOAuthProviderImportPreference, User}; +use mas_data_model::{UpstreamOAuthProviderImportAction, User}; use mas_jose::jwt::Jwt; use mas_policy::Policy; use mas_router::UrlBuilder; @@ -38,6 +38,7 @@ use mas_templates::{ ErrorContext, TemplateContext, Templates, UpstreamExistingLinkContext, UpstreamRegister, UpstreamSuggestLink, }; +use minijinja::Environment; use serde::Deserialize; use thiserror::Error; use ulid::Ulid; @@ -64,8 +65,13 @@ pub(crate) enum RouteError { ProviderNotFound, /// Required claim was missing in id_token - #[error("Required claim {0:?} missing from the upstream provider's response")] - RequiredClaimMissing(&'static str), + #[error("Template {template:?} could not be rendered from the upstream provider's response for required claim")] + RequiredAttributeRender { + template: String, + + #[source] + source: minijinja::Error, + }, /// Session was already consumed #[error("Session already consumed")] @@ -119,15 +125,6 @@ impl IntoResponse for RouteError { } } -#[derive(Deserialize, Default)] -struct StandardClaims { - name: Option, - email: Option, - #[serde(default)] - email_verified: bool, - preferred_username: Option, -} - /// Utility function to import a claim from the upstream provider's response, /// based on the preference for that attribute. /// @@ -144,23 +141,31 @@ struct StandardClaims { /// /// Returns an error if the claim is required but missing. fn import_claim( - name: &'static str, - value: Option, - preference: &UpstreamOAuthProviderImportPreference, + environment: &Environment, + template: &str, + action: &UpstreamOAuthProviderImportAction, mut run: impl FnMut(String, bool), ) -> Result<(), RouteError> { // If this claim is ignored, we don't need to do anything. - if preference.ignore() { + if action.ignore() { return Ok(()); } - // If this claim is required and missing, we can't continue. - if value.is_none() && preference.is_required() { - return Err(RouteError::RequiredClaimMissing(name)); - } + match environment.render_str(template, ()) { + Ok(value) if value.is_empty() => { /* Do nothing on empty strings */ } - if let Some(value) = value { - run(value, preference.is_forced()); + Ok(value) => run(value, action.is_forced()), + + Err(source) => { + if action.is_required() { + return Err(RouteError::RequiredAttributeRender { + template: template.to_owned(), + source, + }); + } + + tracing::warn!(error = &source as &dyn std::error::Error, %template, "Error while rendering template"); + } } Ok(()) @@ -321,7 +326,7 @@ pub(crate) async fn get( // account or logging in an existing user let id_token = upstream_session .id_token() - .map(Jwt::<'_, StandardClaims>::try_from) + .map(Jwt::<'_, minijinja::Value>::try_from) .transpose()?; let provider = repo @@ -336,9 +341,20 @@ pub(crate) async fn get( let mut ctx = UpstreamRegister::new(&link); + let env = { + let mut e = Environment::new(); + e.add_global("user", payload); + e + }; + import_claim( - "name", - payload.name, + &env, + provider + .claims_imports + .displayname + .template + .as_deref() + .unwrap_or("{{ user.name }}"), &provider.claims_imports.displayname, |value, force| { ctx.set_display_name(value, force); @@ -346,8 +362,13 @@ pub(crate) async fn get( )?; import_claim( - "email", - payload.email, + &env, + provider + .claims_imports + .email + .template + .as_deref() + .unwrap_or("{{ user.email }}"), &provider.claims_imports.email, |value, force| { ctx.set_email(value, force); @@ -355,8 +376,13 @@ pub(crate) async fn get( )?; import_claim( - "preferred_username", - payload.preferred_username, + &env, + provider + .claims_imports + .localpart + .template + .as_deref() + .unwrap_or("{{ user.preferred_username }}"), &provider.claims_imports.localpart, |value, force| { ctx.set_localpart(value, force); @@ -450,7 +476,7 @@ pub(crate) async fn post( let id_token = upstream_session .id_token() - .map(Jwt::<'_, StandardClaims>::try_from) + .map(Jwt::<'_, minijinja::Value>::try_from) .transpose()?; let provider = repo @@ -463,12 +489,28 @@ pub(crate) async fn post( .map(|id_token| id_token.into_parts().1) .unwrap_or_default(); + let provider_email_verified = payload + .get_item(&minijinja::Value::from("email_verified")) + .map(|v| v.is_true()) + .unwrap_or(false); + // Let's try to import the claims from the ID token + let env = { + let mut e = Environment::new(); + e.add_global("user", payload); + e + }; + let mut name = None; import_claim( - "name", - payload.name, + &env, + provider + .claims_imports + .displayname + .template + .as_deref() + .unwrap_or("{{ user.name }}"), &provider.claims_imports.displayname, |value, force| { // Import the display name if it is either forced or the user has requested it @@ -480,8 +522,13 @@ pub(crate) async fn post( let mut email = None; import_claim( - "email", - payload.email, + &env, + provider + .claims_imports + .email + .template + .as_deref() + .unwrap_or("{{ user.email }}"), &provider.claims_imports.email, |value, force| { // Import the email if it is either forced or the user has requested it @@ -493,8 +540,13 @@ pub(crate) async fn post( let mut username = username; import_claim( - "preferred_username", - payload.preferred_username, + &env, + provider + .claims_imports + .localpart + .template + .as_deref() + .unwrap_or("{{ user.preferred_username }}"), &provider.claims_imports.localpart, |value, force| { // If the username is forced, override whatever was in the form @@ -535,13 +587,12 @@ pub(crate) async fn post( .user_email() .add(&mut rng, &clock, &user, email) .await?; - // Mark the email as verified according to the policy and whether the provider // claims it is, and make it the primary email. if provider .claims_imports .verify_email - .should_mark_as_verified(payload.email_verified) + .should_mark_as_verified(provider_email_verified) { let user_email = repo .user_email() diff --git a/docs/config.schema.json b/docs/config.schema.json index 5c0a830a..dfef337d 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -348,17 +348,24 @@ "type": "object", "properties": { "displayname": { - "description": "Import the displayname of the user based on the `name` claim", - "default": null, + "description": "Import the displayname of the user.", + "default": { + "action": "ignore", + "template": null + }, "allOf": [ { - "$ref": "#/definitions/ImportPreference" + "$ref": "#/definitions/DisplaynameImportPreference" } ] }, "email": { "description": "Import the email address of the user based on the `email` and `email_verified` claims", - "default": null, + "default": { + "action": "ignore", + "set_email_verification": "import", + "template": null + }, "allOf": [ { "$ref": "#/definitions/EmailImportPreference" @@ -366,11 +373,25 @@ ] }, "localpart": { - "description": "Import the localpart of the MXID based on the `preferred_username` claim", - "default": null, + "description": "Import the localpart of the MXID", + "default": { + "action": "ignore", + "template": null + }, "allOf": [ { - "$ref": "#/definitions/ImportPreference" + "$ref": "#/definitions/LocalpartImportPreference" + } + ] + }, + "subject": { + "description": "How to determine the subject of the user", + "default": { + "template": null + }, + "allOf": [ + { + "$ref": "#/definitions/SubjectImportPreference" } ] } @@ -612,6 +633,26 @@ } } }, + "DisplaynameImportPreference": { + "description": "What should be done for the displayname attribute", + "type": "object", + "properties": { + "action": { + "description": "How to handle the attribute", + "default": "ignore", + "allOf": [ + { + "$ref": "#/definitions/ImportAction" + } + ] + }, + "template": { + "description": "The Jinja2 template to use for the displayname attribute\n\nIf not provided, the default template is `{{ user.name }}`", + "default": null, + "type": "string" + } + } + }, "EmailConfig": { "description": "Configuration related to sending emails", "type": "object", @@ -728,7 +769,7 @@ } }, "EmailImportPreference": { - "description": "What should be done with the email claim", + "description": "What should be done with the email attribute", "type": "object", "properties": { "action": { @@ -748,6 +789,11 @@ "$ref": "#/definitions/SetEmailVerification" } ] + }, + "template": { + "description": "The Jinja2 template to use for the email address attribute\n\nIf not provided, the default template is `{{ user.email }}`", + "default": null, + "type": "string" } } }, @@ -942,21 +988,6 @@ } ] }, - "ImportPreference": { - "description": "What should be done with a claim", - "type": "object", - "properties": { - "action": { - "description": "How to handle the claim", - "default": "ignore", - "allOf": [ - { - "$ref": "#/definitions/ImportAction" - } - ] - } - } - }, "IpNetwork": { "oneOf": [ { @@ -1348,6 +1379,26 @@ } } }, + "LocalpartImportPreference": { + "description": "What should be done for the localpart attribute", + "type": "object", + "properties": { + "action": { + "description": "How to handle the attribute", + "default": "ignore", + "allOf": [ + { + "$ref": "#/definitions/ImportAction" + } + ] + }, + "template": { + "description": "The Jinja2 template to use for the localpart attribute\n\nIf not provided, the default template is `{{ user.preferred_username }}`", + "default": null, + "type": "string" + } + } + }, "MatrixConfig": { "description": "Configuration related to the Matrix homeserver", "type": "object", @@ -1914,6 +1965,17 @@ } ] }, + "SubjectImportPreference": { + "description": "What should be done for the subject attribute", + "type": "object", + "properties": { + "template": { + "description": "The Jinja2 template to use for the subject attribute\n\nIf not provided, the default template is `{{ user.sub }}`", + "default": null, + "type": "string" + } + } + }, "TelemetryConfig": { "description": "Configuration related to sending monitoring data", "type": "object",