Skip to content

Commit

Permalink
Add id_token_signed_response_alg and userinfo_signed_response_alg (#3664
Browse files Browse the repository at this point in the history
)
  • Loading branch information
MatMaul authored Dec 17, 2024
1 parent 0fafef3 commit 80903ed
Show file tree
Hide file tree
Showing 22 changed files with 338 additions and 131 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions crates/cli/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
Expand Down
28 changes: 28 additions & 0 deletions crates/config/src/sections/upstream_oauth2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -472,6 +482,16 @@ pub struct Provider {
#[serde(skip_serializing_if = "Option::is_none")]
pub token_endpoint_auth_signing_alg: Option<JsonWebSignatureAlg>,

/// 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,

Expand All @@ -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<JsonWebSignatureAlg>,

/// The URL to use for the provider's authorization endpoint
///
/// Defaults to the `authorization_endpoint` provided through discovery
Expand Down
2 changes: 2 additions & 0 deletions crates/data-model/src/upstream_oauth2/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,12 @@ pub struct UpstreamOAuthProvider {
pub token_endpoint_override: Option<Url>,
pub userinfo_endpoint_override: Option<Url>,
pub fetch_userinfo: bool,
pub userinfo_signed_response_alg: Option<JsonWebSignatureAlg>,
pub client_id: String,
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: JsonWebSignatureAlg,
pub response_mode: ResponseMode,
pub created_at: DateTime<Utc>,
pub disabled_at: Option<DateTime<Utc>>,
Expand Down
3 changes: 3 additions & 0 deletions crates/handlers/src/upstream_oauth2/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]),
Expand All @@ -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(),
Expand Down
66 changes: 47 additions & 19 deletions crates/handlers/src/upstream_oauth2/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)?;
Expand All @@ -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,
),
)
Expand All @@ -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)?;

Expand All @@ -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
};
Expand Down
2 changes: 2 additions & 0 deletions crates/handlers/src/upstream_oauth2/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,13 +922,15 @@ 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,
authorization_endpoint_override: None,
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,
Expand Down
5 changes: 5 additions & 0 deletions crates/handlers/src/views/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 80903ed

Please sign in to comment.