Make the id_token optional on upstream OAuth 2.0 providers

This makes it possible to use non-OIDC providers as upstream OAuth 2.0 providers, like GitHub.
This commit is contained in:
Quentin Gliech
2024-11-27 18:19:28 +01:00
parent b90dd98d0e
commit e39ea44e60
5 changed files with 81 additions and 117 deletions

View File

@@ -251,7 +251,7 @@ pub async fn config_sync(
}
if provider.jwks_uri.is_none() {
error!("Provider has discovery disabled but no JWKS URI set");
warn!("Provider has discovery disabled but no JWKS URI set");
}
}

View File

@@ -14,10 +14,9 @@ use axum_extra::response::Html;
use hyper::StatusCode;
use mas_axum_utils::{cookies::CookieJar, sentry::SentryEventID};
use mas_data_model::{UpstreamOAuthProvider, UpstreamOAuthProviderResponseMode};
use mas_jose::claims::TokenHash;
use mas_keystore::{Encrypter, Keystore};
use mas_oidc_client::requests::{
authorization_code::AuthorizationValidationData, jose::JwtVerificationData,
};
use mas_oidc_client::requests::jose::JwtVerificationData;
use mas_router::UrlBuilder;
use mas_storage::{
upstream_oauth2::{
@@ -27,7 +26,7 @@ use mas_storage::{
BoxClock, BoxRepository, BoxRng, Clock,
};
use mas_templates::{FormPostContext, Templates};
use oauth2_types::errors::ClientErrorCode;
use oauth2_types::{errors::ClientErrorCode, requests::AccessTokenRequest};
use serde::{Deserialize, Serialize};
use serde_json::json;
use thiserror::Error;
@@ -88,9 +87,6 @@ pub(crate) enum RouteError {
#[error("State parameter mismatch")]
StateMismatch,
#[error("Missing ID token")]
MissingIDToken,
#[error("Could not extract subject from ID token")]
ExtractSubject(#[source] minijinja::Error),
@@ -125,7 +121,8 @@ impl_from_error_for_route!(mas_templates::TemplateError);
impl_from_error_for_route!(mas_storage::RepositoryError);
impl_from_error_for_route!(mas_oidc_client::error::DiscoveryError);
impl_from_error_for_route!(mas_oidc_client::error::JwksError);
impl_from_error_for_route!(mas_oidc_client::error::TokenAuthorizationCodeError);
impl_from_error_for_route!(mas_oidc_client::error::TokenRequestError);
impl_from_error_for_route!(mas_oidc_client::error::IdTokenError);
impl_from_error_for_route!(mas_oidc_client::error::UserInfoError);
impl_from_error_for_route!(super::ProviderCredentialsError);
impl_from_error_for_route!(super::cookie::UpstreamSessionNotFound);
@@ -253,11 +250,6 @@ pub(crate) async fn handler(
let mut lazy_metadata = LazyProviderInfos::new(&metadata_cache, &provider, &client);
// Fetch the JWKS
let jwks =
mas_oidc_client::requests::jose::fetch_jwks(&client, lazy_metadata.jwks_uri().await?)
.await?;
// Figure out the client credentials
let client_credentials = client_credentials_for_provider(
&provider,
@@ -268,41 +260,72 @@ pub(crate) async fn handler(
let redirect_uri = url_builder.upstream_oauth_callback(provider.id);
// TODO: all that should be borrowed
let validation_data = AuthorizationValidationData {
state: session.state_str.clone(),
nonce: session.nonce.clone(),
code_challenge_verifier: session.code_challenge_verifier.clone(),
redirect_uri,
};
let token_response = mas_oidc_client::requests::token::request_access_token(
&client,
client_credentials,
lazy_metadata.token_endpoint().await?,
AccessTokenRequest::AuthorizationCode(oauth2_types::requests::AuthorizationCodeGrant {
code: code.clone(),
redirect_uri: Some(redirect_uri),
code_verifier: session.code_challenge_verifier.clone(),
}),
clock.now(),
&mut rng,
)
.await?;
let verification_data = JwtVerificationData {
issuer: &provider.issuer,
jwks: &jwks,
// TODO: make that configurable
signing_algorithm: &mas_iana::jose::JsonWebSignatureAlg::Rs256,
client_id: &provider.client_id,
};
let mut context = AttributeMappingContext::new();
if let Some(id_token) = token_response.id_token.as_ref() {
// Fetch the JWKS
let jwks =
mas_oidc_client::requests::jose::fetch_jwks(&client, lazy_metadata.jwks_uri().await?)
.await?;
let (response, id_token_map) =
mas_oidc_client::requests::authorization_code::access_token_with_authorization_code(
&client,
client_credentials,
lazy_metadata.token_endpoint().await?,
code,
validation_data,
Some(verification_data),
let verification_data = JwtVerificationData {
issuer: &provider.issuer,
jwks: &jwks,
// TODO: make that configurable
signing_algorithm: &mas_iana::jose::JsonWebSignatureAlg::Rs256,
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,
None,
clock.now(),
&mut rng,
)
.await?;
)?;
let (_header, id_token) = id_token_map
.clone()
.ok_or(RouteError::MissingIDToken)?
.into_parts();
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(
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(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);
}
let mut context = AttributeMappingContext::new().with_id_token_claims(id_token);
if let Some(extra_callback_parameters) = extra_callback_parameters.clone() {
context = context.with_extra_callback_parameters(extra_callback_parameters);
}
@@ -312,9 +335,8 @@ pub(crate) async fn handler(
mas_oidc_client::requests::userinfo::fetch_userinfo(
&client,
lazy_metadata.userinfo_endpoint().await?,
response.access_token.as_str(),
Some(verification_data),
&id_token_map.ok_or(RouteError::MissingIDToken)?,
token_response.access_token.as_str(),
None,
)
.await?
))
@@ -364,7 +386,7 @@ pub(crate) async fn handler(
&clock,
session,
&link,
response.id_token,
token_response.id_token,
extra_callback_parameters,
userinfo,
)

View File

@@ -7,7 +7,9 @@
//! Requests for the Token endpoint.
use chrono::{DateTime, Utc};
use http::header::ACCEPT;
use mas_http::RequestBuilderExt;
use mime::APPLICATION_JSON;
use oauth2_types::requests::{AccessTokenRequest, AccessTokenResponse};
use rand::Rng;
use url::Url;
@@ -48,7 +50,9 @@ pub async fn request_access_token(
) -> Result<AccessTokenResponse, TokenRequestError> {
tracing::debug!(?request, "Requesting access token...");
let token_request = http_client.post(token_endpoint.as_str());
let token_request = http_client
.post(token_endpoint.as_str())
.header(ACCEPT, APPLICATION_JSON.as_ref());
let token_response = client_credentials
.authenticated_form(token_request, &request, now, rng)?

View File

@@ -13,7 +13,6 @@ use std::collections::HashMap;
use headers::{ContentType, HeaderMapExt, HeaderValue};
use http::header::ACCEPT;
use mas_http::RequestBuilderExt;
use mas_jose::claims;
use mime::Mime;
use serde_json::Value;
use url::Url;
@@ -22,7 +21,6 @@ use super::jose::JwtVerificationData;
use crate::{
error::{IdTokenError, ResponseExt, UserInfoError},
requests::jose::verify_signed_jwt,
types::IdToken,
};
/// Obtain information about an authenticated end-user.
@@ -59,7 +57,6 @@ pub async fn fetch_userinfo(
userinfo_endpoint: &Url,
access_token: &str,
jwt_verification_data: Option<JwtVerificationData<'_>>,
auth_id_token: &IdToken<'_>,
) -> Result<HashMap<String, Value>, UserInfoError> {
tracing::debug!("Obtaining user info…");
@@ -94,7 +91,7 @@ pub async fn fetch_userinfo(
});
}
let mut claims = if let Some(verification_data) = jwt_verification_data {
let claims = if let Some(verification_data) = jwt_verification_data {
let response_body = userinfo_response.text().await?;
verify_signed_jwt(&response_body, verification_data)
.map_err(IdTokenError::from)?
@@ -104,18 +101,5 @@ pub async fn fetch_userinfo(
userinfo_response.json().await?
};
let mut auth_claims = auth_id_token.payload().clone();
// Subject identifier must always be the same.
let sub = claims::SUB
.extract_required(&mut claims)
.map_err(IdTokenError::from)?;
let auth_sub = claims::SUB
.extract_required(&mut auth_claims)
.map_err(IdTokenError::from)?;
if sub != auth_sub {
return Err(IdTokenError::WrongSubjectIdentifier.into());
}
Ok(claims)
}

View File

@@ -4,24 +4,19 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.
use assert_matches::assert_matches;
use mas_oidc_client::{
error::{IdTokenError, UserInfoError},
requests::userinfo::fetch_userinfo,
};
use mas_oidc_client::requests::userinfo::fetch_userinfo;
use serde_json::json;
use wiremock::{
matchers::{header, method, path},
Mock, ResponseTemplate,
};
use crate::{id_token, init_test, ACCESS_TOKEN, SUBJECT_IDENTIFIER};
use crate::{init_test, ACCESS_TOKEN, SUBJECT_IDENTIFIER};
#[tokio::test]
async fn pass_fetch_userinfo() {
let (http_client, mock_server, issuer) = init_test().await;
let userinfo_endpoint = issuer.join("userinfo").unwrap();
let (auth_id_token, _) = id_token(issuer.as_str());
Mock::given(method("GET"))
.and(path("/userinfo"))
@@ -36,50 +31,9 @@ async fn pass_fetch_userinfo() {
.mount(&mock_server)
.await;
let claims = fetch_userinfo(
&http_client,
&userinfo_endpoint,
ACCESS_TOKEN,
None,
&auth_id_token,
)
.await
.unwrap();
let claims = fetch_userinfo(&http_client, &userinfo_endpoint, ACCESS_TOKEN, None)
.await
.unwrap();
assert_eq!(claims.get("email").unwrap(), "janedoe@example.com");
}
#[tokio::test]
async fn fail_wrong_subject_identifier() {
let (http_client, mock_server, issuer) = init_test().await;
let userinfo_endpoint = issuer.join("userinfo").unwrap();
let (auth_id_token, _) = id_token(issuer.as_str());
Mock::given(method("GET"))
.and(path("/userinfo"))
.and(header(
"authorization",
format!("Bearer {ACCESS_TOKEN}").as_str(),
))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"sub": "wrong_subject_identifier",
"email": "janedoe@example.com",
})))
.mount(&mock_server)
.await;
let error = fetch_userinfo(
&http_client,
&userinfo_endpoint,
ACCESS_TOKEN,
None,
&auth_id_token,
)
.await
.unwrap_err();
assert_matches!(
error,
UserInfoError::IdToken(IdTokenError::WrongSubjectIdentifier)
);
}