diff --git a/server/core/src/actors/v1_read.rs b/server/core/src/actors/v1_read.rs index 3d2a2eb13..c145c26ae 100644 --- a/server/core/src/actors/v1_read.rs +++ b/server/core/src/actors/v1_read.rs @@ -1422,13 +1422,13 @@ impl QueryServerReadV1 { .map_err(Oauth2Error::ServerError)?; let ident = idms_prox_read .validate_client_auth_info_to_ident(client_auth_info, ct) - .map_err(|e| { + .inspect_err(|e| { error!("Invalid identity: {:?}", e); - Oauth2Error::AuthenticationRequired - })?; + }) + .ok(); // Now we can send to the idm server for authorisation checking. - idms_prox_read.check_oauth2_authorisation(&ident, &auth_req, ct) + idms_prox_read.check_oauth2_authorisation(ident.as_ref(), &auth_req, ct) } #[instrument( diff --git a/server/core/src/https/oauth2.rs b/server/core/src/https/oauth2.rs index 4f3e98fbc..cfb783b6e 100644 --- a/server/core/src/https/oauth2.rs +++ b/server/core/src/https/oauth2.rs @@ -289,7 +289,8 @@ async fn oauth2_authorise( .body(body) .unwrap() } - Err(Oauth2Error::AuthenticationRequired) => { + Ok(AuthoriseResponse::AuthenticationRequired { .. }) + | Err(Oauth2Error::AuthenticationRequired) => { // This will trigger our ui to auth and retry. #[allow(clippy::unwrap_used)] Response::builder() diff --git a/server/core/src/https/views/constants.rs b/server/core/src/https/views/constants.rs index 450d763d5..d04737ce7 100644 --- a/server/core/src/https/views/constants.rs +++ b/server/core/src/https/views/constants.rs @@ -28,6 +28,7 @@ pub(crate) enum Urls { UpdateCredentials, Oauth2Resume, Login, + Ui, } impl AsRef for Urls { @@ -39,6 +40,7 @@ impl AsRef for Urls { Self::UpdateCredentials => "/ui/update_credentials", Self::Oauth2Resume => "/ui/oauth2/resume", Self::Login => "/ui/login", + Self::Ui => "/ui", } } } diff --git a/server/core/src/https/views/login.rs b/server/core/src/https/views/login.rs index d891a72eb..8e3651442 100644 --- a/server/core/src/https/views/login.rs +++ b/server/core/src/https/views/login.rs @@ -76,10 +76,15 @@ pub struct Reauth { pub purpose: ReauthPurpose, } +pub struct Oauth2Ctx { + pub client_name: String, +} + pub struct LoginDisplayCtx { pub domain_info: DomainInfoRead, // We only need this on the first re-auth screen to indicate what we are doing pub reauth: Option, + pub oauth2: Option, pub error: Option, } @@ -182,6 +187,17 @@ pub async fn view_reauth_get( return_location: &str, display_ctx: LoginDisplayCtx, ) -> Response { + // No matter what, we always clear the stored oauth2 cookie to prevent + // ui loops + let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { + let mut authreq_cookie = authreq_cookie.clone(); + authreq_cookie.make_removal(); + authreq_cookie.set_path(Urls::Ui.as_ref()); + jar.add(authreq_cookie) + } else { + jar + }; + let session_valid_result = state .qe_r_ref .handle_auth_valid(client_auth_info.clone(), kopid.eventid) @@ -247,12 +263,15 @@ pub async fn view_reauth_get( let remember_me = !username.is_empty(); - LoginView { - display_ctx, - username, - remember_me, - } - .into_response() + ( + jar, + LoginView { + display_ctx, + username, + remember_me, + }, + ) + .into_response() } Err(err_code) => UnrecoverableErrorView { err_code, @@ -262,6 +281,33 @@ pub async fn view_reauth_get( } } +pub fn view_oauth2_get( + jar: CookieJar, + display_ctx: LoginDisplayCtx, + login_hint: Option, +) -> Response { + let (username, remember_me) = if let Some(login_hint) = login_hint { + (login_hint, false) + } else if let Some(cookie_username) = + // cookie jar with remember me. + jar.get(COOKIE_USERNAME).map(|c| c.value().to_string()) + { + (cookie_username, true) + } else { + (String::default(), false) + }; + + ( + jar, + LoginView { + display_ctx, + username, + remember_me, + }, + ) + .into_response() +} + pub async fn view_index_get( State(state): State, VerifiedClientInformation(client_auth_info): VerifiedClientInformation, @@ -275,10 +321,21 @@ pub async fn view_index_get( .handle_auth_valid(client_auth_info, kopid.eventid) .await; + // No matter what, we always clear the stored oauth2 cookie to prevent + // ui loops + let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { + let mut authreq_cookie = authreq_cookie.clone(); + authreq_cookie.make_removal(); + authreq_cookie.set_path(Urls::Ui.as_ref()); + jar.add(authreq_cookie) + } else { + jar + }; + match session_valid_result { Ok(()) => { // Send the user to the landing. - Redirect::to(Urls::Apps.as_ref()).into_response() + (jar, Redirect::to(Urls::Apps.as_ref())).into_response() } Err(OperationError::NotAuthenticated) | Err(OperationError::SessionExpired) => { // cookie jar with remember me. @@ -291,16 +348,20 @@ pub async fn view_index_get( let display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: None, error: None, }; - LoginView { - display_ctx, - username, - remember_me, - } - .into_response() + ( + jar, + LoginView { + display_ctx, + username, + remember_me, + }, + ) + .into_response() } Err(err_code) => UnrecoverableErrorView { err_code, @@ -368,6 +429,7 @@ pub async fn view_login_begin_post( let mut display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: None, error: None, }; @@ -450,6 +512,7 @@ pub async fn view_login_mech_choose_post( let display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: None, error: None, }; @@ -505,6 +568,7 @@ pub async fn view_login_totp_post( Err(_) => { let display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: None, error: None, }; @@ -610,6 +674,7 @@ async fn credential_step( let display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: None, error: None, }; diff --git a/server/core/src/https/views/oauth2.rs b/server/core/src/https/views/oauth2.rs index 51af8fd4d..4cdc83052 100644 --- a/server/core/src/https/views/oauth2.rs +++ b/server/core/src/https/views/oauth2.rs @@ -1,4 +1,8 @@ -use crate::https::{extractors::VerifiedClientInformation, middleware::KOpId, ServerState}; +use crate::https::{ + extractors::{DomainInfo, DomainInfoRead, VerifiedClientInformation}, + middleware::KOpId, + ServerState, +}; use kanidmd_lib::idm::oauth2::{ AuthorisationRequest, AuthorisePermitSuccess, AuthoriseResponse, Oauth2Error, }; @@ -23,6 +27,7 @@ use axum_htmx::HX_REDIRECT; use serde::Deserialize; use super::constants::Urls; +use super::login::{LoginDisplayCtx, Oauth2Ctx}; use super::{cookies, UnrecoverableErrorView}; #[derive(Template)] @@ -45,23 +50,25 @@ pub async fn view_index_get( State(state): State, Extension(kopid): Extension, VerifiedClientInformation(client_auth_info): VerifiedClientInformation, + DomainInfo(domain_info): DomainInfo, jar: CookieJar, Query(auth_req): Query, ) -> Response { - oauth2_auth_req(state, kopid, client_auth_info, jar, auth_req).await + oauth2_auth_req(state, kopid, client_auth_info, domain_info, jar, auth_req).await } pub async fn view_resume_get( State(state): State, Extension(kopid): Extension, VerifiedClientInformation(client_auth_info): VerifiedClientInformation, + DomainInfo(domain_info): DomainInfo, jar: CookieJar, ) -> Result { let maybe_auth_req = cookies::get_signed::(&state, &jar, COOKIE_OAUTH2_REQ); if let Some(auth_req) = maybe_auth_req { - Ok(oauth2_auth_req(state, kopid, client_auth_info, jar, auth_req).await) + Ok(oauth2_auth_req(state, kopid, client_auth_info, domain_info, jar, auth_req).await) } else { error!("unable to resume session, no auth_req was found in the cookie"); Err(UnrecoverableErrorView { @@ -75,6 +82,7 @@ async fn oauth2_auth_req( state: ServerState, kopid: KOpId, client_auth_info: ClientAuthInfo, + domain_info: DomainInfoRead, jar: CookieJar, auth_req: AuthorisationRequest, ) -> Response { @@ -83,21 +91,23 @@ async fn oauth2_auth_req( .handle_oauth2_authorise(client_auth_info, auth_req.clone(), kopid.eventid) .await; + // No matter what, we always clear the stored oauth2 cookie to prevent + // ui loops + let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { + let mut authreq_cookie = authreq_cookie.clone(); + authreq_cookie.make_removal(); + authreq_cookie.set_path(Urls::Ui.as_ref()); + jar.add(authreq_cookie) + } else { + jar + }; + match res { Ok(AuthoriseResponse::Permitted(AuthorisePermitSuccess { mut redirect_uri, state, code, })) => { - let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { - let mut authreq_cookie = authreq_cookie.clone(); - authreq_cookie.make_removal(); - authreq_cookie.set_path("/ui"); - jar.add(authreq_cookie) - } else { - jar - }; - redirect_uri .query_pairs_mut() .clear() @@ -133,17 +143,32 @@ async fn oauth2_auth_req( } .into_response() } - Err(Oauth2Error::AuthenticationRequired) => { - // Sign the auth req and hide it in our cookie. - let maybe_jar = cookies::make_signed(&state, COOKIE_OAUTH2_REQ, &auth_req, "/ui") - .map(|mut cookie| { - cookie.set_same_site(SameSite::Strict); - jar.add(cookie) - }) - .ok_or(OperationError::InvalidSessionState); + + Ok(AuthoriseResponse::AuthenticationRequired { + client_name, + login_hint, + }) => { + // Sign the auth req and hide it in our cookie - we'll come back for + // you later. + let maybe_jar = + cookies::make_signed(&state, COOKIE_OAUTH2_REQ, &auth_req, Urls::Ui.as_ref()) + .map(|mut cookie| { + cookie.set_same_site(SameSite::Strict); + jar.add(cookie) + }) + .ok_or(OperationError::InvalidSessionState); match maybe_jar { - Ok(jar) => (jar, Redirect::to(Urls::Login.as_ref())).into_response(), + Ok(jar) => { + let display_ctx = LoginDisplayCtx { + domain_info, + oauth2: Some(Oauth2Ctx { client_name }), + reauth: None, + error: None, + }; + + super::login::view_oauth2_get(jar, display_ctx, login_hint) + } Err(err_code) => UnrecoverableErrorView { err_code, operation_id: kopid.eventid, diff --git a/server/core/src/https/views/profile.rs b/server/core/src/https/views/profile.rs index 3953d35eb..42d635cb8 100644 --- a/server/core/src/https/views/profile.rs +++ b/server/core/src/https/views/profile.rs @@ -73,6 +73,7 @@ pub(crate) async fn view_profile_unlock_get( let display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: Some(Reauth { username: uat.spn, purpose: ReauthPurpose::ProfileSettings, diff --git a/server/core/src/https/views/reset.rs b/server/core/src/https/views/reset.rs index e838a5fef..60f2ef7bc 100644 --- a/server/core/src/https/views/reset.rs +++ b/server/core/src/https/views/reset.rs @@ -653,6 +653,7 @@ pub(crate) async fn view_self_reset_get( } else { let display_ctx = LoginDisplayCtx { domain_info, + oauth2: None, reauth: Some(Reauth { username: uat.spn, purpose: ReauthPurpose::ProfileSettings, diff --git a/server/core/templates/login_base.html b/server/core/templates/login_base.html index f452424e4..3c86a95c7 100644 --- a/server/core/templates/login_base.html +++ b/server/core/templates/login_base.html @@ -20,6 +20,10 @@ + (% else if let Some(oauth2) = display_ctx.oauth2 %) + (% endif %)
(% block logincontainer %) diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs index 340538666..d74844c46 100644 --- a/server/lib/src/idm/oauth2.rs +++ b/server/lib/src/idm/oauth2.rs @@ -208,6 +208,12 @@ impl fmt::Display for Oauth2TokenType { #[derive(Debug)] pub enum AuthoriseResponse { + AuthenticationRequired { + // A pretty-name of the client + client_name: String, + // A username hint, if any + login_hint: Option, + }, ConsentRequested { // A pretty-name of the client client_name: String, @@ -1775,7 +1781,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> { #[instrument(level = "debug", skip_all)] pub fn check_oauth2_authorisation( &self, - ident: &Identity, + maybe_ident: Option<&Identity>, auth_req: &AuthorisationRequest, ct: Duration, ) -> Result { @@ -1889,6 +1895,11 @@ impl<'a> IdmServerProxyReadTransaction<'a> { None }; + // ============================================================================= + // By this point, we have validated the majority of the security related + // parameters of the request. We can now inspect the identity and decide + // if we should ask the user to re-authenticate and proceed. + // TODO: https://openid.net/specs/openid-connect-basic-1_0.html#RequestParameters // Are we going to provide the functions for these? Most of these can be "later". // IF CHANGED: Update OidcDiscoveryResponse!!! @@ -1908,7 +1919,13 @@ impl<'a> IdmServerProxyReadTransaction<'a> { // TODO: id_token_hint - a past token which can be used as a hint. - // NOTE: login_hint is handled in the UI code, not here. + let Some(ident) = maybe_ident else { + debug!("No identity available, assume authentication required"); + return Ok(AuthoriseResponse::AuthenticationRequired { + client_name: o2rs.displayname.clone(), + login_hint: auth_req.oidc_ext.login_hint.clone(), + }); + }; let Some(account_uuid) = ident.get_uuid() else { error!("consent request ident does not have a valid uuid, unable to proceed"); @@ -2939,7 +2956,7 @@ mod tests { }; $idms_prox_read - .check_oauth2_authorisation($ident, &auth_req, $ct) + .check_oauth2_authorisation(Some($ident), &auth_req, $ct) .expect("OAuth2 authorisation failed") }}; } @@ -3431,7 +3448,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::UnsupportedResponseType ); @@ -3451,7 +3468,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::InvalidRequest ); @@ -3471,7 +3488,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::InvalidClientId ); @@ -3491,7 +3508,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::InvalidOrigin ); @@ -3511,11 +3528,33 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::InvalidOrigin ); + // Not Authenticated + let auth_req = AuthorisationRequest { + response_type: "code".to_string(), + client_id: "test_resource_server".to_string(), + state: "123".to_string(), + pkce_request: pkce_request.clone(), + redirect_uri: Url::parse("https://demo.example.com/oauth2/result").unwrap(), + scope: OAUTH2_SCOPE_OPENID.to_string(), + nonce: None, + oidc_ext: Default::default(), + unknown_keys: Default::default(), + }; + + let req = idms_prox_read + .check_oauth2_authorisation(None, &auth_req, ct) + .unwrap(); + + assert!(matches!( + req, + AuthoriseResponse::AuthenticationRequired { .. } + )); + // Requested scope is not available let auth_req = AuthorisationRequest { response_type: "code".to_string(), @@ -3531,7 +3570,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::AccessDenied ); @@ -3551,7 +3590,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&idm_admin_ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&idm_admin_ident), &auth_req, ct) .unwrap_err() == Oauth2Error::AccessDenied ); @@ -3571,7 +3610,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&anon_ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&anon_ident), &auth_req, ct) .unwrap_err() == Oauth2Error::AccessDenied ); @@ -3863,7 +3902,7 @@ mod tests { }; let consent_request = idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("OAuth2 authorisation failed"); trace!(?consent_request); @@ -3932,7 +3971,7 @@ mod tests { }; let consent_request = idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("OAuth2 authorisation failed"); trace!(?consent_request); @@ -5134,7 +5173,7 @@ mod tests { }; idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("Oauth2 authorisation failed"); } @@ -5347,7 +5386,7 @@ mod tests { }; let consent_request = idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("Oauth2 authorisation failed"); // Should be in the consent phase; @@ -5405,7 +5444,7 @@ mod tests { }; let consent_request = idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("Oauth2 authorisation failed"); // Should be present in the consent phase however! @@ -5542,7 +5581,7 @@ mod tests { }; let consent_request = idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("Failed to perform OAuth2 authorisation request."); // Should be in the consent phase; @@ -5620,7 +5659,7 @@ mod tests { assert!( idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .unwrap_err() == Oauth2Error::InvalidOrigin ); @@ -6575,7 +6614,7 @@ mod tests { }; let consent_request = idms_prox_read - .check_oauth2_authorisation(&ident, &auth_req, ct) + .check_oauth2_authorisation(Some(&ident), &auth_req, ct) .expect("OAuth2 authorisation failed"); // Should be in the consent phase;