diff --git a/src/api/common.rs b/src/api/common.rs index 4ddfb602..86389bea 100644 --- a/src/api/common.rs +++ b/src/api/common.rs @@ -21,6 +21,15 @@ use crate::resource::{ types::{Domain, Project}, }; +/// Get the domain by ID or Name +/// +/// # Arguments +/// * `state` - The service state +/// * `id` - The domain ID +/// * `name` - The domain name +/// +/// # Returns +/// * `Result` - The domain object pub async fn get_domain, N: AsRef>( state: &ServiceState, id: Option, @@ -51,6 +60,14 @@ pub async fn get_domain, N: AsRef>( } } +/// Find the project referred in the scope. +/// +/// # Arguments +/// * `state` - The service state. +/// * `scope` - The scope to find the project. +/// +/// # Returns +/// The resolved project. pub async fn find_project_from_scope( state: &ServiceState, scope: &ProjectScope, diff --git a/src/api/error.rs b/src/api/error.rs index 0adca732..a1d0fc1c 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -22,7 +22,6 @@ use serde_json::json; use thiserror::Error; use tracing::error; -use crate::api::v3::federation::error::OidcError; use crate::assignment::error::AssignmentProviderError; use crate::auth::AuthenticationError; use crate::catalog::error::CatalogProviderError; @@ -46,6 +45,9 @@ pub enum KeystoneApiError { #[error("Attempted to authenticate with an unsupported method.")] AuthMethodNotSupported, + #[error("{0}.")] + BadRequest(String), + #[error("The request you have made requires authentication.")] Unauthorized, @@ -76,12 +78,11 @@ pub enum KeystoneApiError { source: AssignmentProviderError, }, - #[error(transparent)] - AuthenticationInfo { - //#[from] - source: crate::auth::AuthenticationError, - }, - + // #[error(transparent)] + // AuthenticationInfo { + // //#[from] + // source: crate::auth::AuthenticationError, + // }, #[error(transparent)] CatalogError { #[from] @@ -94,12 +95,11 @@ pub enum KeystoneApiError { source: FederationProviderError, }, - #[error(transparent)] - Oidc { - #[from] - source: OidcError, - }, - + // #[error(transparent)] + // Oidc { + // #[from] + // source: OidcError, + // }, #[error(transparent)] IdentityError { #[from] @@ -148,6 +148,9 @@ pub enum KeystoneApiError { #[error(transparent)] JsonExtractorRejection(#[from] JsonRejection), + #[error("The account is disabled for user: {0}")] + UserDisabled(String), + /// Others. #[error(transparent)] Other(#[from] eyre::Report), @@ -156,60 +159,34 @@ pub enum KeystoneApiError { impl IntoResponse for KeystoneApiError { fn into_response(self) -> Response { error!("Error happened during request processing: {:#?}", self); - //tracing::debug!("stacktrace: {}", self.backtrace()); - match self { - KeystoneApiError::Conflict(_) => ( - StatusCode::CONFLICT, - Json(json!({"error": {"code": StatusCode::CONFLICT.as_u16(), "message": self.to_string()}})), - ).into_response(), - KeystoneApiError::NotFound{..} => ( - StatusCode::NOT_FOUND, - Json(json!({"error": {"code": StatusCode::NOT_FOUND.as_u16(), "message": self.to_string()}})), - ) - .into_response(), - KeystoneApiError::Unauthorized => { - (StatusCode::UNAUTHORIZED, - Json(json!({"error": {"code": StatusCode::UNAUTHORIZED.as_u16(), "message": self.to_string()}})), - ).into_response() - } - KeystoneApiError::AuthenticationInfo{ .. } => { - (StatusCode::UNAUTHORIZED, - Json(json!({"error": {"code": StatusCode::UNAUTHORIZED.as_u16(), "message": self.to_string()}})), - ).into_response() - } - KeystoneApiError::Forbidden => { - (StatusCode::FORBIDDEN, - Json(json!({"error": {"code": StatusCode::FORBIDDEN.as_u16(), "message": self.to_string()}})), - ).into_response() - } - KeystoneApiError::InternalError(_) | KeystoneApiError::IdentityError { .. } | KeystoneApiError::ResourceError { .. } | KeystoneApiError::AssignmentError { .. } | KeystoneApiError::TokenError{..} | KeystoneApiError::Federation{..} | KeystoneApiError::Other(..) => { - (StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({"error": {"code": StatusCode::INTERNAL_SERVER_ERROR.as_u16(), "message": self.to_string()}})), - ).into_response() - } - KeystoneApiError::Oidc{ source: ref err } => { - match err { - OidcError::OpenIdConnectReqwest{..} | OidcError::OpenIdConnectConfiguration{..} => { -( - StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({"error": {"code": StatusCode::INTERNAL_SERVER_ERROR.as_u16(), "message": self.to_string()}})), - ).into_response() - } - _ => { -( - StatusCode::BAD_REQUEST, - Json(json!({"error": {"code": StatusCode::BAD_REQUEST.as_u16(), "message": self.to_string()}})), - ).into_response() - } - } - } - _ => { + + let status_code = match self { + KeystoneApiError::Conflict(_) => StatusCode::CONFLICT, + KeystoneApiError::NotFound { .. } => StatusCode::NOT_FOUND, + KeystoneApiError::BadRequest(..) => StatusCode::BAD_REQUEST, + KeystoneApiError::UserDisabled(..) => StatusCode::UNAUTHORIZED, + KeystoneApiError::Unauthorized => StatusCode::UNAUTHORIZED, + // KeystoneApiError::AuthenticationInfo { .. } => StatusCode::UNAUTHORIZED, + KeystoneApiError::Forbidden => StatusCode::FORBIDDEN, + KeystoneApiError::InternalError(_) + | KeystoneApiError::IdentityError { .. } + | KeystoneApiError::ResourceError { .. } + | KeystoneApiError::AssignmentError { .. } + | KeystoneApiError::TokenError { .. } + | KeystoneApiError::Federation { .. } + | KeystoneApiError::Other(..) => StatusCode::INTERNAL_SERVER_ERROR, + _ => // KeystoneApiError::SubjectTokenMissing | KeystoneApiError::InvalidHeader | KeystoneApiError::InvalidToken | KeystoneApiError::Token{..} | KeystoneApiError::WebAuthN{..} | KeystoneApiError::Uuid {..} | KeystoneApiError::Serde {..} | KeystoneApiError::DomainIdOrName | KeystoneApiError::ProjectIdOrName | KeystoneApiError::ProjectDomain => - (StatusCode::BAD_REQUEST, - Json(json!({"error": {"code": StatusCode::BAD_REQUEST.as_u16(), "message": self.to_string()}})), - ).into_response() + { + StatusCode::BAD_REQUEST } - } + }; + + ( + status_code, + Json(json!({"error": {"code": status_code.as_u16(), "message": self.to_string()}})), + ) + .into_response() } } @@ -323,7 +300,8 @@ impl From for KeystoneApiError { AuthenticationError::AuthenticatedInfoBuilder { source } => { KeystoneApiError::InternalError(source.to_string()) } - other => KeystoneApiError::AuthenticationInfo { source: other }, + AuthenticationError::UserDisabled(data) => KeystoneApiError::UserDisabled(data), + AuthenticationError::Unauthorized => KeystoneApiError::Unauthorized, } } } diff --git a/src/api/mod.rs b/src/api/mod.rs index db6a6041..0e4dae8c 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -11,7 +11,8 @@ // limitations under the License. // // SPDX-License-Identifier: Apache-2.0 - +//! Keystone API +//! use axum::{ http::{HeaderMap, header}, response::IntoResponse, diff --git a/src/api/v3/auth/token/mod.rs b/src/api/v3/auth/token/mod.rs index 3a993676..e186b66c 100644 --- a/src/api/v3/auth/token/mod.rs +++ b/src/api/v3/auth/token/mod.rs @@ -67,13 +67,28 @@ async fn authenticate_request( } } else if method == "token" { if let Some(token) = &req.auth.identity.token { - authenticated_info = Some( + let mut authz = state + .provider + .get_token_provider() + .authenticate_by_token(&token.id, Some(false), None) + .await?; + // Resolve the user + authz.user = Some( state .provider - .get_token_provider() - .authenticate_by_token(&token.id, Some(false), None) - .await?, + .get_identity_provider() + .get_user(&state.db, &authz.user_id) + .await + .map(|x| { + x.ok_or_else(|| KeystoneApiError::NotFound { + resource: "user".into(), + identifier: authz.user_id.clone(), + }) + })??, ); + authenticated_info = Some(authz); + + {} } } } @@ -85,6 +100,17 @@ async fn authenticate_request( }) } +/// Build the AuthZ information from the request +/// +/// # Arguments +/// +/// * `state` - The service state +/// * `req` - The Request +/// +/// # Result +/// +/// * `Ok(AuthzInfo)` - The AuthZ information +/// * `Err(KeystoneApiError)` - The error async fn get_authz_info( state: &ServiceState, req: &AuthRequest, @@ -278,6 +304,7 @@ mod tests { .user(UserResponse { id: "uid".to_string(), domain_id: "udid".into(), + enabled: true, ..Default::default() }) .build() @@ -342,9 +369,22 @@ mod tests { }, ) .returning(|_, _, _| Ok(AuthenticatedInfo::builder().user_id("uid").build().unwrap())); + let mut identity_mock = MockIdentityProvider::default(); + identity_mock + .expect_get_user() + .withf(|_, id: &'_ str| id == "uid") + .returning(|_, id: &'_ str| { + Ok(Some(UserResponse { + id: id.to_string(), + domain_id: "user_domain_id".into(), + enabled: true, + ..Default::default() + })) + }); let provider = Provider::mocked_builder() .config(config.clone()) + .identity(identity_mock) .token(token_mock) .build() .unwrap(); @@ -353,7 +393,16 @@ mod tests { Arc::new(Service::new(config, DatabaseConnection::Disconnected, provider).unwrap()); assert_eq!( - AuthenticatedInfo::builder().user_id("uid").build().unwrap(), + AuthenticatedInfo::builder() + .user_id("uid") + .user(UserResponse { + id: "uid".to_string(), + domain_id: "user_domain_id".into(), + enabled: true, + ..Default::default() + }) + .build() + .unwrap(), authenticate_request( &state, &AuthRequest { @@ -707,6 +756,7 @@ mod tests { .user(UserResponse { id: "uid".to_string(), domain_id: "udid".into(), + enabled: true, ..Default::default() }) .build() @@ -838,6 +888,7 @@ mod tests { } #[tokio::test] + #[traced_test] async fn test_post_project_disabled() { let config = Config::default(); let mut identity_mock = MockIdentityProvider::default(); @@ -849,6 +900,7 @@ mod tests { .user(UserResponse { id: "uid".to_string(), domain_id: "udid".into(), + enabled: true, ..Default::default() }) .build() diff --git a/src/api/v3/federation/auth.rs b/src/api/v3/federation/auth.rs index 54d1aca8..444557dc 100644 --- a/src/api/v3/federation/auth.rs +++ b/src/api/v3/federation/auth.rs @@ -39,111 +39,6 @@ pub(super) fn openapi_router() -> OpenApiRouter { OpenApiRouter::new().routes(routes!(post)) } -// /// Authenticate using identity provider -// #[utoipa::path( -// get, -// path = "/identity_providers/{idp_id}/auth", -// description = "Authenticate using identity provider", -// responses( -// (status = CREATED, description = "identity provider object", body = IdentityProviderAuthResponse), -// ), -// tag="identity_providers" -// )] -// #[tracing::instrument(name = "api::identity_provider_auth", level = "debug", skip(state))] -// #[debug_handler] -// pub async fn get( -// State(state): State, -// Path(idp_id): Path, -// ) -> Result { -// state -// .config -// .auth -// .methods -// .iter() -// .find(|m| *m == "openid") -// .ok_or(KeystoneApiError::AuthMethodNotSupported)?; -// -// let idp = state -// .provider -// .get_federation_provider() -// .get_identity_provider(&state.db, &idp_id) -// .await -// .map(|x| { -// x.ok_or_else(|| KeystoneApiError::NotFound { -// resource: "identity provider".into(), -// identifier: idp_id, -// }) -// })??; -// -// if let Some(discovery_url) = &idp.oidc_discovery_url { -// let http_client = reqwest::ClientBuilder::new() -// // Following redirects opens the client up to SSRF vulnerabilities. -// .redirect(reqwest::redirect::Policy::none()) -// .build() -// .map_err(OidcError::from)?; -// -// let provider_metadata = CoreProviderMetadata::discover_async( -// IssuerUrl::new(discovery_url.to_string()).map_err(OidcError::from)?, -// &http_client, -// ) -// .await -// .map_err(|err| OidcError::discovery(&err))?; -// let client = CoreClient::from_provider_metadata( -// provider_metadata, -// ClientId::new(idp.oidc_client_id.ok_or(OidcError::ClientIdRequired)?), -// idp.oidc_client_secret.map(ClientSecret::new), -// ) -// // Set the URL the user will be redirected to after the authorization process. -// .set_redirect_uri( -// RedirectUrl::new("http://localhost:8080/v3/federation/auth/callback".to_string()) -// .map_err(OidcError::from)?, -// ); -// -// let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); -// -// // Generate the full authorization URL. -// let (auth_url, csrf_token, nonce) = client -// .authorize_url( -// CoreAuthenticationFlow::AuthorizationCode, -// CsrfToken::new_random, -// Nonce::new_random, -// ) -// // Set the desired scopes. -// .add_scope(Scope::new("openid".to_string())) -// // Set the PKCE code challenge. -// .set_pkce_challenge(pkce_challenge) -// .url(); -// -// state -// .provider -// .get_federation_provider() -// .create_auth_state( -// &state.db, -// AuthState { -// state: csrf_token.secret().clone(), -// nonce: nonce.secret().clone(), -// idp_id: idp.id.clone(), -// mapping_id: String::from("kc"), -// redirect_uri: String::new(), -// pkce_verifier: pkce_verifier.into_secret(), -// expires_at: Local::now().into(), -// scope: None, -// }, -// ) -// .await?; -// -// debug!( -// "url: {:?}, csrf: {:?}, nonce: {:?}", -// auth_url, -// csrf_token.secret(), -// nonce.secret() -// ); -// return Ok((StatusCode::FOUND, [(LOCATION, &auth_url.to_string())]).into_response()); -// } -// -// Ok((StatusCode::CREATED).into_response()) -// } - /// Authenticate using identity provider #[utoipa::path( post, @@ -154,7 +49,12 @@ pub(super) fn openapi_router() -> OpenApiRouter { ), tag="identity_providers" )] -#[tracing::instrument(name = "api::identity_provider_auth", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::identity_provider_auth", + level = "debug", + skip(state), + err(Debug) +)] #[debug_handler] pub async fn post( State(state): State, diff --git a/src/api/v3/federation/error.rs b/src/api/v3/federation/error.rs index be0d17ee..3acf4d3d 100644 --- a/src/api/v3/federation/error.rs +++ b/src/api/v3/federation/error.rs @@ -13,20 +13,21 @@ // SPDX-License-Identifier: Apache-2.0 use thiserror::Error; -use tracing::error; +use tracing::{Level, error, instrument}; +use crate::api::error::KeystoneApiError; use crate::api::v3::federation::types::*; #[derive(Error, Debug)] pub enum OidcError { - #[error("discovery error")] + #[error("discovery error: {msg}")] Discovery { msg: String }, - #[error("Client without discovery is not supported")] + #[error("client without discovery is not supported")] ClientWithoutDiscoveryNotSupported, #[error( - "Federated authentication requires mapping being specified in the payload or default set on the identity provider" + "federated authentication requires mapping being specified in the payload or default set on the identity provider" )] MappingRequired, @@ -51,7 +52,7 @@ pub enum OidcError { source: openidconnect::ConfigurationError, }, - #[error("error parsing the url")] + #[error(transparent)] UrlParse { #[from] source: url::ParseError, @@ -60,11 +61,12 @@ pub enum OidcError { #[error("server did not returned an ID token")] NoToken, - #[error("Identity Provider client_id is missing")] + #[error("identity Provider client_id is missing")] ClientIdRequired, #[error("ID token does not contain user id claim {0}")] UserIdClaimRequired(String), + #[error("ID token does not contain user id claim {0}")] UserNameClaimRequired(String), #[error("can not identify resulting domain_id for the user")] @@ -88,11 +90,6 @@ pub enum OidcError { source: MappedUserDataBuilderError, }, - #[error(transparent)] - AuthenticationInfo { - #[from] - source: crate::auth::AuthenticationError, - }, #[error("Authentication expired")] AuthStateExpired, } @@ -108,39 +105,68 @@ impl OidcError { msg: fail.to_string(), } } - // pub fn url(fail: url::ParseError) -> Self { - // Self::RequestToken { - // msg: fail.to_string(), - // } - // } - - // pub fn claim_verification(fail: &T) -> Self { - // Self::ClaimVerification{msg: fail.to_string()} - // } } -//impl -// From< -// openidconnect::DiscoveryError< -// openidconnect::HttpClientError, -// >, -// > for OidcError -//{ -// fn from( -// source: openidconnect::DiscoveryError< -// openidconnect::HttpClientError, -// >, -// ) -> Self { -// Self::OidcDiscovery { -// msg: source.to_string(), -// } -// } -//} - -//impl OidcError { -// fn discovery(source: RE) -> Self { -// Self::OidcDiscovery { -// source: source.to_string(), -// } -// } -//} +/// Convert OIDC error into the [HTTP](KeystoneApiError) with the expected message +impl From for KeystoneApiError { + #[instrument(level = Level::ERROR)] + fn from(value: OidcError) -> Self { + error!("Federation error: {:#?}", value); + match value { + e @ OidcError::Discovery { .. } => { + KeystoneApiError::InternalError(e.to_string()) + } + e @ OidcError::ClientWithoutDiscoveryNotSupported => { + KeystoneApiError::InternalError(e.to_string()) + } + OidcError::MappingRequired => { + KeystoneApiError::BadRequest("Federated authentication requires mapping being specified in the payload or default set on the identity provider.".to_string()) + } + OidcError::RequestToken { msg } => { + KeystoneApiError::BadRequest(format!("Error exchanging authorization code for the authorization token: {msg}")) + } + OidcError::ClaimVerification { source } => { + KeystoneApiError::BadRequest(format!("Error in claims verification: {}", source)) + } + OidcError::OpenIdConnectReqwest { source } => { + KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {}", source)) + } + OidcError::OpenIdConnectConfiguration { source } => { + KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {}", source)) + } + OidcError::UrlParse { source } => { + KeystoneApiError::BadRequest(format!("Error in OpenIDConnect logic: {}", source)) + } + e @ OidcError::NoToken => { + KeystoneApiError::InternalError(format!("Error in OpenIDConnect logic: {}", e)) + } + OidcError::ClientIdRequired => { + KeystoneApiError::BadRequest("Identity Provider mut set `client_id`.".to_string()) + } + OidcError::UserIdClaimRequired(source) => { + KeystoneApiError::BadRequest(format!("OIDC ID token does not contain user id claim: {source}")) + } + OidcError::UserNameClaimRequired(source) => { + KeystoneApiError::BadRequest(format!("OIDC ID token does not contain user name claim: {source}")) + } + OidcError::UserDomainUnbound => { + KeystoneApiError::BadRequest("Cannot identify domain_id of the user.".to_string()) + } + OidcError::BoundSubjectMismatch{ expected, found } => { + KeystoneApiError::BadRequest(format!("OIDC Bound subject mismatches: {expected} != {found}")) + } + OidcError::BoundAudiencesMismatch{ expected, found } => { + KeystoneApiError::BadRequest(format!("OIDC Bound audiences mismatches: {expected} != {found}")) + } + OidcError::BoundClaimsMismatch{ claim, expected, found } => { + KeystoneApiError::BadRequest(format!("OIDC Bound claim {claim} mismatch: {expected} != {found}")) + } + e @ OidcError::MappedUserDataBuilder { .. } => { + KeystoneApiError::InternalError(e.to_string()) + } + OidcError::AuthStateExpired => { + KeystoneApiError::BadRequest("Authentication has expired. Please start again.".to_string()) + } + } + } +} diff --git a/src/api/v3/federation/identity_provider.rs b/src/api/v3/federation/identity_provider.rs index 8547c51c..ea5389b2 100644 --- a/src/api/v3/federation/identity_provider.rs +++ b/src/api/v3/federation/identity_provider.rs @@ -30,7 +30,6 @@ pub(super) fn openapi_router() -> OpenApiRouter { OpenApiRouter::new() .routes(routes!(list, create)) .routes(routes!(show, update, remove)) - //.routes(routes!(auth::auth)) } /// List identity providers @@ -48,7 +47,8 @@ pub(super) fn openapi_router() -> OpenApiRouter { #[tracing::instrument( name = "api::identity_provider_list", level = "debug", - skip(state, _user_auth) + skip(state, _user_auth), + err(Debug) )] async fn list( Auth(_user_auth): Auth, @@ -79,7 +79,12 @@ async fn list( ), tag="identity_providers" )] -#[tracing::instrument(name = "api::identity_provider_get", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::identity_provider_get", + level = "debug", + skip(state), + err(Debug) +)] async fn show( Auth(user_auth): Auth, Path(idp_id): Path, @@ -108,7 +113,12 @@ async fn show( ), tag="identity_providers" )] -#[tracing::instrument(name = "api::identity_provider_create", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::identity_provider_create", + level = "debug", + skip(state), + err(Debug) +)] #[debug_handler] async fn create( Auth(user_auth): Auth, @@ -136,7 +146,12 @@ async fn create( ), tag="identity_providers" )] -#[tracing::instrument(name = "api::identity_provider_update", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::identity_provider_update", + level = "debug", + skip(state), + err(Debug) +)] async fn update( Auth(user_auth): Auth, Path(idp_id): Path, @@ -164,7 +179,12 @@ async fn update( ), tag="identity_providers" )] -#[tracing::instrument(name = "api::identity_provider_delete", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::identity_provider_delete", + level = "debug", + skip(state), + err(Debug) +)] async fn remove( Auth(user_auth): Auth, Path(id): Path, diff --git a/src/api/v3/federation/mapping.rs b/src/api/v3/federation/mapping.rs index a9b91874..d7ae8659 100644 --- a/src/api/v3/federation/mapping.rs +++ b/src/api/v3/federation/mapping.rs @@ -44,7 +44,12 @@ pub(super) fn openapi_router() -> OpenApiRouter { ), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_list", level = "debug", skip(state, _user_auth))] +#[tracing::instrument( + name = "api::mapping_list", + level = "debug", + skip(state, _user_auth), + err(Debug) +)] async fn list( Auth(_user_auth): Auth, Query(query): Query, @@ -74,7 +79,13 @@ async fn list( ), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_get", level = "debug", skip(state))] +#[tracing::instrument( + name = "api::mapping_get", + level = "debug", + skip(state), + err(Debug), + err(Debug) +)] async fn show( Auth(user_auth): Auth, Path(idp_id): Path, @@ -131,7 +142,7 @@ async fn create( ), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_update", level = "debug", skip(state))] +#[tracing::instrument(name = "api::mapping_update", level = "debug", skip(state), err(Debug))] async fn update( Auth(user_auth): Auth, Path(idp_id): Path, @@ -159,7 +170,7 @@ async fn update( ), tag="mappings" )] -#[tracing::instrument(name = "api::mapping_delete", level = "debug", skip(state))] +#[tracing::instrument(name = "api::mapping_delete", level = "debug", skip(state), err(Debug))] async fn remove( Auth(user_auth): Auth, Path(id): Path, diff --git a/src/api/v3/federation/mod.rs b/src/api/v3/federation/mod.rs index 4d3cd32c..12e86a01 100644 --- a/src/api/v3/federation/mod.rs +++ b/src/api/v3/federation/mod.rs @@ -11,7 +11,12 @@ // limitations under the License. // // SPDX-License-Identifier: Apache-2.0 - +//! Federation management +//! +//! - IDP +//! - Mapping +//! - Auth initialization +//! - Auth callback use utoipa_axum::router::OpenApiRouter; use crate::keystone::ServiceState; diff --git a/src/api/v3/federation/oidc.rs b/src/api/v3/federation/oidc.rs index 64452f16..736eddf7 100644 --- a/src/api/v3/federation/oidc.rs +++ b/src/api/v3/federation/oidc.rs @@ -51,6 +51,15 @@ pub(super) fn openapi_router() -> OpenApiRouter { OpenApiRouter::new().routes(routes!(callback)) } +/// Extract AuthZ information from the saved scope +/// +/// # Arguments +/// * `state`: The service state +/// * `scope`: The scope to extract the AuthZ information from +/// +/// # Returns +/// * `AuthzInfo`: The AuthZ information +/// * `KeystoneApiError`: An error if the scope is not valid async fn get_authz_info( state: &ServiceState, scope: Option<&ProviderScope>, @@ -94,7 +103,8 @@ async fn get_authz_info( #[tracing::instrument( name = "api::identity_provider_auth_callback", level = "debug", - skip(state) + skip(state), + err(Debug) )] #[debug_handler] pub async fn callback( @@ -166,8 +176,7 @@ pub async fn callback( return Err(OidcError::ClientWithoutDiscoveryNotSupported)?; }; - // Set the URL the user will be redirected to after the authorization process. - + // Finish authorization request by exchanging the authorization code for the token. let token_response = client .exchange_code(AuthorizationCode::new(query.code)) .map_err(OidcError::from)? @@ -177,14 +186,11 @@ pub async fn callback( .await .map_err(|err| OidcError::request_token(&err))?; - debug!("Response is {:?}", token_response); - //// Extract the ID token claims after verifying its authenticity and nonce. let id_token = token_response.id_token().ok_or(OidcError::NoToken)?; let claims = id_token .claims(&client.id_token_verifier(), &Nonce::new(auth_state.nonce)) .map_err(OidcError::from)?; - debug!("id_token: {:?}, claims: {:?}", id_token, claims,); if let Some(bound_issuer) = &idp.bound_issuer { if Url::parse(bound_issuer) .map_err(OidcError::from) @@ -199,7 +205,6 @@ pub async fn callback( } let claims_as_json = serde_json::to_value(claims)?; - debug!("Json: {:?}", claims_as_json); validate_bound_claims(&mapping, claims, &claims_as_json)?; let mapped_user_data = map_user_data(&idp, &mapping, &claims_as_json)?; @@ -249,6 +254,7 @@ pub async fn callback( .protocol_id("oidc".to_string()) .build() .map_err(AuthenticationError::from)?; + authed_info.validate()?; // TODO: Persist group memberships @@ -289,6 +295,17 @@ pub async fn callback( .into_response()) } +/// Validate bound claims in the token +/// +/// # Arguments +/// +/// * `mapping` - The mapping to validate against +/// * `claims` - The claims to validate +/// * `claims_as_json` - The claims as json to validate +/// +/// # Returns +/// +/// * `Result<(), OidcError>` fn validate_bound_claims( mapping: &ProviderMapping, claims: &IdTokenClaims, @@ -345,6 +362,14 @@ fn validate_bound_claims( } /// Map the user data using the referred mapping +/// +/// # Arguments +/// * `idp` - The identity provider +/// * `mapping` - The mapping to use +/// * `claims_as_json` - The claims as json +/// +/// # Returns +/// The mapped user data fn map_user_data( idp: &ProviderIdentityProvider, mapping: &ProviderMapping, diff --git a/src/api/v3/federation/types/auth.rs b/src/api/v3/federation/types/auth.rs index e28ba89c..cf97b5b1 100644 --- a/src/api/v3/federation/types/auth.rs +++ b/src/api/v3/federation/types/auth.rs @@ -20,6 +20,7 @@ use axum::{ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; +/// Request for initializing the federated authentication #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct IdentityProviderAuthRequest { /// Redirect URI to include in the auth request @@ -32,14 +33,19 @@ pub struct IdentityProviderAuthRequest { pub scope: Option, } +/// Authentication initialization response #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct IdentityProviderAuthResponse { + /// Url the client must open in the browser to continue the authentication pub auth_url: String, } +/// Authentication callback request the user is sending to complete the authentication request #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct AuthCallbackParameters { + /// Authentication state. pub state: String, + /// Authorization code. pub code: String, } diff --git a/src/api/v3/federation/types/identity_provider.rs b/src/api/v3/federation/types/identity_provider.rs index 34c75fd8..abb08798 100644 --- a/src/api/v3/federation/types/identity_provider.rs +++ b/src/api/v3/federation/types/identity_provider.rs @@ -25,6 +25,7 @@ use utoipa::{IntoParams, ToSchema}; use crate::api::error::KeystoneApiError; use crate::federation::types; +/// Identity provider data #[derive(Builder, Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct IdentityProvider { @@ -68,58 +69,72 @@ pub struct IdentityProvider { pub provider_config: Option, } +/// Identity provider response #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] pub struct IdentityProviderResponse { /// IDP object pub identity_provider: IdentityProvider, } +/// Identity provider data #[derive(Builder, Clone, Default, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct IdentityProviderCreate { + /// Identity provider name. pub name: String, + /// Identity provider domain. #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub domain_id: Option, + /// OIDC/OAuth2 discovery url #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub oidc_discovery_url: Option, + /// OIDC/OAuth2 Client id #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub oidc_client_id: Option, + /// OIDC/OAuth2 Client secret #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub oidc_client_secret: Option, + /// OIDC response more #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub oidc_response_mode: Option, + /// OIDC response types #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub oidc_response_types: Option>, + /// JWT validation public keys #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub jwt_validation_pubkeys: Option>, + /// Bound issuer #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub bound_issuer: Option, + /// Default mapping name that should be used by default #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub default_mapping_name: Option, + /// Additional special provider specific configuration #[serde(skip_serializing_if = "Option::is_none")] #[builder(default)] pub provider_config: Option, } +/// Identity provider data #[derive(Builder, Clone, Default, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct IdentityProviderUpdate { @@ -154,6 +169,7 @@ pub struct IdentityProviderUpdate { pub provider_config: Option>, } +/// Identity provider create request #[derive(Builder, Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct IdentityProviderCreateRequest { @@ -161,6 +177,7 @@ pub struct IdentityProviderCreateRequest { pub identity_provider: IdentityProviderCreate, } +/// Identity provider update request #[derive(Builder, Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct IdentityProviderUpdateRequest { @@ -240,7 +257,7 @@ impl From for KeystoneApiError { } } -/// Identity Providers +/// List of Identity Providers #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize, ToSchema)] pub struct IdentityProviderList { /// Collection of identity provider objects @@ -253,7 +270,7 @@ impl IntoResponse for IdentityProviderList { } } -/// List identity provider query parameters +/// Query parameters for listing federated identity providers #[derive(Clone, Debug, Default, Deserialize, Serialize, IntoParams)] pub struct IdentityProviderListParameters { /// Filters the response by IDP name. diff --git a/src/api/v3/federation/types/mapping.rs b/src/api/v3/federation/types/mapping.rs index e2a41960..8eb36415 100644 --- a/src/api/v3/federation/types/mapping.rs +++ b/src/api/v3/federation/types/mapping.rs @@ -25,6 +25,7 @@ use utoipa::{IntoParams, ToSchema}; use crate::api::error::KeystoneApiError; use crate::federation::types; +/// OIDC/JWT mapping data #[derive(Builder, Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct Mapping { @@ -92,6 +93,7 @@ pub struct MappingResponse { pub mapping: Mapping, } +/// OIDC/JWT mapping data #[derive(Builder, Clone, Default, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct MappingCreate { @@ -150,6 +152,7 @@ pub struct MappingCreate { pub token_project_id: Option, } +/// OIDC/JWT mapping data #[derive(Builder, Clone, Default, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(into))] pub struct MappingUpdate { @@ -215,6 +218,7 @@ pub struct MappingUpdate { pub token_project_id: Option>, } +/// OIDC/JWT mapping create request #[derive(Builder, Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct MappingCreateRequest { @@ -222,6 +226,7 @@ pub struct MappingCreateRequest { pub mapping: MappingCreate, } +/// OIDC/JWT mapping update request #[derive(Builder, Clone, Debug, Deserialize, PartialEq, Serialize, ToSchema)] #[builder(setter(strip_option, into))] pub struct MappingUpdateRequest { @@ -314,7 +319,7 @@ impl From for KeystoneApiError { } } -/// Identity Providers +/// List of OIDC/JWT mappings #[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize, ToSchema)] pub struct MappingList { /// Collection of identity provider objects @@ -327,7 +332,7 @@ impl IntoResponse for MappingList { } } -/// List identity provider query parameters +/// Query parameters for listing OIDC/JWT mappings #[derive(Clone, Debug, Default, Deserialize, Serialize, IntoParams)] pub struct MappingListParameters { /// Filters the response by IDP name. diff --git a/src/api/v3/mod.rs b/src/api/v3/mod.rs index 08a7d536..dcecd34a 100644 --- a/src/api/v3/mod.rs +++ b/src/api/v3/mod.rs @@ -12,6 +12,8 @@ // // SPDX-License-Identifier: Apache-2.0 +//! v3 API + use axum::{ extract::{OriginalUri, Request}, http::{HeaderMap, header}, @@ -42,7 +44,7 @@ pub(super) fn openapi_router() -> OpenApiRouter { .routes(routes!(version)) } -/// Version +/// Version discovery endpoint #[utoipa::path( get, path = "/", diff --git a/src/api/v3/user/passkey/mod.rs b/src/api/v3/user/passkey/mod.rs index 31d539ed..041ae12e 100644 --- a/src/api/v3/user/passkey/mod.rs +++ b/src/api/v3/user/passkey/mod.rs @@ -225,10 +225,23 @@ async fn login_finish( } let authed_info = AuthenticatedInfo::builder() .user_id(user_id.clone()) + .user( + state + .provider + .get_identity_provider() + .get_user(&state.db, &user_id) + .await + .map(|x| { + x.ok_or_else(|| KeystoneApiError::NotFound { + resource: "user".into(), + identifier: user_id, + }) + })??, + ) .methods(vec!["passkey".into()]) .build() .map_err(AuthenticationError::from)?; - //.map_err(|e| OidcError::from(e))?; + authed_info.validate()?; let token = state .provider diff --git a/src/auth/mod.rs b/src/auth/mod.rs index c29e1565..b823fec6 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -12,41 +12,67 @@ // // SPDX-License-Identifier: Apache-2.0 +//! Authorization and authentication information. +//! +//! Authentication and authorization types with corresponding validation. Authentication specific +//! validation may stay in the corresponding provider (i.e. user password is expired), but general +//! validation rules must be present here to be shared across different authentication methods. The +//! same is valid for the authorization validation (project/domain must exist and be enabled). + use derive_builder::Builder; use serde::{Deserialize, Serialize}; use thiserror::Error; -use tracing::error; +use tracing::{error, warn}; use crate::identity::types as identity_provider_types; use crate::resource::types::{Domain, Project}; #[derive(Error, Debug)] pub enum AuthenticationError { + /// Builder error #[error("building authentication information: {source}")] AuthenticatedInfoBuilder { #[from] source: AuthenticatedInfoBuilderError, }, + /// Unauthorized #[error("The request you have made requires authentication.")] Unauthorized, + + /// User is disabled + #[error("The account is disabled for user: {0}")] + UserDisabled(String), } /// Information about successful authentication #[derive(Builder, Clone, Debug, Default, Deserialize, PartialEq, Serialize)] #[builder(setter(into, strip_option))] pub struct AuthenticatedInfo { + /// User id pub user_id: String, + + /// Resolved user object #[builder(default)] pub user: Option, + + /// Resolved user domain information #[builder(default)] pub user_domain: Option, + + /// Authentication methods #[builder(default)] pub methods: Vec, + + /// Audit IDs #[builder(default)] pub audit_ids: Vec, + + /// Federated IDP id #[builder(default)] pub idp_id: Option, + + /// Federated protocol id #[builder(default)] pub protocol_id: Option, } @@ -56,19 +82,54 @@ impl AuthenticatedInfo { AuthenticatedInfoBuilder::default() } + /// Validate the authentication information + /// + /// - User attribute must be set + /// - User must be enabled + /// - User object id must match user_id pub fn validate(&self) -> Result<(), AuthenticationError> { + // TODO: all validations (disabled user, locked, etc) should be placed here since every + // authentication method goes different way and we risk missing validations + if let Some(user) = &self.user { + if user.id != self.user_id { + warn!( + "User data does not match the user_id attribute: {} vs {}", + self.user_id, user.id + ); + return Err(AuthenticationError::Unauthorized); + } + if !user.enabled { + return Err(AuthenticationError::UserDisabled(self.user_id.clone())); + } + } else { + warn!( + "User data must be resolved in the AuthenticatedInfo before validating: {:?}", + self + ); + return Err(AuthenticationError::Unauthorized); + } + Ok(()) } } +/// Authorization information #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub enum AuthzInfo { + /// Unscoped Unscoped, + /// Project scope Project(Project), + /// Domain scope Domain(Domain), } impl AuthzInfo { + /// Validate the authorization information + /// + /// - Unscoped: always valid + /// - Project: check if the project is enabled + /// - Domain: check if the domain is enabled pub fn validate(&self) -> Result<(), AuthenticationError> { match self { AuthzInfo::Unscoped => {} @@ -86,3 +147,119 @@ impl AuthzInfo { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + use tracing_test::traced_test; + + use crate::identity::types::UserResponse; + + #[test] + fn test_authn_validate_no_user() { + let authn = AuthenticatedInfo::builder().user_id("uid").build().unwrap(); + if let Err(AuthenticationError::Unauthorized) = authn.validate() { + } else { + panic!("should be unauthorized"); + } + } + + #[test] + #[traced_test] + fn test_authn_validate_user_disabled() { + let authn = AuthenticatedInfo::builder() + .user_id("uid") + .user(UserResponse { + id: "uid".to_string(), + enabled: false, + ..Default::default() + }) + .build() + .unwrap(); + if let Err(AuthenticationError::UserDisabled(uid)) = authn.validate() { + assert_eq!("uid", uid); + } else { + panic!("should fail for disabled user"); + } + } + + #[test] + #[traced_test] + fn test_authn_validate_user_mismatch() { + let authn = AuthenticatedInfo::builder() + .user_id("uid1") + .user(UserResponse { + id: "uid2".to_string(), + enabled: false, + ..Default::default() + }) + .build() + .unwrap(); + if let Err(AuthenticationError::Unauthorized) = authn.validate() { + } else { + panic!("should fail when user_id != user.id"); + } + } + + #[test] + #[traced_test] + fn test_authz_validate_project() { + let authz = AuthzInfo::Project(Project { + id: "pid".into(), + domain_id: "pdid".into(), + enabled: true, + ..Default::default() + }); + assert!(authz.validate().is_ok()); + } + + #[test] + #[traced_test] + fn test_authz_validate_project_disabled() { + let authz = AuthzInfo::Project(Project { + id: "pid".into(), + domain_id: "pdid".into(), + enabled: false, + ..Default::default() + }); + if let Err(AuthenticationError::Unauthorized) = authz.validate() { + } else { + panic!("should fail when project is not enabled"); + } + } + + #[test] + #[traced_test] + fn test_authz_validate_domain() { + let authz = AuthzInfo::Domain(Domain { + id: "id".into(), + name: "name".into(), + enabled: true, + ..Default::default() + }); + assert!(authz.validate().is_ok()); + } + + #[test] + #[traced_test] + fn test_authz_validate_domain_disabled() { + let authz = AuthzInfo::Domain(Domain { + id: "id".into(), + name: "name".into(), + enabled: false, + ..Default::default() + }); + if let Err(AuthenticationError::Unauthorized) = authz.validate() { + } else { + panic!("should fail when domain is not enabled"); + } + } + + #[test] + #[traced_test] + fn test_authz_validate_unscoped() { + let authz = AuthzInfo::Unscoped; + assert!(authz.validate().is_ok()); + } +} diff --git a/src/identity/backends/sql.rs b/src/identity/backends/sql.rs index d179bb9b..41203948 100644 --- a/src/identity/backends/sql.rs +++ b/src/identity/backends/sql.rs @@ -80,14 +80,8 @@ impl IdentityBackend for SqlBackend { expected_hash, )? { if let Some(user) = user::get(db, &local_user.user_id).await? { - // TODO: Check user is locked // TODO: Check password is expired // TODO: reset failed login attempt - if !user.enabled.is_some_and(|val| val) { - return Err(IdentityProviderError::UserDisabled( - local_user.user_id, - )); - } let user_builder = common::get_local_user_builder( &self.config, &user, diff --git a/src/identity/error.rs b/src/identity/error.rs index 402c6c6b..fb11d287 100644 --- a/src/identity/error.rs +++ b/src/identity/error.rs @@ -38,9 +38,6 @@ pub enum IdentityProviderError { #[error("group {0} not found")] GroupNotFound(String), - #[error("The account is disabled for user: {0}")] - UserDisabled(String), - /// Identity provider error #[error(transparent)] IdentityDatabase { diff --git a/src/token/error.rs b/src/token/error.rs index 6db82ec4..750c23b4 100644 --- a/src/token/error.rs +++ b/src/token/error.rs @@ -162,4 +162,7 @@ pub enum TokenProviderError { #[error("federated payload must contain idp_id and protocol_id")] FederatedPayloadMissingData, + + #[error("user cannot be found: {0}")] + UserNotFound(String), } diff --git a/src/token/mod.rs b/src/token/mod.rs index 9d7978cb..c494d41f 100644 --- a/src/token/mod.rs +++ b/src/token/mod.rs @@ -337,6 +337,10 @@ impl TokenApi for TokenProvider { authentication_info: AuthenticatedInfo, authz_info: AuthzInfo, ) -> Result { + // This should be executed already, but let's better repeat it as last line of defence. + // It is also necessary to call this before to stop before we start to resolve authz info. + authentication_info.validate()?; + // TODO: Check whether it is allowed to change the scope of the token if AuthenticatedInfo // already contains scope it was issued for. let mut authentication_info = authentication_info;