From 3b29a3e7f3b2328f58b1b56b63a78382237d2d08 Mon Sep 17 00:00:00 2001 From: Eugene Date: Thu, 24 Oct 2024 23:18:28 +0200 Subject: [PATCH] fixed #929 - sso: broken `additional_trusted_audiences` config option --- warpgate-sso/src/request.rs | 94 +++++------------------------ warpgate-sso/src/sso.rs | 115 ++++++++++++++++++++++++++++++++---- 2 files changed, 121 insertions(+), 88 deletions(-) diff --git a/warpgate-sso/src/request.rs b/warpgate-sso/src/request.rs index 3d98de0f6..2e64332d8 100644 --- a/warpgate-sso/src/request.rs +++ b/warpgate-sso/src/request.rs @@ -1,23 +1,9 @@ -use futures::future::OptionFuture; -use openidconnect::core::{CoreGenderClaim, CoreIdToken}; -use openidconnect::reqwest::async_http_client; use openidconnect::url::Url; -use openidconnect::{ - AccessTokenHash, AdditionalClaims, AuthorizationCode, CsrfToken, Nonce, OAuth2TokenResponse, - PkceCodeVerifier, RedirectUrl, RequestTokenError, TokenResponse, UserInfoClaims, -}; +use openidconnect::{CsrfToken, Nonce, PkceCodeVerifier, RedirectUrl}; use serde::{Deserialize, Serialize}; -use tracing::{debug, error}; +use tracing::debug; -#[derive(Debug, Deserialize, Serialize, Clone)] -struct WarpgateClaims { - // This uses the "warpgate_roles" claim from OIDC - warpgate_roles: Option>, -} - -impl AdditionalClaims for WarpgateClaims {} - -use crate::{make_client, SsoError, SsoInternalProviderConfig, SsoLoginResponse}; +use crate::{SsoClient, SsoError, SsoInternalProviderConfig, SsoLoginResponse}; #[derive(Serialize, Deserialize, Debug)] pub struct SsoLoginRequest { @@ -39,70 +25,20 @@ impl SsoLoginRequest { } pub async fn verify_code(self, code: String) -> Result { - let client = make_client(&self.config) - .await? - .set_redirect_uri(self.redirect_url.clone()); - - let mut req = client.exchange_code(AuthorizationCode::new(code)); - if let Some(verifier) = self.pkce_verifier { - req = req.set_pkce_verifier(verifier); - } - - let token_response = req - .request_async(async_http_client) - .await - .map_err(|e| match e { - RequestTokenError::ServerResponse(response) => { - SsoError::Verification(response.error().to_string()) - } - RequestTokenError::Parse(err, path) => SsoError::Verification(format!( - "Parse error: {:?} / {:?}", - err, - String::from_utf8_lossy(&path) - )), - e => SsoError::Verification(format!("{e}")), - })?; - - let id_token: &CoreIdToken = token_response.id_token().ok_or(SsoError::NotOidc)?; - let claims = id_token.claims(&client.id_token_verifier(), &self.nonce)?; - - let user_info_req = client - .user_info(token_response.access_token().to_owned(), None) - .map_err(|err| { - error!("Failed to fetch userinfo: {err:?}"); - err - }) - .ok(); - - let userinfo_claims: Option> = - OptionFuture::from(user_info_req.map(|req| req.request_async(async_http_client))) - .await - .and_then(|res| { - res.map_err(|err| { - error!("Failed to fetch userinfo: {err:?}"); - err - }) - .ok() - }); - - if let Some(expected_access_token_hash) = claims.access_token_hash() { - let actual_access_token_hash = AccessTokenHash::from_token( - token_response.access_token(), - &id_token.signing_alg()?, - )?; - if actual_access_token_hash != *expected_access_token_hash { - return Err(SsoError::Mitm); - } - } + // + let result = SsoClient::new(self.config) + .finish_login(self.pkce_verifier, self.redirect_url, &self.nonce, code) + .await?; - debug!("OIDC claims: {:?}", claims); - debug!("OIDC userinfo claims: {:?}", userinfo_claims); + debug!("OIDC claims: {:?}", result.claims); + debug!("OIDC userinfo claims: {:?}", result.userinfo_claims); macro_rules! get_claim { ($method:ident) => { - claims + result + .claims .$method() - .or(userinfo_claims.as_ref().and_then(|x| x.$method())) + .or(result.userinfo_claims.as_ref().and_then(|x| x.$method())) }; } @@ -118,9 +54,11 @@ impl SsoLoginRequest { email_verified: get_claim!(email_verified), - groups: userinfo_claims.and_then(|x| x.additional_claims().warpgate_roles.clone()), + groups: result + .userinfo_claims + .and_then(|x| x.additional_claims().warpgate_roles.clone()), - id_token: id_token.clone(), + id_token: result.token.clone(), }) } } diff --git a/warpgate-sso/src/sso.rs b/warpgate-sso/src/sso.rs index 688153441..fd08a07c5 100644 --- a/warpgate-sso/src/sso.rs +++ b/warpgate-sso/src/sso.rs @@ -1,18 +1,39 @@ use std::borrow::Cow; use std::ops::Deref; -use openidconnect::core::{CoreAuthenticationFlow, CoreClient, CoreIdToken}; +use futures::future::OptionFuture; +use openidconnect::core::{ + CoreAuthenticationFlow, CoreClient, CoreGenderClaim, CoreIdToken, CoreIdTokenClaims, +}; use openidconnect::reqwest::async_http_client; use openidconnect::url::Url; use openidconnect::{ - CsrfToken, DiscoveryError, LogoutRequest, Nonce, PkceCodeChallenge, PostLogoutRedirectUrl, - ProviderMetadataWithLogout, RedirectUrl, Scope, + AccessTokenHash, AdditionalClaims, AuthorizationCode, CsrfToken, DiscoveryError, LogoutRequest, + Nonce, OAuth2TokenResponse, PkceCodeChallenge, PkceCodeVerifier, PostLogoutRedirectUrl, + ProviderMetadataWithLogout, RedirectUrl, RequestTokenError, Scope, TokenResponse, + UserInfoClaims, }; +use serde::{Deserialize, Serialize}; +use tracing::error; use crate::config::SsoInternalProviderConfig; use crate::request::SsoLoginRequest; use crate::SsoError; +#[derive(Debug, Deserialize, Serialize, Clone)] +pub struct WarpgateClaims { + // This uses the "warpgate_roles" claim from OIDC + pub warpgate_roles: Option>, +} + +impl AdditionalClaims for WarpgateClaims {} + +pub struct SsoResult { + pub token: CoreIdToken, + pub claims: CoreIdTokenClaims, + pub userinfo_claims: Option>, +} + pub struct SsoClient { config: SsoInternalProviderConfig, } @@ -30,7 +51,7 @@ pub async fn discover_metadata( }) } -pub async fn make_client(config: &SsoInternalProviderConfig) -> Result { +async fn make_client(config: &SsoInternalProviderConfig) -> Result { let metadata = discover_metadata(config).await?; let client = CoreClient::from_provider_metadata( @@ -40,12 +61,6 @@ pub async fn make_client(config: &SsoInternalProviderConfig) -> Result, + redirect_url: RedirectUrl, + nonce: &Nonce, + code: String, + ) -> Result { + let client = make_client(&self.config) + .await? + .set_redirect_uri(redirect_url); + + let mut req = client.exchange_code(AuthorizationCode::new(code)); + if let Some(verifier) = pkce_verifier { + req = req.set_pkce_verifier(verifier); + } + + let token_response = req + .request_async(async_http_client) + .await + .map_err(|e| match e { + RequestTokenError::ServerResponse(response) => { + SsoError::Verification(response.error().to_string()) + } + RequestTokenError::Parse(err, path) => SsoError::Verification(format!( + "Parse error: {:?} / {:?}", + err, + String::from_utf8_lossy(&path) + )), + e => SsoError::Verification(format!("{e}")), + })?; + + let mut token_verifier = client.id_token_verifier(); + dbg!(self.config.additional_trusted_audiences()); + + if let Some(trusted_audiences) = self.config.additional_trusted_audiences() { + token_verifier = token_verifier + .set_other_audience_verifier_fn(|aud| { + dbg!(aud); + trusted_audiences.contains(aud.deref())}); + } + + let id_token: &CoreIdToken = token_response.id_token().ok_or(SsoError::NotOidc)?; + let claims = id_token.claims(&token_verifier, nonce)?; + + let user_info_req = client + .user_info(token_response.access_token().to_owned(), None) + .map_err(|err| { + error!("Failed to fetch userinfo: {err:?}"); + err + }) + .ok(); + + let userinfo_claims: Option> = + OptionFuture::from(user_info_req.map(|req| req.request_async(async_http_client))) + .await + .and_then(|res| { + res.map_err(|err| { + error!("Failed to fetch userinfo: {err:?}"); + err + }) + .ok() + }); + + if let Some(expected_access_token_hash) = claims.access_token_hash() { + let actual_access_token_hash = AccessTokenHash::from_token( + token_response.access_token(), + &id_token.signing_alg()?, + )?; + if actual_access_token_hash != *expected_access_token_hash { + return Err(SsoError::Mitm); + } + } + + Ok(SsoResult { + token: id_token.clone(), + userinfo_claims, + claims: claims.clone(), + }) + } + pub async fn logout(&self, token: CoreIdToken, redirect_url: Url) -> Result { let metadata = discover_metadata(&self.config).await?; let Some(ref url) = metadata.additional_metadata().end_session_endpoint else {