Upstream OAuth 2.0 providers: Support signed userinfo and customising the expected id_token signature algorithm

crates/config/src/sections/upstream_oauth2.rs

@@ -399,13 +399,25 @@ fn is_default_true(value: &bool) -> bool {
 }
 
 #[allow(clippy::ref_option)]
-fn is_signed_response_alg_default(signed_response_alg: &Option<JsonWebSignatureAlg>) -> bool {
+fn is_signed_response_alg_default(signed_response_alg: &JsonWebSignatureAlg) -> bool {
     *signed_response_alg == signed_response_alg_default()
 }
 
+#[allow(clippy::ref_option)]
+fn is_optional_signed_response_alg_default(
+    optional_signed_response_alg: &Option<JsonWebSignatureAlg>,
+) -> bool {
+    *optional_signed_response_alg == optional_signed_response_alg_default()
+}
+
 #[allow(clippy::unnecessary_wraps)]
-fn signed_response_alg_default() -> Option<JsonWebSignatureAlg> {
-    Some(JsonWebSignatureAlg::Rs256)
+fn signed_response_alg_default() -> JsonWebSignatureAlg {
+    JsonWebSignatureAlg::Rs256
+}
+
+#[allow(clippy::unnecessary_wraps)]
+fn optional_signed_response_alg_default() -> Option<JsonWebSignatureAlg> {
+    Some(signed_response_alg_default())
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
@@ -485,13 +497,12 @@ pub struct Provider {
     /// Expected signature for the JWT payload returned by the token
     /// authentication endpoint.
     ///
-    /// If null, the response is expected to be an unsigned JSON payload.
     /// Defaults to `RS256`.
     #[serde(
         default = "signed_response_alg_default",
         skip_serializing_if = "is_signed_response_alg_default"
     )]
-    pub id_token_signed_response_alg: Option<JsonWebSignatureAlg>,
+    pub id_token_signed_response_alg: JsonWebSignatureAlg,
 
     /// The scopes to request from the provider
     pub scope: String,
@@ -524,8 +535,8 @@ pub struct Provider {
     /// If null, the response is expected to be an unsigned JSON payload.
     /// Defaults to `RS256`.
     #[serde(
-        default = "signed_response_alg_default",
-        skip_serializing_if = "is_signed_response_alg_default"
+        default = "optional_signed_response_alg_default",
+        skip_serializing_if = "is_optional_signed_response_alg_default"
     )]
     pub userinfo_signed_response_alg: Option<JsonWebSignatureAlg>,
 

crates/data-model/src/upstream_oauth2/provider.rs

@@ -235,7 +235,7 @@ pub struct UpstreamOAuthProvider {
     pub encrypted_client_secret: Option<String>,
     pub token_endpoint_signing_alg: Option<JsonWebSignatureAlg>,
     pub token_endpoint_auth_method: TokenAuthMethod,
-    pub id_token_signed_response_alg: Option<JsonWebSignatureAlg>,
+    pub id_token_signed_response_alg: JsonWebSignatureAlg,
     pub response_mode: ResponseMode,
     pub created_at: DateTime<Utc>,
     pub disabled_at: Option<DateTime<Utc>>,

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;
@@ -411,7 +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: None,
+            id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
             created_at: clock.now(),
             disabled_at: None,
             claims_imports: UpstreamOAuthProviderClaimsImports::default(),

crates/handlers/src/upstream_oauth2/callback.rs

@@ -278,62 +278,53 @@ pub(crate) async fn handler(
 
     let mut context = AttributeMappingContext::new();
     if let Some(id_token) = token_response.id_token.as_ref() {
-        if let Some(signed_response_alg) = &provider.id_token_signed_response_alg {
-            jwks = Some(
-                mas_oidc_client::requests::jose::fetch_jwks(
-                    &client,
-                    lazy_metadata.jwks_uri().await?,
-                )
+        jwks = Some(
+            mas_oidc_client::requests::jose::fetch_jwks(&client, lazy_metadata.jwks_uri().await?)
                 .await?,
-            );
-
-            let id_token_verification_data = JwtVerificationData {
-                issuer: &provider.issuer,
-                jwks: &jwks.clone().unwrap(),
-                signing_algorithm: 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,
-                id_token_verification_data,
-                None,
-                clock.now(),
-            )?;
-
-            let (_headers, mut claims) = id_token.into_parts();
-
-            // Access token hash must match.
-            mas_jose::claims::AT_HASH
-                .extract_optional_with_options(
-                    &mut claims,
-                    TokenHash::new(
-                        id_token_verification_data.signing_algorithm,
-                        &token_response.access_token,
-                    ),
-                )
-                .map_err(mas_oidc_client::error::IdTokenError::from)?;
+        );
 
-            // Code hash must match.
-            mas_jose::claims::C_HASH
-                .extract_optional_with_options(
-                    &mut claims,
-                    TokenHash::new(id_token_verification_data.signing_algorithm, &code),
-                )
-                .map_err(mas_oidc_client::error::IdTokenError::from)?;
-
-            // Nonce must match.
-            mas_jose::claims::NONCE
-                .extract_required_with_options(&mut claims, session.nonce.as_str())
-                .map_err(mas_oidc_client::error::IdTokenError::from)?;
-
-            context = context.with_id_token_claims(claims);
-        } else {
-            let claims = serde_json::from_str(id_token)
-                .map_err(mas_oidc_client::error::IdTokenError::from)?;
-            context = context.with_id_token_claims(claims);
-        }
+        let id_token_verification_data = JwtVerificationData {
+            issuer: &provider.issuer,
+            jwks: &jwks.clone().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,
+            id_token_verification_data,
+            None,
+            clock.now(),
+        )?;
+
+        let (_headers, mut claims) = id_token.into_parts();
+
+        // Access token hash must match.
+        mas_jose::claims::AT_HASH
+            .extract_optional_with_options(
+                &mut claims,
+                TokenHash::new(
+                    id_token_verification_data.signing_algorithm,
+                    &token_response.access_token,
+                ),
+            )
+            .map_err(mas_oidc_client::error::IdTokenError::from)?;
+
+        // Code hash must match.
+        mas_jose::claims::C_HASH
+            .extract_optional_with_options(
+                &mut claims,
+                TokenHash::new(id_token_verification_data.signing_algorithm, &code),
+            )
+            .map_err(mas_oidc_client::error::IdTokenError::from)?;
+
+        // Nonce must match.
+        mas_jose::claims::NONCE
+            .extract_required_with_options(&mut claims, session.nonce.as_str())
+            .map_err(mas_oidc_client::error::IdTokenError::from)?;
+
+        context = context.with_id_token_claims(claims);
     }
 
     if let Some(extra_callback_parameters) = extra_callback_parameters.clone() {

crates/handlers/src/upstream_oauth2/link.rs

@@ -922,7 +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: None,
+                    id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
                     client_id: "client".to_owned(),
                     encrypted_client_secret: None,
                     claims_imports,

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,7 @@ mod test {
                     scope: [OPENID].into_iter().collect(),
                     token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
                     token_endpoint_signing_alg: None,
-                    id_token_signed_response_alg: None,
+                    id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
                     fetch_userinfo: false,
                     userinfo_signed_response_alg: None,
                     client_id: "client".to_owned(),
@@ -443,7 +444,7 @@ mod test {
                     scope: [OPENID].into_iter().collect(),
                     token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
                     token_endpoint_signing_alg: None,
-                    id_token_signed_response_alg: None,
+                    id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
                     fetch_userinfo: false,
                     userinfo_signed_response_alg: None,
                     client_id: "client".to_owned(),

crates/oidc-client/src/error.rs

@@ -200,10 +200,6 @@ pub enum IdTokenError {
     /// one we got before.
     #[error("wrong authentication time")]
     WrongAuthTime,
-
-    #[error(transparent)]
-    /// TODO
-    Deserialize(#[from] serde_json::Error),
 }
 
 /// All errors that can occur when adding client credentials to the request.

crates/storage-pg/migrations/20241202123523_upstream_oauth_responses_alg.sql

@@ -6,5 +6,5 @@
 -- 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 DEFAULT 'RS256',
+  ADD COLUMN "id_token_signed_response_alg" TEXT NOT NULL DEFAULT 'RS256',
   ADD COLUMN "userinfo_signed_response_alg" TEXT DEFAULT 'RS256';

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,7 @@ mod tests {
                     brand_name: None,
                     scope: Scope::from_iter([OPENID]),
                     token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
-                    id_token_signed_response_alg: None,
+                    id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
                     fetch_userinfo: false,
                     userinfo_signed_response_alg: None,
                     token_endpoint_signing_alg: None,
@@ -309,7 +310,7 @@ mod tests {
                         fetch_userinfo: false,
                         userinfo_signed_response_alg: None,
                         token_endpoint_signing_alg: None,
-                        id_token_signed_response_alg: None,
+                        id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
                         client_id,
                         encrypted_client_secret: None,
                         claims_imports: UpstreamOAuthProviderClaimsImports::default(),

crates/storage-pg/src/upstream_oauth2/provider.rs

@@ -56,7 +56,7 @@ struct ProviderLookup {
     encrypted_client_secret: Option<String>,
     token_endpoint_signing_alg: Option<String>,
     token_endpoint_auth_method: String,
-    id_token_signed_response_alg: Option<String>,
+    id_token_signed_response_alg: String,
     fetch_userinfo: bool,
     userinfo_signed_response_alg: Option<String>,
     created_at: DateTime<Utc>,
@@ -100,11 +100,8 @@ impl TryFrom<ProviderLookup> for UpstreamOAuthProvider {
                     .row(id)
                     .source(e)
             })?;
-        let id_token_signed_response_alg = value
-            .id_token_signed_response_alg
-            .map(|x| x.parse())
-            .transpose()
-            .map_err(|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)
@@ -349,10 +346,7 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> {
                 .token_endpoint_signing_alg
                 .as_ref()
                 .map(ToString::to_string),
-            params
-                .id_token_signed_response_alg
-                .as_ref()
-                .map(ToString::to_string),
+            params.id_token_signed_response_alg.to_string(),
             params.fetch_userinfo,
             params
                 .userinfo_signed_response_alg
@@ -558,10 +552,7 @@ impl UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'_> {
                 .token_endpoint_signing_alg
                 .as_ref()
                 .map(ToString::to_string),
-            params
-                .id_token_signed_response_alg
-                .as_ref()
-                .map(ToString::to_string),
+            params.id_token_signed_response_alg.to_string(),
             params.fetch_userinfo,
             params
                 .userinfo_signed_response_alg

crates/storage/src/upstream_oauth2/provider.rs

@@ -45,9 +45,8 @@ pub struct UpstreamOAuthProviderParams {
     /// Expected signature for the JWT payload returned by the token
     /// authentication endpoint.
     ///
-    /// If `None`, the response is expected to be an unsigned JSON payload.
     /// Defaults to `RS256`.
-    pub id_token_signed_response_alg: Option<JsonWebSignatureAlg>,
+    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

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

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,7 +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: None,
+                id_token_signed_response_alg: JsonWebSignatureAlg::Rs256,
                 client_id: "client-id".to_owned(),
                 encrypted_client_secret: None,
                 claims_imports: UpstreamOAuthProviderClaimsImports::default(),

docs/config.schema.json

@@ -1876,7 +1876,7 @@
           ]
         },
         "id_token_signed_response_alg": {
-          "description": "Expected signature for the JWT payload returned by the token authentication endpoint.\n\nIf null, the response is expected to be an unsigned JSON payload. Defaults to `RS256`.",
+          "description": "Expected signature for the JWT payload returned by the token authentication endpoint.\n\nDefaults to `RS256`.",
           "allOf": [
             {
               "$ref": "#/definitions/JsonWebSignatureAlg"