diff --git a/Cargo.lock b/Cargo.lock index c0bd59eba..f1f754763 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3714,6 +3714,7 @@ dependencies = [ "http", "mas-data-model", "mas-i18n", + "mas-iana", "mas-router", "mas-spa", "minijinja", diff --git a/crates/cli/src/sync.rs b/crates/cli/src/sync.rs index 68c94f886..124ceb530 100644 --- a/crates/cli/src/sync.rs +++ b/crates/cli/src/sync.rs @@ -280,9 +280,8 @@ pub async fn config_sync( brand_name: provider.brand_name, scope: provider.scope.parse()?, token_endpoint_auth_method, - token_endpoint_signing_alg: provider - .token_endpoint_auth_signing_alg - .clone(), + token_endpoint_signing_alg: provider.token_endpoint_auth_signing_alg, + id_token_signed_response_alg: provider.id_token_signed_response_alg, client_id: provider.client_id, encrypted_client_secret, claims_imports: map_claims_imports(&provider.claims_imports), @@ -293,6 +292,7 @@ pub async fn config_sync( discovery_mode, pkce_mode, fetch_userinfo: provider.fetch_userinfo, + userinfo_signed_response_alg: provider.userinfo_signed_response_alg, response_mode, additional_authorization_parameters: provider .additional_authorization_parameters diff --git a/crates/config/src/sections/upstream_oauth2.rs b/crates/config/src/sections/upstream_oauth2.rs index 4ca1c7627..4db5b4121 100644 --- a/crates/config/src/sections/upstream_oauth2.rs +++ b/crates/config/src/sections/upstream_oauth2.rs @@ -398,6 +398,16 @@ fn is_default_true(value: &bool) -> bool { *value } +#[allow(clippy::ref_option)] +fn is_signed_response_alg_default(signed_response_alg: &JsonWebSignatureAlg) -> bool { + *signed_response_alg == signed_response_alg_default() +} + +#[allow(clippy::unnecessary_wraps)] +fn signed_response_alg_default() -> JsonWebSignatureAlg { + JsonWebSignatureAlg::Rs256 +} + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct SignInWithApple { /// The private key used to sign the `id_token` @@ -472,6 +482,16 @@ pub struct Provider { #[serde(skip_serializing_if = "Option::is_none")] pub token_endpoint_auth_signing_alg: Option, + /// Expected signature for the JWT payload returned by the token + /// authentication endpoint. + /// + /// Defaults to `RS256`. + #[serde( + default = "signed_response_alg_default", + skip_serializing_if = "is_signed_response_alg_default" + )] + pub id_token_signed_response_alg: JsonWebSignatureAlg, + /// The scopes to request from the provider pub scope: String, @@ -497,6 +517,14 @@ pub struct Provider { #[serde(default)] pub fetch_userinfo: bool, + /// Expected signature for the JWT payload returned by the userinfo + /// endpoint. + /// + /// If not specified, the response is expected to be an unsigned JSON + /// payload. + #[serde(skip_serializing_if = "Option::is_none")] + pub userinfo_signed_response_alg: Option, + /// The URL to use for the provider's authorization endpoint /// /// Defaults to the `authorization_endpoint` provided through discovery diff --git a/crates/data-model/src/upstream_oauth2/provider.rs b/crates/data-model/src/upstream_oauth2/provider.rs index ff6f1d1fc..bfbbb8d9f 100644 --- a/crates/data-model/src/upstream_oauth2/provider.rs +++ b/crates/data-model/src/upstream_oauth2/provider.rs @@ -230,10 +230,12 @@ pub struct UpstreamOAuthProvider { pub token_endpoint_override: Option, pub userinfo_endpoint_override: Option, pub fetch_userinfo: bool, + pub userinfo_signed_response_alg: Option, pub client_id: String, pub encrypted_client_secret: Option, pub token_endpoint_signing_alg: Option, pub token_endpoint_auth_method: TokenAuthMethod, + pub id_token_signed_response_alg: JsonWebSignatureAlg, pub response_mode: ResponseMode, pub created_at: DateTime, pub disabled_at: Option>, diff --git a/crates/handlers/src/upstream_oauth2/cache.rs b/crates/handlers/src/upstream_oauth2/cache.rs index 1e5f683e0..838a82f5b 100644 --- a/crates/handlers/src/upstream_oauth2/cache.rs +++ b/crates/handlers/src/upstream_oauth2/cache.rs @@ -289,6 +289,7 @@ mod tests { use mas_data_model::{ UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderTokenAuthMethod, }; + use mas_iana::jose::JsonWebSignatureAlg; use mas_storage::{clock::MockClock, Clock}; use oauth2_types::scope::{Scope, OPENID}; use ulid::Ulid; @@ -400,6 +401,7 @@ mod tests { discovery_mode: UpstreamOAuthProviderDiscoveryMode::Insecure, pkce_mode: UpstreamOAuthProviderPkceMode::Auto, fetch_userinfo: false, + userinfo_signed_response_alg: None, jwks_uri_override: None, authorization_endpoint_override: None, scope: Scope::from_iter([OPENID]), @@ -410,6 +412,7 @@ mod tests { token_endpoint_signing_alg: None, token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, response_mode: mas_data_model::UpstreamOAuthProviderResponseMode::Query, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, created_at: clock.now(), disabled_at: None, claims_imports: UpstreamOAuthProviderClaimsImports::default(), diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index 6ce4b8e39..1e3e2990e 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -274,25 +274,26 @@ pub(crate) async fn handler( ) .await?; + let mut jwks = None; + let mut context = AttributeMappingContext::new(); if let Some(id_token) = token_response.id_token.as_ref() { - // Fetch the JWKS - let jwks = + jwks = Some( mas_oidc_client::requests::jose::fetch_jwks(&client, lazy_metadata.jwks_uri().await?) - .await?; + .await?, + ); - let verification_data = JwtVerificationData { + let id_token_verification_data = JwtVerificationData { issuer: &provider.issuer, - jwks: &jwks, - // TODO: make that configurable - signing_algorithm: &mas_iana::jose::JsonWebSignatureAlg::Rs256, + jwks: jwks.as_ref().unwrap(), + signing_algorithm: &provider.id_token_signed_response_alg, client_id: &provider.client_id, }; // Decode and verify the ID token let id_token = mas_oidc_client::requests::jose::verify_id_token( id_token, - verification_data, + id_token_verification_data, None, clock.now(), )?; @@ -304,7 +305,7 @@ pub(crate) async fn handler( .extract_optional_with_options( &mut claims, TokenHash::new( - verification_data.signing_algorithm, + id_token_verification_data.signing_algorithm, &token_response.access_token, ), ) @@ -314,7 +315,7 @@ pub(crate) async fn handler( mas_jose::claims::C_HASH .extract_optional_with_options( &mut claims, - TokenHash::new(verification_data.signing_algorithm, &code), + TokenHash::new(id_token_verification_data.signing_algorithm, &code), ) .map_err(mas_oidc_client::error::IdTokenError::from)?; @@ -331,15 +332,42 @@ pub(crate) async fn handler( } let userinfo = if provider.fetch_userinfo { - Some(json!( - mas_oidc_client::requests::userinfo::fetch_userinfo( - &client, - lazy_metadata.userinfo_endpoint().await?, - token_response.access_token.as_str(), - None, - ) - .await? - )) + Some(json!(match &provider.userinfo_signed_response_alg { + Some(signing_algorithm) => { + let jwks = match jwks { + Some(jwks) => jwks, + None => { + mas_oidc_client::requests::jose::fetch_jwks( + &client, + lazy_metadata.jwks_uri().await?, + ) + .await? + } + }; + + mas_oidc_client::requests::userinfo::fetch_userinfo( + &client, + lazy_metadata.userinfo_endpoint().await?, + token_response.access_token.as_str(), + Some(JwtVerificationData { + issuer: &provider.issuer, + jwks: &jwks, + signing_algorithm, + client_id: &provider.client_id, + }), + ) + .await? + } + None => { + mas_oidc_client::requests::userinfo::fetch_userinfo( + &client, + lazy_metadata.userinfo_endpoint().await?, + token_response.access_token.as_str(), + None, + ) + .await? + } + })) } else { None }; diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index b58da8a3b..b183de977 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -922,6 +922,7 @@ mod tests { scope: Scope::from_iter([OPENID]), token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, token_endpoint_signing_alg: None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, client_id: "client".to_owned(), encrypted_client_secret: None, claims_imports, @@ -929,6 +930,7 @@ mod tests { token_endpoint_override: None, userinfo_endpoint_override: None, fetch_userinfo: false, + userinfo_signed_response_alg: None, jwks_uri_override: None, discovery_mode: mas_data_model::UpstreamOAuthProviderDiscoveryMode::Oidc, pkce_mode: mas_data_model::UpstreamOAuthProviderPkceMode::Auto, diff --git a/crates/handlers/src/views/login.rs b/crates/handlers/src/views/login.rs index b40b5041c..e42f8d643 100644 --- a/crates/handlers/src/views/login.rs +++ b/crates/handlers/src/views/login.rs @@ -346,6 +346,7 @@ mod test { use mas_data_model::{ UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderTokenAuthMethod, }; + use mas_iana::jose::JsonWebSignatureAlg; use mas_router::Route; use mas_storage::{ upstream_oauth2::{UpstreamOAuthProviderParams, UpstreamOAuthProviderRepository}, @@ -403,7 +404,9 @@ mod test { scope: [OPENID].into_iter().collect(), token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, token_endpoint_signing_alg: None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, fetch_userinfo: false, + userinfo_signed_response_alg: None, client_id: "client".to_owned(), encrypted_client_secret: None, claims_imports: UpstreamOAuthProviderClaimsImports::default(), @@ -441,7 +444,9 @@ mod test { scope: [OPENID].into_iter().collect(), token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, token_endpoint_signing_alg: None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, fetch_userinfo: false, + userinfo_signed_response_alg: None, client_id: "client".to_owned(), encrypted_client_secret: None, claims_imports: UpstreamOAuthProviderClaimsImports::default(), diff --git a/crates/storage-pg/.sqlx/query-887bd597132831c5caab2356f2d935c00a32274161ec5265da91d1c75ad0bb2b.json b/crates/storage-pg/.sqlx/query-1d758df58ccfead4cb39ee8f88f60b382b7881e9c4ead31ff257ff5ff4414b6e.json similarity index 72% rename from crates/storage-pg/.sqlx/query-887bd597132831c5caab2356f2d935c00a32274161ec5265da91d1c75ad0bb2b.json rename to crates/storage-pg/.sqlx/query-1d758df58ccfead4cb39ee8f88f60b382b7881e9c4ead31ff257ff5ff4414b6e.json index e93f49091..31d0bcb48 100644 --- a/crates/storage-pg/.sqlx/query-887bd597132831c5caab2356f2d935c00a32274161ec5265da91d1c75ad0bb2b.json +++ b/crates/storage-pg/.sqlx/query-1d758df58ccfead4cb39ee8f88f60b382b7881e9c4ead31ff257ff5ff4414b6e.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n client_id,\n encrypted_client_secret,\n token_endpoint_signing_alg,\n token_endpoint_auth_method,\n fetch_userinfo,\n created_at,\n disabled_at,\n claims_imports as \"claims_imports: Json\",\n jwks_uri_override,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n additional_parameters as \"additional_parameters: Json>\"\n FROM upstream_oauth_providers\n WHERE upstream_oauth_provider_id = $1\n ", + "query": "\n SELECT\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n client_id,\n encrypted_client_secret,\n token_endpoint_signing_alg,\n token_endpoint_auth_method,\n id_token_signed_response_alg,\n fetch_userinfo,\n userinfo_signed_response_alg,\n created_at,\n disabled_at,\n claims_imports as \"claims_imports: Json\",\n jwks_uri_override,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n additional_parameters as \"additional_parameters: Json>\"\n FROM upstream_oauth_providers\n WHERE upstream_oauth_provider_id = $1\n ", "describe": { "columns": [ { @@ -50,61 +50,71 @@ }, { "ordinal": 9, + "name": "id_token_signed_response_alg", + "type_info": "Text" + }, + { + "ordinal": 10, "name": "fetch_userinfo", "type_info": "Bool" }, { - "ordinal": 10, + "ordinal": 11, + "name": "userinfo_signed_response_alg", + "type_info": "Text" + }, + { + "ordinal": 12, "name": "created_at", "type_info": "Timestamptz" }, { - "ordinal": 11, + "ordinal": 13, "name": "disabled_at", "type_info": "Timestamptz" }, { - "ordinal": 12, + "ordinal": 14, "name": "claims_imports: Json", "type_info": "Jsonb" }, { - "ordinal": 13, + "ordinal": 15, "name": "jwks_uri_override", "type_info": "Text" }, { - "ordinal": 14, + "ordinal": 16, "name": "authorization_endpoint_override", "type_info": "Text" }, { - "ordinal": 15, + "ordinal": 17, "name": "token_endpoint_override", "type_info": "Text" }, { - "ordinal": 16, + "ordinal": 18, "name": "userinfo_endpoint_override", "type_info": "Text" }, { - "ordinal": 17, + "ordinal": 19, "name": "discovery_mode", "type_info": "Text" }, { - "ordinal": 18, + "ordinal": 20, "name": "pkce_mode", "type_info": "Text" }, { - "ordinal": 19, + "ordinal": 21, "name": "response_mode", "type_info": "Text" }, { - "ordinal": 20, + "ordinal": 22, "name": "additional_parameters: Json>", "type_info": "Jsonb" } @@ -129,6 +139,8 @@ true, false, true, + false, + true, true, true, true, @@ -138,5 +150,5 @@ true ] }, - "hash": "887bd597132831c5caab2356f2d935c00a32274161ec5265da91d1c75ad0bb2b" + "hash": "1d758df58ccfead4cb39ee8f88f60b382b7881e9c4ead31ff257ff5ff4414b6e" } diff --git a/crates/storage-pg/.sqlx/query-39657c8064532745c8a8a944b73f650b468a4677eddf671c69c329d361edf00e.json b/crates/storage-pg/.sqlx/query-27d6f228a9a608b5d03d30cb4074be94dc893df9107e982583aa954b5067dfd1.json similarity index 72% rename from crates/storage-pg/.sqlx/query-39657c8064532745c8a8a944b73f650b468a4677eddf671c69c329d361edf00e.json rename to crates/storage-pg/.sqlx/query-27d6f228a9a608b5d03d30cb4074be94dc893df9107e982583aa954b5067dfd1.json index 1f6b0e0f5..77d46d050 100644 --- a/crates/storage-pg/.sqlx/query-39657c8064532745c8a8a944b73f650b468a4677eddf671c69c329d361edf00e.json +++ b/crates/storage-pg/.sqlx/query-27d6f228a9a608b5d03d30cb4074be94dc893df9107e982583aa954b5067dfd1.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n client_id,\n encrypted_client_secret,\n token_endpoint_signing_alg,\n token_endpoint_auth_method,\n fetch_userinfo,\n created_at,\n disabled_at,\n claims_imports as \"claims_imports: Json\",\n jwks_uri_override,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n additional_parameters as \"additional_parameters: Json>\"\n FROM upstream_oauth_providers\n WHERE disabled_at IS NULL\n ", + "query": "\n SELECT\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n client_id,\n encrypted_client_secret,\n token_endpoint_signing_alg,\n token_endpoint_auth_method,\n id_token_signed_response_alg,\n fetch_userinfo,\n userinfo_signed_response_alg,\n created_at,\n disabled_at,\n claims_imports as \"claims_imports: Json\",\n jwks_uri_override,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n additional_parameters as \"additional_parameters: Json>\"\n FROM upstream_oauth_providers\n WHERE disabled_at IS NULL\n ", "describe": { "columns": [ { @@ -50,61 +50,71 @@ }, { "ordinal": 9, + "name": "id_token_signed_response_alg", + "type_info": "Text" + }, + { + "ordinal": 10, "name": "fetch_userinfo", "type_info": "Bool" }, { - "ordinal": 10, + "ordinal": 11, + "name": "userinfo_signed_response_alg", + "type_info": "Text" + }, + { + "ordinal": 12, "name": "created_at", "type_info": "Timestamptz" }, { - "ordinal": 11, + "ordinal": 13, "name": "disabled_at", "type_info": "Timestamptz" }, { - "ordinal": 12, + "ordinal": 14, "name": "claims_imports: Json", "type_info": "Jsonb" }, { - "ordinal": 13, + "ordinal": 15, "name": "jwks_uri_override", "type_info": "Text" }, { - "ordinal": 14, + "ordinal": 16, "name": "authorization_endpoint_override", "type_info": "Text" }, { - "ordinal": 15, + "ordinal": 17, "name": "token_endpoint_override", "type_info": "Text" }, { - "ordinal": 16, + "ordinal": 18, "name": "userinfo_endpoint_override", "type_info": "Text" }, { - "ordinal": 17, + "ordinal": 19, "name": "discovery_mode", "type_info": "Text" }, { - "ordinal": 18, + "ordinal": 20, "name": "pkce_mode", "type_info": "Text" }, { - "ordinal": 19, + "ordinal": 21, "name": "response_mode", "type_info": "Text" }, { - "ordinal": 20, + "ordinal": 22, "name": "additional_parameters: Json>", "type_info": "Jsonb" } @@ -127,6 +137,8 @@ true, false, true, + false, + true, true, true, true, @@ -136,5 +148,5 @@ true ] }, - "hash": "39657c8064532745c8a8a944b73f650b468a4677eddf671c69c329d361edf00e" + "hash": "27d6f228a9a608b5d03d30cb4074be94dc893df9107e982583aa954b5067dfd1" } diff --git a/crates/storage-pg/.sqlx/query-8e1c0760c0b652cf62e47779f9d0aef89463cc60eeae2088d0fedf0aeb75718b.json b/crates/storage-pg/.sqlx/query-8e1c0760c0b652cf62e47779f9d0aef89463cc60eeae2088d0fedf0aeb75718b.json deleted file mode 100644 index 81e3ec219..000000000 --- a/crates/storage-pg/.sqlx/query-8e1c0760c0b652cf62e47779f9d0aef89463cc60eeae2088d0fedf0aeb75718b.json +++ /dev/null @@ -1,32 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n INSERT INTO upstream_oauth_providers (\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n token_endpoint_auth_method,\n fetch_userinfo,\n token_endpoint_signing_alg,\n client_id,\n encrypted_client_secret,\n claims_imports,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n jwks_uri_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n created_at\n ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10,\n $11, $12, $13, $14, $15, $16, $17, $18, $19)\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid", - "Text", - "Text", - "Text", - "Text", - "Text", - "Bool", - "Text", - "Text", - "Text", - "Jsonb", - "Text", - "Text", - "Text", - "Text", - "Text", - "Text", - "Text", - "Timestamptz" - ] - }, - "nullable": [] - }, - "hash": "8e1c0760c0b652cf62e47779f9d0aef89463cc60eeae2088d0fedf0aeb75718b" -} diff --git a/crates/storage-pg/.sqlx/query-99f2a0b53e08d23408dc2837d32d734c8a0e706662e72f3b2585b0c38f42c063.json b/crates/storage-pg/.sqlx/query-99f2a0b53e08d23408dc2837d32d734c8a0e706662e72f3b2585b0c38f42c063.json new file mode 100644 index 000000000..e24c20364 --- /dev/null +++ b/crates/storage-pg/.sqlx/query-99f2a0b53e08d23408dc2837d32d734c8a0e706662e72f3b2585b0c38f42c063.json @@ -0,0 +1,43 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO upstream_oauth_providers (\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n token_endpoint_auth_method,\n token_endpoint_signing_alg,\n id_token_signed_response_alg,\n fetch_userinfo,\n userinfo_signed_response_alg,\n client_id,\n encrypted_client_secret,\n claims_imports,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n jwks_uri_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n additional_parameters,\n created_at\n ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11,\n $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22)\n ON CONFLICT (upstream_oauth_provider_id)\n DO UPDATE\n SET\n issuer = EXCLUDED.issuer,\n human_name = EXCLUDED.human_name,\n brand_name = EXCLUDED.brand_name,\n scope = EXCLUDED.scope,\n token_endpoint_auth_method = EXCLUDED.token_endpoint_auth_method,\n token_endpoint_signing_alg = EXCLUDED.token_endpoint_signing_alg,\n id_token_signed_response_alg = EXCLUDED.id_token_signed_response_alg,\n fetch_userinfo = EXCLUDED.fetch_userinfo,\n userinfo_signed_response_alg = EXCLUDED.userinfo_signed_response_alg,\n disabled_at = NULL,\n client_id = EXCLUDED.client_id,\n encrypted_client_secret = EXCLUDED.encrypted_client_secret,\n claims_imports = EXCLUDED.claims_imports,\n authorization_endpoint_override = EXCLUDED.authorization_endpoint_override,\n token_endpoint_override = EXCLUDED.token_endpoint_override,\n userinfo_endpoint_override = EXCLUDED.userinfo_endpoint_override,\n jwks_uri_override = EXCLUDED.jwks_uri_override,\n discovery_mode = EXCLUDED.discovery_mode,\n pkce_mode = EXCLUDED.pkce_mode,\n response_mode = EXCLUDED.response_mode,\n additional_parameters = EXCLUDED.additional_parameters\n RETURNING created_at\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "created_at", + "type_info": "Timestamptz" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Bool", + "Text", + "Text", + "Text", + "Jsonb", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Jsonb", + "Timestamptz" + ] + }, + "nullable": [ + false + ] + }, + "hash": "99f2a0b53e08d23408dc2837d32d734c8a0e706662e72f3b2585b0c38f42c063" +} diff --git a/crates/storage-pg/.sqlx/query-bf7747552fe6f5489dec3c91fe1cb13a737644b94871c28334a29c88977dd84c.json b/crates/storage-pg/.sqlx/query-bf7747552fe6f5489dec3c91fe1cb13a737644b94871c28334a29c88977dd84c.json deleted file mode 100644 index 190c6221f..000000000 --- a/crates/storage-pg/.sqlx/query-bf7747552fe6f5489dec3c91fe1cb13a737644b94871c28334a29c88977dd84c.json +++ /dev/null @@ -1,41 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n INSERT INTO upstream_oauth_providers (\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n token_endpoint_auth_method,\n fetch_userinfo,\n token_endpoint_signing_alg,\n client_id,\n encrypted_client_secret,\n claims_imports,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n jwks_uri_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n additional_parameters,\n created_at\n ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11,\n $12, $13, $14, $15, $16, $17, $18, $19, $20)\n ON CONFLICT (upstream_oauth_provider_id)\n DO UPDATE\n SET\n issuer = EXCLUDED.issuer,\n human_name = EXCLUDED.human_name,\n brand_name = EXCLUDED.brand_name,\n scope = EXCLUDED.scope,\n token_endpoint_auth_method = EXCLUDED.token_endpoint_auth_method,\n fetch_userinfo = EXCLUDED.fetch_userinfo,\n token_endpoint_signing_alg = EXCLUDED.token_endpoint_signing_alg,\n disabled_at = NULL,\n client_id = EXCLUDED.client_id,\n encrypted_client_secret = EXCLUDED.encrypted_client_secret,\n claims_imports = EXCLUDED.claims_imports,\n authorization_endpoint_override = EXCLUDED.authorization_endpoint_override,\n token_endpoint_override = EXCLUDED.token_endpoint_override,\n userinfo_endpoint_override = EXCLUDED.userinfo_endpoint_override,\n jwks_uri_override = EXCLUDED.jwks_uri_override,\n discovery_mode = EXCLUDED.discovery_mode,\n pkce_mode = EXCLUDED.pkce_mode,\n response_mode = EXCLUDED.response_mode,\n additional_parameters = EXCLUDED.additional_parameters\n RETURNING created_at\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "created_at", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Uuid", - "Text", - "Text", - "Text", - "Text", - "Text", - "Bool", - "Text", - "Text", - "Text", - "Jsonb", - "Text", - "Text", - "Text", - "Text", - "Text", - "Text", - "Text", - "Jsonb", - "Timestamptz" - ] - }, - "nullable": [ - false - ] - }, - "hash": "bf7747552fe6f5489dec3c91fe1cb13a737644b94871c28334a29c88977dd84c" -} diff --git a/crates/storage-pg/.sqlx/query-e25af41189846e26da99e5d8a1462eab5efe330f60ef8c6c813c747424ba7ec9.json b/crates/storage-pg/.sqlx/query-e25af41189846e26da99e5d8a1462eab5efe330f60ef8c6c813c747424ba7ec9.json new file mode 100644 index 000000000..1a2a19d81 --- /dev/null +++ b/crates/storage-pg/.sqlx/query-e25af41189846e26da99e5d8a1462eab5efe330f60ef8c6c813c747424ba7ec9.json @@ -0,0 +1,34 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO upstream_oauth_providers (\n upstream_oauth_provider_id,\n issuer,\n human_name,\n brand_name,\n scope,\n token_endpoint_auth_method,\n token_endpoint_signing_alg,\n id_token_signed_response_alg,\n fetch_userinfo,\n userinfo_signed_response_alg,\n client_id,\n encrypted_client_secret,\n claims_imports,\n authorization_endpoint_override,\n token_endpoint_override,\n userinfo_endpoint_override,\n jwks_uri_override,\n discovery_mode,\n pkce_mode,\n response_mode,\n created_at\n ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10,\n $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21)\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Bool", + "Text", + "Text", + "Text", + "Jsonb", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Text", + "Timestamptz" + ] + }, + "nullable": [] + }, + "hash": "e25af41189846e26da99e5d8a1462eab5efe330f60ef8c6c813c747424ba7ec9" +} diff --git a/crates/storage-pg/migrations/20241202123523_upstream_oauth_responses_alg.sql b/crates/storage-pg/migrations/20241202123523_upstream_oauth_responses_alg.sql new file mode 100644 index 000000000..baa7e92cc --- /dev/null +++ b/crates/storage-pg/migrations/20241202123523_upstream_oauth_responses_alg.sql @@ -0,0 +1,10 @@ +-- Copyright 2024 New Vector Ltd. +-- +-- SPDX-License-Identifier: AGPL-3.0-only +-- Please see LICENSE in the repository root for full details. + +-- Add columns to upstream_oauth_providers to specify the +-- expected signing algorithm for the endpoint JWT responses. +ALTER TABLE "upstream_oauth_providers" + ADD COLUMN "id_token_signed_response_alg" TEXT NOT NULL DEFAULT 'RS256', + ADD COLUMN "userinfo_signed_response_alg" TEXT; diff --git a/crates/storage-pg/src/iden.rs b/crates/storage-pg/src/iden.rs index c6f53f3b0..46cb2bc85 100644 --- a/crates/storage-pg/src/iden.rs +++ b/crates/storage-pg/src/iden.rs @@ -98,7 +98,9 @@ pub enum UpstreamOAuthProviders { EncryptedClientSecret, TokenEndpointSigningAlg, TokenEndpointAuthMethod, + IdTokenSignedResponseAlg, FetchUserinfo, + UserinfoSignedResponseAlg, CreatedAt, DisabledAt, ClaimsImports, diff --git a/crates/storage-pg/src/upstream_oauth2/mod.rs b/crates/storage-pg/src/upstream_oauth2/mod.rs index 04a774185..bdaac8b26 100644 --- a/crates/storage-pg/src/upstream_oauth2/mod.rs +++ b/crates/storage-pg/src/upstream_oauth2/mod.rs @@ -22,6 +22,7 @@ mod tests { use mas_data_model::{ UpstreamOAuthProviderClaimsImports, UpstreamOAuthProviderTokenAuthMethod, }; + use mas_iana::jose::JsonWebSignatureAlg; use mas_storage::{ clock::MockClock, upstream_oauth2::{ @@ -60,7 +61,9 @@ mod tests { brand_name: None, scope: Scope::from_iter([OPENID]), token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, fetch_userinfo: false, + userinfo_signed_response_alg: None, token_endpoint_signing_alg: None, client_id: "client-id".to_owned(), encrypted_client_secret: None, @@ -305,7 +308,9 @@ mod tests { scope: scope.clone(), token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None, fetch_userinfo: false, + userinfo_signed_response_alg: None, token_endpoint_signing_alg: None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, client_id, encrypted_client_secret: None, claims_imports: UpstreamOAuthProviderClaimsImports::default(), diff --git a/crates/storage-pg/src/upstream_oauth2/provider.rs b/crates/storage-pg/src/upstream_oauth2/provider.rs index 35412d460..b9befe59c 100644 --- a/crates/storage-pg/src/upstream_oauth2/provider.rs +++ b/crates/storage-pg/src/upstream_oauth2/provider.rs @@ -56,7 +56,9 @@ struct ProviderLookup { encrypted_client_secret: Option, token_endpoint_signing_alg: Option, token_endpoint_auth_method: String, + id_token_signed_response_alg: String, fetch_userinfo: bool, + userinfo_signed_response_alg: Option, created_at: DateTime, disabled_at: Option>, claims_imports: Json, @@ -98,6 +100,24 @@ impl TryFrom for UpstreamOAuthProvider { .row(id) .source(e) })?; + let id_token_signed_response_alg = + value.id_token_signed_response_alg.parse().map_err(|e| { + DatabaseInconsistencyError::on("upstream_oauth_providers") + .column("id_token_signed_response_alg") + .row(id) + .source(e) + })?; + + let userinfo_signed_response_alg = value + .userinfo_signed_response_alg + .map(|x| x.parse()) + .transpose() + .map_err(|e| { + DatabaseInconsistencyError::on("upstream_oauth_providers") + .column("userinfo_signed_response_alg") + .row(id) + .source(e) + })?; let authorization_endpoint_override = value .authorization_endpoint_override @@ -178,8 +198,10 @@ impl TryFrom for UpstreamOAuthProvider { client_id: value.client_id, encrypted_client_secret: value.encrypted_client_secret, token_endpoint_auth_method, - fetch_userinfo: value.fetch_userinfo, token_endpoint_signing_alg, + id_token_signed_response_alg, + fetch_userinfo: value.fetch_userinfo, + userinfo_signed_response_alg, created_at: value.created_at, disabled_at: value.disabled_at, claims_imports: value.claims_imports.0, @@ -235,7 +257,9 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { encrypted_client_secret, token_endpoint_signing_alg, token_endpoint_auth_method, + id_token_signed_response_alg, fetch_userinfo, + userinfo_signed_response_alg, created_at, disabled_at, claims_imports as "claims_imports: Json", @@ -294,8 +318,10 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { brand_name, scope, token_endpoint_auth_method, - fetch_userinfo, token_endpoint_signing_alg, + id_token_signed_response_alg, + fetch_userinfo, + userinfo_signed_response_alg, client_id, encrypted_client_secret, claims_imports, @@ -308,7 +334,7 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { response_mode, created_at ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, - $11, $12, $13, $14, $15, $16, $17, $18, $19) + $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21) "#, Uuid::from(id), ¶ms.issuer, @@ -316,11 +342,16 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { params.brand_name.as_deref(), params.scope.to_string(), params.token_endpoint_auth_method.to_string(), - params.fetch_userinfo, params .token_endpoint_signing_alg .as_ref() .map(ToString::to_string), + params.id_token_signed_response_alg.to_string(), + params.fetch_userinfo, + params + .userinfo_signed_response_alg + .as_ref() + .map(ToString::to_string), ¶ms.client_id, params.encrypted_client_secret.as_deref(), Json(¶ms.claims_imports) as _, @@ -356,7 +387,9 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { encrypted_client_secret: params.encrypted_client_secret, token_endpoint_signing_alg: params.token_endpoint_signing_alg, token_endpoint_auth_method: params.token_endpoint_auth_method, + id_token_signed_response_alg: params.id_token_signed_response_alg, fetch_userinfo: params.fetch_userinfo, + userinfo_signed_response_alg: params.userinfo_signed_response_alg, created_at, disabled_at: None, claims_imports: params.claims_imports, @@ -465,8 +498,10 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { brand_name, scope, token_endpoint_auth_method, - fetch_userinfo, token_endpoint_signing_alg, + id_token_signed_response_alg, + fetch_userinfo, + userinfo_signed_response_alg, client_id, encrypted_client_secret, claims_imports, @@ -480,7 +515,7 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { additional_parameters, created_at ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, - $12, $13, $14, $15, $16, $17, $18, $19, $20) + $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22) ON CONFLICT (upstream_oauth_provider_id) DO UPDATE SET @@ -489,8 +524,10 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { brand_name = EXCLUDED.brand_name, scope = EXCLUDED.scope, token_endpoint_auth_method = EXCLUDED.token_endpoint_auth_method, - fetch_userinfo = EXCLUDED.fetch_userinfo, token_endpoint_signing_alg = EXCLUDED.token_endpoint_signing_alg, + id_token_signed_response_alg = EXCLUDED.id_token_signed_response_alg, + fetch_userinfo = EXCLUDED.fetch_userinfo, + userinfo_signed_response_alg = EXCLUDED.userinfo_signed_response_alg, disabled_at = NULL, client_id = EXCLUDED.client_id, encrypted_client_secret = EXCLUDED.encrypted_client_secret, @@ -511,11 +548,16 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { params.brand_name.as_deref(), params.scope.to_string(), params.token_endpoint_auth_method.to_string(), - params.fetch_userinfo, params .token_endpoint_signing_alg .as_ref() .map(ToString::to_string), + params.id_token_signed_response_alg.to_string(), + params.fetch_userinfo, + params + .userinfo_signed_response_alg + .as_ref() + .map(ToString::to_string), ¶ms.client_id, params.encrypted_client_secret.as_deref(), Json(¶ms.claims_imports) as _, @@ -552,7 +594,9 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { encrypted_client_secret: params.encrypted_client_secret, token_endpoint_signing_alg: params.token_endpoint_signing_alg, token_endpoint_auth_method: params.token_endpoint_auth_method, + id_token_signed_response_alg: params.id_token_signed_response_alg, fetch_userinfo: params.fetch_userinfo, + userinfo_signed_response_alg: params.userinfo_signed_response_alg, created_at, disabled_at: None, claims_imports: params.claims_imports, @@ -679,9 +723,9 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { .expr_as( Expr::col(( UpstreamOAuthProviders::Table, - UpstreamOAuthProviders::CreatedAt, + UpstreamOAuthProviders::IdTokenSignedResponseAlg, )), - ProviderLookupIden::CreatedAt, + ProviderLookupIden::IdTokenSignedResponseAlg, ) .expr_as( Expr::col(( @@ -690,6 +734,20 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { )), ProviderLookupIden::FetchUserinfo, ) + .expr_as( + Expr::col(( + UpstreamOAuthProviders::Table, + UpstreamOAuthProviders::UserinfoSignedResponseAlg, + )), + ProviderLookupIden::UserinfoSignedResponseAlg, + ) + .expr_as( + Expr::col(( + UpstreamOAuthProviders::Table, + UpstreamOAuthProviders::CreatedAt, + )), + ProviderLookupIden::CreatedAt, + ) .expr_as( Expr::col(( UpstreamOAuthProviders::Table, @@ -839,7 +897,9 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> { encrypted_client_secret, token_endpoint_signing_alg, token_endpoint_auth_method, + id_token_signed_response_alg, fetch_userinfo, + userinfo_signed_response_alg, created_at, disabled_at, claims_imports as "claims_imports: Json", diff --git a/crates/storage/src/upstream_oauth2/provider.rs b/crates/storage/src/upstream_oauth2/provider.rs index 47050089d..e2271e818 100644 --- a/crates/storage/src/upstream_oauth2/provider.rs +++ b/crates/storage/src/upstream_oauth2/provider.rs @@ -42,11 +42,24 @@ pub struct UpstreamOAuthProviderParams { /// `private_key_jwt` authentication methods are used pub token_endpoint_signing_alg: Option, + /// Expected signature for the JWT payload returned by the token + /// authentication endpoint. + /// + /// Defaults to `RS256`. + pub id_token_signed_response_alg: JsonWebSignatureAlg, + /// Whether to fetch the user profile from the userinfo endpoint, /// or to rely on the data returned in the `id_token` from the /// `token_endpoint`. pub fetch_userinfo: bool, + /// Expected signature for the JWT payload returned by the userinfo + /// endpoint. + /// + /// If not specified, the response is expected to be an unsigned JSON + /// payload. Defaults to `None`. + pub userinfo_signed_response_alg: Option, + /// The client ID to use when authenticating to the upstream pub client_id: String, diff --git a/crates/templates/Cargo.toml b/crates/templates/Cargo.toml index 1d1e60d22..696a517b9 100644 --- a/crates/templates/Cargo.toml +++ b/crates/templates/Cargo.toml @@ -37,5 +37,6 @@ rand.workspace = true oauth2-types.workspace = true mas-data-model.workspace = true mas-i18n.workspace = true +mas-iana.workspace = true mas-router.workspace = true mas-spa.workspace = true diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 5bf207b11..6a20a4502 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -26,6 +26,7 @@ use mas_data_model::{ UserEmail, UserEmailVerification, UserRecoverySession, }; use mas_i18n::DataLocale; +use mas_iana::jose::JsonWebSignatureAlg; use mas_router::{Account, GraphQL, PostAuthAction, UrlBuilder}; use oauth2_types::scope::{Scope, OPENID}; use rand::{ @@ -1395,6 +1396,7 @@ impl TemplateContext for UpstreamRegister { scope: Scope::from_iter([OPENID]), token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::ClientSecretBasic, token_endpoint_signing_alg: None, + id_token_signed_response_alg: JsonWebSignatureAlg::Rs256, client_id: "client-id".to_owned(), encrypted_client_secret: None, claims_imports: UpstreamOAuthProviderClaimsImports::default(), @@ -1403,6 +1405,7 @@ impl TemplateContext for UpstreamRegister { jwks_uri_override: None, userinfo_endpoint_override: None, fetch_userinfo: false, + userinfo_signed_response_alg: None, discovery_mode: UpstreamOAuthProviderDiscoveryMode::Oidc, pkce_mode: UpstreamOAuthProviderPkceMode::Auto, response_mode: UpstreamOAuthProviderResponseMode::Query, diff --git a/docs/config.schema.json b/docs/config.schema.json index 2396c7cfc..778c0d61a 100644 --- a/docs/config.schema.json +++ b/docs/config.schema.json @@ -1875,6 +1875,14 @@ } ] }, + "id_token_signed_response_alg": { + "description": "Expected signature for the JWT payload returned by the token authentication endpoint.\n\nDefaults to `RS256`.", + "allOf": [ + { + "$ref": "#/definitions/JsonWebSignatureAlg" + } + ] + }, "scope": { "description": "The scopes to request from the provider", "type": "string" @@ -1900,6 +1908,14 @@ "default": false, "type": "boolean" }, + "userinfo_signed_response_alg": { + "description": "Expected signature for the JWT payload returned by the userinfo endpoint.\n\nIf not specified, the response is expected to be an unsigned JSON payload.", + "allOf": [ + { + "$ref": "#/definitions/JsonWebSignatureAlg" + } + ] + }, "authorization_endpoint": { "description": "The URL to use for the provider's authorization endpoint\n\nDefaults to the `authorization_endpoint` provided through discovery", "type": "string",