Fixes the logout flow in htmx and improves the login error dialog (#2889)

server/core/src/actors/v1_write.rs

@@ -541,17 +541,24 @@ impl QueryServerWriteV1 {
         let mut idms_prox_write = self.idms.proxy_write(ct).await;
 
         // We specifically need a uat here to assess the auth type!
-        let ident = idms_prox_write
-            .validate_client_auth_info_to_ident(client_auth_info, ct)
-            .map_err(|e| {
-                admin_error!(err = ?e, "Invalid identity");
-                e
-            })?;
+        let validate_result =
+            idms_prox_write.validate_client_auth_info_to_ident(client_auth_info, ct);
+
+        let ident = match validate_result {
+            Ok(ident) => ident,
+            Err(OperationError::SessionExpired) | Err(OperationError::NotAuthenticated) => {
+                return Ok(())
+            }
+            Err(err) => {
+                admin_error!(?err, "Invalid identity");
+                return Err(err);
+            }
+        };
 
         if !ident.can_logout() {
             info!("Ignoring request to logout session - these sessions are not recorded");
             return Ok(());
-        }
+        };
 
         let target = ident.get_uuid().ok_or_else(|| {
             admin_error!("Invalid identity - no uuid present");

server/core/src/https/errors.rs

@@ -1,14 +1,13 @@
 //! Where we hide the error handling widgets
 //!
 
-use axum::http::{HeaderValue, StatusCode};
 use axum::http::header::ACCESS_CONTROL_ALLOW_ORIGIN;
+use axum::http::{HeaderValue, StatusCode};
 use axum::response::{IntoResponse, Response};
 use utoipa::ToSchema;
 
 use kanidm_proto::internal::OperationError;
 
-
 /// The web app's top level error type, this takes an `OperationError` and converts it into a HTTP response.
 #[derive(Debug, ToSchema)]
 pub enum WebError {

server/core/src/https/views/apps.rs

@@ -1,18 +1,18 @@
 use askama::Template;
 use axum::{
-    Extension,
     extract::State,
     http::uri::Uri,
     response::{IntoResponse, Response},
+    Extension,
 };
-use axum_htmx::{HxPushUrl, HxReswap, HxRetarget, SwapOption};
 use axum_htmx::extractors::HxRequest;
+use axum_htmx::{HxPushUrl, HxReswap, HxRetarget, SwapOption};
 
 use kanidm_proto::internal::AppLink;
 
-use crate::https::{extractors::VerifiedClientInformation, middleware::KOpId, ServerState};
-use crate::https::views::errors::HtmxError;
 use super::HtmlTemplate;
+use crate::https::views::errors::HtmxError;
+use crate::https::{extractors::VerifiedClientInformation, middleware::KOpId, ServerState};
 
 #[derive(Template)]
 #[template(path = "apps.html")]
@@ -40,12 +40,10 @@ pub(crate) async fn view_apps_get(
         .qe_r_ref
         .handle_list_applinks(client_auth_info, kopid.eventid)
         .await
-        .map_err(|old| HtmxError::new(&kopid, old) )?;
+        .map_err(|old| HtmxError::new(&kopid, old))?;
 
     let apps_view = AppsView {
-        apps_partial: AppsPartialView {
-            apps: app_links,
-        },
+        apps_partial: AppsPartialView { apps: app_links },
     };
 
     Ok(if hx_request {
@@ -60,7 +58,8 @@ pub(crate) async fn view_apps_get(
             // We send our own main, replace the existing one.
             HxReswap(SwapOption::OuterHtml),
             HtmlTemplate(apps_view),
-        ).into_response()
+        )
+            .into_response()
     } else {
         HtmlTemplate(apps_view).into_response()
     })

server/core/src/https/views/errors.rs

@@ -16,7 +16,6 @@ use crate::https::middleware::KOpId;
 //     recovery_boosted: bool,
 // }
 
-
 /// The web app's top level error type, this takes an `OperationError` and converts it into a HTTP response.
 #[derive(Debug, ToSchema)]
 pub(crate) enum HtmxError {
@@ -46,7 +45,9 @@ impl IntoResponse for HtmxError {
                     OperationError::SystemProtectedObject | OperationError::AccessDenied => {
                         (StatusCode::FORBIDDEN, body).into_response()
                     }
-                    OperationError::NoMatchingEntries => (StatusCode::NOT_FOUND, body).into_response(),
+                    OperationError::NoMatchingEntries => {
+                        (StatusCode::NOT_FOUND, body).into_response()
+                    }
                     OperationError::PasswordQuality(_)
                     | OperationError::EmptyRequest
                     | OperationError::SchemaViolation(_)

server/core/src/https/views/login.rs

@@ -100,6 +100,45 @@ struct LoginWebauthnView {
     chal: String,
 }
 
+#[derive(Template, Default)]
+#[template(path = "login_denied.html")]
+struct LoginDeniedView {
+    reason: String,
+    operation_id: Uuid,
+}
+
+pub async fn view_logout_get(
+    State(state): State<ServerState>,
+    VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
+    Extension(kopid): Extension<KOpId>,
+    mut jar: CookieJar,
+) -> Response {
+    if let Err(err_code) = state
+        .qe_w_ref
+        .handle_logout(client_auth_info, kopid.eventid)
+        .await
+    {
+        HtmlTemplate(UnrecoverableErrorView {
+            err_code,
+            operation_id: kopid.eventid,
+        })
+        .into_response()
+    } else {
+        let response = Redirect::to("/ui").into_response();
+
+        jar = if let Some(bearer_cookie) = jar.get(COOKIE_BEARER_TOKEN) {
+            let mut bearer_cookie = bearer_cookie.clone();
+            bearer_cookie.make_removal();
+            bearer_cookie.set_path("/");
+            jar.add(bearer_cookie)
+        } else {
+            jar
+        };
+
+        (jar, response).into_response()
+    }
+}
+
 pub async fn view_index_get(
     State(state): State<ServerState>,
     VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
@@ -606,6 +645,7 @@ async fn view_login_step(
                         let mut bearer_cookie = Cookie::new(COOKIE_BEARER_TOKEN, token_str.clone());
                         bearer_cookie.set_secure(state.secure_cookies);
                         bearer_cookie.set_same_site(SameSite::Lax);
+                        // Prevent Document.cookie accessing this. Still works with fetch.
                         bearer_cookie.set_http_only(true);
                         // We set a domain here because it allows subdomains
                         // of the idm to share the cookie. If domain was incorrect
@@ -617,7 +657,7 @@ async fn view_login_step(
                             let mut username_cookie =
                                 Cookie::new(COOKIE_USERNAME, session_context.username.clone());
                             username_cookie.set_secure(state.secure_cookies);
-                            username_cookie.set_same_site(SameSite::Strict);
+                            username_cookie.set_same_site(SameSite::Lax);
                             username_cookie.set_http_only(true);
                             username_cookie.set_domain(state.domain.clone());
                             username_cookie.set_path("/");
@@ -636,12 +676,15 @@ async fn view_login_step(
                     }
                 }
             }
-            AuthState::Denied(_reason) => {
+            AuthState::Denied(reason) => {
                 debug!("🧩 -> AuthState::Denied");
                 jar = jar.remove(Cookie::from(COOKIE_AUTH_SESSION_ID));
 
-                // Render a denial.
-                break Redirect::temporary("/ui/getrekt").into_response();
+                break HtmlTemplate(LoginDeniedView {
+                    reason,
+                    operation_id: kopid.eventid,
+                })
+                .into_response();
             }
         }
     };

server/core/src/https/views/mod.rs

@@ -17,8 +17,8 @@ use crate::https::{
 };
 
 mod apps;
-mod login;
 mod errors;
+mod login;
 
 #[derive(Template)]
 #[template(path = "unrecoverable_error.html")]
@@ -31,6 +31,7 @@ pub fn view_router() -> Router<ServerState> {
     let unguarded_router = Router::new()
         .route("/", get(login::view_index_get))
         .route("/apps", get(apps::view_apps_get))
+        .route("/logout", get(login::view_logout_get))
         // The login routes are htmx-free to make them simpler, which means
         // they need manual guarding for direct get requests which can occur
         // if a user attempts to reload the page.

server/core/templates/login_denied.html

@@ -0,0 +1,13 @@
+(% extends "login_base.html" %)
+
+(% block logincontainer %)
+	<h3>Login Failed</h3>
+	<main id="main">
+		<p>Reason: (( reason ))</p>
+		<p>Operation ID: (( operation_id ))</p>
+		<a href="/ui">
+			<button type="button" class="btn btn-success">Return to Login</button>
+		</a>
+	</main>
+
+(% endblock %)

server/core/templates/unrecoverable_error.html

@@ -8,6 +8,7 @@
 (% block body %)
 	<h2>Error</h2>
 	<main id="main">
+		<p>An unrecoverable error occured. Please contact your administrator with the details below.</p>
 		<p>Error Code: (( err_code ))</p>
 		<p>Operation ID: (( operation_id ))</p>
 	</main>