Skip to content

Commit 9626af1

Browse files
committed
Add id_token_signed_response_alg and userinfo_signed_response_alg
1 parent 55c9b60 commit 9626af1

21 files changed

+397
-169
lines changed

crates/cli/src/sync.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,9 +280,8 @@ pub async fn config_sync(
280280
brand_name: provider.brand_name,
281281
scope: provider.scope.parse()?,
282282
token_endpoint_auth_method,
283-
token_endpoint_signing_alg: provider
284-
.token_endpoint_auth_signing_alg
285-
.clone(),
283+
token_endpoint_signing_alg: provider.token_endpoint_auth_signing_alg,
284+
id_token_signed_response_alg: provider.id_token_signed_response_alg,
286285
client_id: provider.client_id,
287286
encrypted_client_secret,
288287
claims_imports: map_claims_imports(&provider.claims_imports),
@@ -293,6 +292,7 @@ pub async fn config_sync(
293292
discovery_mode,
294293
pkce_mode,
295294
fetch_userinfo: provider.fetch_userinfo,
295+
userinfo_signed_response_alg: provider.userinfo_signed_response_alg,
296296
response_mode,
297297
additional_authorization_parameters: provider
298298
.additional_authorization_parameters

crates/config/src/sections/upstream_oauth2.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,16 @@ fn is_default_true(value: &bool) -> bool {
398398
*value
399399
}
400400

401+
#[allow(clippy::ref_option)]
402+
fn is_signed_response_alg_default(signed_response_alg: &Option<JsonWebSignatureAlg>) -> bool {
403+
*signed_response_alg == signed_response_alg_default()
404+
}
405+
406+
#[allow(clippy::unnecessary_wraps)]
407+
fn signed_response_alg_default() -> Option<JsonWebSignatureAlg> {
408+
Some(JsonWebSignatureAlg::Rs256)
409+
}
410+
401411
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
402412
pub struct SignInWithApple {
403413
/// The private key used to sign the `id_token`
@@ -472,6 +482,17 @@ pub struct Provider {
472482
#[serde(skip_serializing_if = "Option::is_none")]
473483
pub token_endpoint_auth_signing_alg: Option<JsonWebSignatureAlg>,
474484

485+
/// Expected signature for the JWT payload returned by the token
486+
/// authentication endpoint.
487+
///
488+
/// If null, the response is expected to be an unsigned JSON payload.
489+
/// Defaults to `RS256`.
490+
#[serde(
491+
default = "signed_response_alg_default",
492+
skip_serializing_if = "is_signed_response_alg_default"
493+
)]
494+
pub id_token_signed_response_alg: Option<JsonWebSignatureAlg>,
495+
475496
/// The scopes to request from the provider
476497
pub scope: String,
477498

@@ -497,6 +518,17 @@ pub struct Provider {
497518
#[serde(default)]
498519
pub fetch_userinfo: bool,
499520

521+
/// Expected signature for the JWT payload returned by the userinfo
522+
/// endpoint.
523+
///
524+
/// If null, the response is expected to be an unsigned JSON payload.
525+
/// Defaults to `RS256`.
526+
#[serde(
527+
default = "signed_response_alg_default",
528+
skip_serializing_if = "is_signed_response_alg_default"
529+
)]
530+
pub userinfo_signed_response_alg: Option<JsonWebSignatureAlg>,
531+
500532
/// The URL to use for the provider's authorization endpoint
501533
///
502534
/// Defaults to the `authorization_endpoint` provided through discovery

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,12 @@ pub struct UpstreamOAuthProvider {
230230
pub token_endpoint_override: Option<Url>,
231231
pub userinfo_endpoint_override: Option<Url>,
232232
pub fetch_userinfo: bool,
233+
pub userinfo_signed_response_alg: Option<JsonWebSignatureAlg>,
233234
pub client_id: String,
234235
pub encrypted_client_secret: Option<String>,
235236
pub token_endpoint_signing_alg: Option<JsonWebSignatureAlg>,
236237
pub token_endpoint_auth_method: TokenAuthMethod,
238+
pub id_token_signed_response_alg: Option<JsonWebSignatureAlg>,
237239
pub response_mode: ResponseMode,
238240
pub created_at: DateTime<Utc>,
239241
pub disabled_at: Option<DateTime<Utc>>,

crates/handlers/src/upstream_oauth2/cache.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ mod tests {
400400
discovery_mode: UpstreamOAuthProviderDiscoveryMode::Insecure,
401401
pkce_mode: UpstreamOAuthProviderPkceMode::Auto,
402402
fetch_userinfo: false,
403+
userinfo_signed_response_alg: None,
403404
jwks_uri_override: None,
404405
authorization_endpoint_override: None,
405406
scope: Scope::from_iter([OPENID]),
@@ -410,6 +411,7 @@ mod tests {
410411
token_endpoint_signing_alg: None,
411412
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
412413
response_mode: mas_data_model::UpstreamOAuthProviderResponseMode::Query,
414+
id_token_signed_response_alg: None,
413415
created_at: clock.now(),
414416
disabled_at: None,
415417
claims_imports: UpstreamOAuthProviderClaimsImports::default(),

crates/handlers/src/upstream_oauth2/callback.rs

Lines changed: 94 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -274,72 +274,109 @@ pub(crate) async fn handler(
274274
)
275275
.await?;
276276

277+
let mut jwks = None;
278+
277279
let mut context = AttributeMappingContext::new();
278280
if let Some(id_token) = token_response.id_token.as_ref() {
279-
// Fetch the JWKS
280-
let jwks =
281-
mas_oidc_client::requests::jose::fetch_jwks(&client, lazy_metadata.jwks_uri().await?)
282-
.await?;
283-
284-
let verification_data = JwtVerificationData {
285-
issuer: &provider.issuer,
286-
jwks: &jwks,
287-
// TODO: make that configurable
288-
signing_algorithm: &mas_iana::jose::JsonWebSignatureAlg::Rs256,
289-
client_id: &provider.client_id,
290-
};
291-
292-
// Decode and verify the ID token
293-
let id_token = mas_oidc_client::requests::jose::verify_id_token(
294-
id_token,
295-
verification_data,
296-
None,
297-
clock.now(),
298-
)?;
299-
300-
let (_headers, mut claims) = id_token.into_parts();
301-
302-
// Access token hash must match.
303-
mas_jose::claims::AT_HASH
304-
.extract_optional_with_options(
305-
&mut claims,
306-
TokenHash::new(
307-
verification_data.signing_algorithm,
308-
&token_response.access_token,
309-
),
310-
)
311-
.map_err(mas_oidc_client::error::IdTokenError::from)?;
312-
313-
// Code hash must match.
314-
mas_jose::claims::C_HASH
315-
.extract_optional_with_options(
316-
&mut claims,
317-
TokenHash::new(verification_data.signing_algorithm, &code),
318-
)
319-
.map_err(mas_oidc_client::error::IdTokenError::from)?;
320-
321-
// Nonce must match.
322-
mas_jose::claims::NONCE
323-
.extract_required_with_options(&mut claims, session.nonce.as_str())
324-
.map_err(mas_oidc_client::error::IdTokenError::from)?;
325-
326-
context = context.with_id_token_claims(claims);
281+
if let Some(signed_response_alg) = &provider.id_token_signed_response_alg {
282+
jwks = Some(
283+
mas_oidc_client::requests::jose::fetch_jwks(
284+
&client,
285+
lazy_metadata.jwks_uri().await?,
286+
)
287+
.await?,
288+
);
289+
290+
let id_token_verification_data = JwtVerificationData {
291+
issuer: &provider.issuer,
292+
jwks: &jwks.clone().unwrap(),
293+
signing_algorithm: &signed_response_alg,
294+
client_id: &provider.client_id,
295+
};
296+
297+
// Decode and verify the ID token
298+
let id_token = mas_oidc_client::requests::jose::verify_id_token(
299+
id_token,
300+
id_token_verification_data,
301+
None,
302+
clock.now(),
303+
)?;
304+
305+
let (_headers, mut claims) = id_token.into_parts();
306+
307+
// Access token hash must match.
308+
mas_jose::claims::AT_HASH
309+
.extract_optional_with_options(
310+
&mut claims,
311+
TokenHash::new(
312+
id_token_verification_data.signing_algorithm,
313+
&token_response.access_token,
314+
),
315+
)
316+
.map_err(mas_oidc_client::error::IdTokenError::from)?;
317+
318+
// Code hash must match.
319+
mas_jose::claims::C_HASH
320+
.extract_optional_with_options(
321+
&mut claims,
322+
TokenHash::new(id_token_verification_data.signing_algorithm, &code),
323+
)
324+
.map_err(mas_oidc_client::error::IdTokenError::from)?;
325+
326+
// Nonce must match.
327+
mas_jose::claims::NONCE
328+
.extract_required_with_options(&mut claims, session.nonce.as_str())
329+
.map_err(mas_oidc_client::error::IdTokenError::from)?;
330+
331+
context = context.with_id_token_claims(claims);
332+
} else {
333+
let claims = serde_json::from_str(&id_token)
334+
.map_err(mas_oidc_client::error::IdTokenError::from)?;
335+
context = context.with_id_token_claims(claims);
336+
}
327337
}
328338

329339
if let Some(extra_callback_parameters) = extra_callback_parameters.clone() {
330340
context = context.with_extra_callback_parameters(extra_callback_parameters);
331341
}
332342

333343
let userinfo = if provider.fetch_userinfo {
334-
Some(json!(
335-
mas_oidc_client::requests::userinfo::fetch_userinfo(
336-
&client,
337-
lazy_metadata.userinfo_endpoint().await?,
338-
token_response.access_token.as_str(),
339-
None,
340-
)
341-
.await?
342-
))
344+
Some(json!(match &provider.userinfo_signed_response_alg {
345+
Some(signing_algorithm) => {
346+
let jwks = match jwks {
347+
Some(jwks) => jwks,
348+
None => {
349+
mas_oidc_client::requests::jose::fetch_jwks(
350+
&client,
351+
lazy_metadata.jwks_uri().await?,
352+
)
353+
.await?
354+
}
355+
};
356+
357+
mas_oidc_client::requests::userinfo::fetch_userinfo(
358+
&client,
359+
lazy_metadata.userinfo_endpoint().await?,
360+
token_response.access_token.as_str(),
361+
Some(JwtVerificationData {
362+
issuer: &provider.issuer,
363+
jwks: &jwks,
364+
signing_algorithm: &signing_algorithm,
365+
client_id: &provider.client_id,
366+
}),
367+
)
368+
.await?
369+
}
370+
None => {
371+
mas_oidc_client::requests::userinfo::fetch_userinfo(
372+
&client,
373+
lazy_metadata.userinfo_endpoint().await?,
374+
token_response.access_token.as_str(),
375+
None,
376+
)
377+
.await?
378+
}
379+
}))
343380
} else {
344381
None
345382
};

crates/handlers/src/upstream_oauth2/link.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,13 +922,15 @@ mod tests {
922922
scope: Scope::from_iter([OPENID]),
923923
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
924924
token_endpoint_signing_alg: None,
925+
id_token_signed_response_alg: None,
925926
client_id: "client".to_owned(),
926927
encrypted_client_secret: None,
927928
claims_imports,
928929
authorization_endpoint_override: None,
929930
token_endpoint_override: None,
930931
userinfo_endpoint_override: None,
931932
fetch_userinfo: false,
933+
userinfo_signed_response_alg: None,
932934
jwks_uri_override: None,
933935
discovery_mode: mas_data_model::UpstreamOAuthProviderDiscoveryMode::Oidc,
934936
pkce_mode: mas_data_model::UpstreamOAuthProviderPkceMode::Auto,

crates/handlers/src/views/login.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,9 @@ mod test {
403403
scope: [OPENID].into_iter().collect(),
404404
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
405405
token_endpoint_signing_alg: None,
406+
id_token_signed_response_alg: None,
406407
fetch_userinfo: false,
408+
userinfo_signed_response_alg: None,
407409
client_id: "client".to_owned(),
408410
encrypted_client_secret: None,
409411
claims_imports: UpstreamOAuthProviderClaimsImports::default(),
@@ -441,7 +443,9 @@ mod test {
441443
scope: [OPENID].into_iter().collect(),
442444
token_endpoint_auth_method: UpstreamOAuthProviderTokenAuthMethod::None,
443445
token_endpoint_signing_alg: None,
446+
id_token_signed_response_alg: None,
444447
fetch_userinfo: false,
448+
userinfo_signed_response_alg: None,
445449
client_id: "client".to_owned(),
446450
encrypted_client_secret: None,
447451
claims_imports: UpstreamOAuthProviderClaimsImports::default(),

crates/oidc-client/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,10 @@ pub enum IdTokenError {
200200
/// one we got before.
201201
#[error("wrong authentication time")]
202202
WrongAuthTime,
203+
204+
#[error(transparent)]
205+
/// TODO
206+
Deserialize(#[from] serde_json::Error),
203207
}
204208

205209
/// All errors that can occur when adding client credentials to the request.

0 commit comments

Comments
 (0)