fix: fetch OIDC discovery before building authorization URL (#91)
get_authorize_url() was synchronous and fell back to constructing
{issuer}/authorize when the discovery cache was empty. This produced
incorrect URLs for providers like Keycloak whose authorization
endpoint is {issuer}/protocol/openid-connect/auth.
Made get_authorize_url() async so it can call get_discovery() to
fetch the real authorization_endpoint from .well-known/openid-configuration
before the first redirect. The discovery document is cached after the
initial fetch.
This commit is contained in:
@@ -138,7 +138,8 @@ pub struct OidcIdClaims {
|
|||||||
pub trait OidcServicePort: Send + Sync + 'static {
|
pub trait OidcServicePort: Send + Sync + 'static {
|
||||||
/// Get the authorization URL for redirecting the user to the IdP.
|
/// Get the authorization URL for redirecting the user to the IdP.
|
||||||
/// Includes PKCE code_challenge (S256) and nonce for ID token binding.
|
/// Includes PKCE code_challenge (S256) and nonce for ID token binding.
|
||||||
fn get_authorize_url(&self, state: &str, nonce: &str, pkce_challenge: &str) -> Result<String, DomainError>;
|
/// This is async because it may need to fetch the OIDC discovery document.
|
||||||
|
async fn get_authorize_url(&self, state: &str, nonce: &str, pkce_challenge: &str) -> Result<String, DomainError>;
|
||||||
|
|
||||||
/// Exchange an authorization code for tokens, providing PKCE code_verifier.
|
/// Exchange an authorization code for tokens, providing PKCE code_verifier.
|
||||||
async fn exchange_code(&self, code: &str, pkce_verifier: &str) -> Result<OidcTokenSet, DomainError>;
|
async fn exchange_code(&self, code: &str, pkce_verifier: &str) -> Result<OidcTokenSet, DomainError>;
|
||||||
|
|||||||
@@ -785,7 +785,7 @@ impl AuthApplicationService {
|
|||||||
|
|
||||||
/// Prepare the OIDC authorization flow: generates CSRF state, PKCE pair,
|
/// Prepare the OIDC authorization flow: generates CSRF state, PKCE pair,
|
||||||
/// nonce, stores them in pending_oidc_flows, and returns the authorize URL.
|
/// nonce, stores them in pending_oidc_flows, and returns the authorize URL.
|
||||||
pub fn prepare_oidc_authorize(&self) -> Result<String, DomainError> {
|
pub async fn prepare_oidc_authorize(&self) -> Result<String, DomainError> {
|
||||||
let oidc = self.oidc_service().ok_or_else(|| DomainError::new(
|
let oidc = self.oidc_service().ok_or_else(|| DomainError::new(
|
||||||
ErrorKind::InternalError, "OIDC", "OIDC service not configured",
|
ErrorKind::InternalError, "OIDC", "OIDC service not configured",
|
||||||
))?;
|
))?;
|
||||||
@@ -826,7 +826,7 @@ impl AuthApplicationService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Build authorization URL with state, nonce, and PKCE challenge
|
// Build authorization URL with state, nonce, and PKCE challenge
|
||||||
let authorize_url = oidc.get_authorize_url(&state_token, &nonce, &pkce_challenge)?;
|
let authorize_url = oidc.get_authorize_url(&state_token, &nonce, &pkce_challenge).await?;
|
||||||
|
|
||||||
tracing::info!("OIDC authorize flow prepared (state={}...)", &state_token[..8]);
|
tracing::info!("OIDC authorize flow prepared (state={}...)", &state_token[..8]);
|
||||||
|
|
||||||
|
|||||||
@@ -238,22 +238,10 @@ impl OidcService {
|
|||||||
|
|
||||||
#[async_trait]
|
#[async_trait]
|
||||||
impl OidcServicePort for OidcService {
|
impl OidcServicePort for OidcService {
|
||||||
fn get_authorize_url(&self, state: &str, nonce: &str, pkce_challenge: &str) -> Result<String, DomainError> {
|
async fn get_authorize_url(&self, state: &str, nonce: &str, pkce_challenge: &str) -> Result<String, DomainError> {
|
||||||
// We need the authorization_endpoint. If not cached, we'll construct it from issuer.
|
// Fetch or use cached discovery to get the correct authorization_endpoint
|
||||||
// In practice, the discovery should be pre-fetched during startup.
|
let discovery = self.get_discovery().await?;
|
||||||
let auth_endpoint = {
|
let auth_endpoint = discovery.authorization_endpoint;
|
||||||
let cache = self.discovery.read().map_err(|_| DomainError::new(
|
|
||||||
ErrorKind::InternalError, "OIDC", "Lock poisoned",
|
|
||||||
))?;
|
|
||||||
match &*cache {
|
|
||||||
Some(disc) => disc.authorization_endpoint.clone(),
|
|
||||||
None => {
|
|
||||||
// Fallback: construct typical endpoint
|
|
||||||
let issuer = self.config.issuer_url.trim_end_matches('/');
|
|
||||||
format!("{}/authorize", issuer)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
let scopes = self.config.scopes.replace(',', " ");
|
let scopes = self.config.scopes.replace(',', " ");
|
||||||
let url = format!(
|
let url = format!(
|
||||||
|
|||||||
@@ -334,7 +334,7 @@ async fn oidc_authorize(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Prepare OIDC authorization flow (generates CSRF state, PKCE pair, nonce)
|
// Prepare OIDC authorization flow (generates CSRF state, PKCE pair, nonce)
|
||||||
let authorize_url = auth_app.prepare_oidc_authorize()?;
|
let authorize_url = auth_app.prepare_oidc_authorize().await?;
|
||||||
|
|
||||||
tracing::info!("OIDC authorize redirect generated");
|
tracing::info!("OIDC authorize redirect generated");
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user