Improved Cookie Removal

If a path isn't set then cookies aren't removed. More aggressively
remove cookies when they are no longer required.
This commit is contained in:
William Brown 2024-12-17 15:16:32 +10:00 committed by Firstyear
parent 50a7d9d700
commit 11438a9dd5
5 changed files with 50 additions and 53 deletions

View file

@ -6,11 +6,16 @@ use compact_jwt::{Jws, JwsSigner};
use serde::de::DeserializeOwned; use serde::de::DeserializeOwned;
use serde::Serialize; use serde::Serialize;
#[instrument(name = "views::cookies::destroy", level = "debug", skip(jar))]
pub fn destroy(jar: CookieJar, ck_id: &str) -> CookieJar { pub fn destroy(jar: CookieJar, ck_id: &str) -> CookieJar {
if let Some(ck) = jar.get(ck_id) { if let Some(ck) = jar.get(ck_id) {
let mut ck = ck.clone(); let mut removal_cookie = ck.clone();
ck.make_removal(); removal_cookie.make_removal();
jar.add(ck) // Need to be set to / to remove on all parent paths.
// If you don't set a path, NOTHING IS REMOVED!!!
removal_cookie.set_path("/");
jar.add(removal_cookie)
} else { } else {
jar jar
} }

View file

@ -15,6 +15,7 @@ use axum::{
use axum_extra::extract::cookie::{Cookie, CookieJar, SameSite}; use axum_extra::extract::cookie::{Cookie, CookieJar, SameSite};
use kanidm_proto::internal::{ use kanidm_proto::internal::{
COOKIE_AUTH_SESSION_ID, COOKIE_BEARER_TOKEN, COOKIE_OAUTH2_REQ, COOKIE_USERNAME, COOKIE_AUTH_SESSION_ID, COOKIE_BEARER_TOKEN, COOKIE_OAUTH2_REQ, COOKIE_USERNAME,
COOKIE_CU_SESSION_TOKEN
}; };
use kanidm_proto::v1::{ use kanidm_proto::v1::{
AuthAllowed, AuthCredential, AuthIssueSession, AuthMech, AuthRequest, AuthStep, AuthAllowed, AuthCredential, AuthIssueSession, AuthMech, AuthRequest, AuthStep,
@ -161,7 +162,7 @@ pub async fn view_logout_get(
Extension(kopid): Extension<KOpId>, Extension(kopid): Extension<KOpId>,
mut jar: CookieJar, mut jar: CookieJar,
) -> Response { ) -> Response {
if let Err(err_code) = state let response = if let Err(err_code) = state
.qe_w_ref .qe_w_ref
.handle_logout(client_auth_info, kopid.eventid) .handle_logout(client_auth_info, kopid.eventid)
.await .await
@ -172,12 +173,16 @@ pub async fn view_logout_get(
} }
.into_response() .into_response()
} else { } else {
let response = Redirect::to(Urls::Login.as_ref()).into_response(); Redirect::to(Urls::Login.as_ref()).into_response()
};
// Always clear cookies even on an error.
jar = cookies::destroy(jar, COOKIE_BEARER_TOKEN); jar = cookies::destroy(jar, COOKIE_BEARER_TOKEN);
jar = cookies::destroy(jar, COOKIE_OAUTH2_REQ);
jar = cookies::destroy(jar, COOKIE_AUTH_SESSION_ID);
jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN);
(jar, response).into_response() (jar, response).into_response()
}
} }
pub async fn view_reauth_get( pub async fn view_reauth_get(
@ -190,14 +195,7 @@ pub async fn view_reauth_get(
) -> Response { ) -> Response {
// No matter what, we always clear the stored oauth2 cookie to prevent // No matter what, we always clear the stored oauth2 cookie to prevent
// ui loops // ui loops
let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { let jar = cookies::destroy(jar, 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 let session_valid_result = state
.qe_r_ref .qe_r_ref
@ -324,14 +322,7 @@ pub async fn view_index_get(
// No matter what, we always clear the stored oauth2 cookie to prevent // No matter what, we always clear the stored oauth2 cookie to prevent
// ui loops // ui loops
let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { let jar = cookies::destroy(jar, 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 { match session_valid_result {
Ok(()) => { Ok(()) => {

View file

@ -96,14 +96,7 @@ async fn oauth2_auth_req(
) -> Response { ) -> Response {
// No matter what, we always clear the stored oauth2 cookie to prevent // No matter what, we always clear the stored oauth2 cookie to prevent
// ui loops // ui loops
let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { let jar = cookies::destroy(jar, 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
};
// If the auth_req was cross-signed, old, or just bad, error. But we have *cleared* it // If the auth_req was cross-signed, old, or just bad, error. But we have *cleared* it
// from the cookie which means we won't see it again. // from the cookie which means we won't see it again.

View file

@ -3,7 +3,7 @@ use axum::extract::{Query, State};
use axum::http::{StatusCode, Uri}; use axum::http::{StatusCode, Uri};
use axum::response::{ErrorResponse, IntoResponse, Redirect, Response}; use axum::response::{ErrorResponse, IntoResponse, Redirect, Response};
use axum::{Extension, Form}; use axum::{Extension, Form};
use axum_extra::extract::cookie::{Cookie, SameSite}; use axum_extra::extract::cookie::SameSite;
use axum_extra::extract::CookieJar; use axum_extra::extract::CookieJar;
use axum_htmx::{ use axum_htmx::{
HxEvent, HxLocation, HxPushUrl, HxRequest, HxReselect, HxResponseTrigger, HxReswap, HxRetarget, HxEvent, HxLocation, HxPushUrl, HxRequest, HxReselect, HxResponseTrigger, HxReswap, HxRetarget,
@ -30,6 +30,7 @@ use super::navbar::NavbarCtx;
use crate::https::extractors::{DomainInfo, DomainInfoRead, VerifiedClientInformation}; use crate::https::extractors::{DomainInfo, DomainInfoRead, VerifiedClientInformation};
use crate::https::middleware::KOpId; use crate::https::middleware::KOpId;
use crate::https::views::constants::ProfileMenuItems; use crate::https::views::constants::ProfileMenuItems;
use crate::https::views::cookies;
use crate::https::views::errors::HtmxError; use crate::https::views::errors::HtmxError;
use crate::https::views::login::{LoginDisplayCtx, Reauth, ReauthPurpose}; use crate::https::views::login::{LoginDisplayCtx, Reauth, ReauthPurpose};
use crate::https::ServerState; use crate::https::ServerState;
@ -210,7 +211,7 @@ pub(crate) async fn commit(
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
state state
.qe_w_ref .qe_w_ref
@ -218,7 +219,10 @@ pub(crate) async fn commit(
.map_err(|op_err| HtmxError::new(&kopid, op_err)) .map_err(|op_err| HtmxError::new(&kopid, op_err))
.await?; .await?;
Ok((HxLocation::from(Uri::from_static("/ui")), "").into_response()) // No longer need the cookie jar.
let jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN);
Ok((jar, HxLocation::from(Uri::from_static("/ui")), "").into_response())
} }
pub(crate) async fn cancel_cred_update( pub(crate) async fn cancel_cred_update(
@ -228,7 +232,7 @@ pub(crate) async fn cancel_cred_update(
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
state state
.qe_w_ref .qe_w_ref
@ -236,7 +240,11 @@ pub(crate) async fn cancel_cred_update(
.map_err(|op_err| HtmxError::new(&kopid, op_err)) .map_err(|op_err| HtmxError::new(&kopid, op_err))
.await?; .await?;
// No longer need the cookie jar.
let jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN);
Ok(( Ok((
jar,
HxLocation::from(Uri::from_static(Urls::Profile.as_ref())), HxLocation::from(Uri::from_static(Urls::Profile.as_ref())),
"", "",
) )
@ -250,7 +258,7 @@ pub(crate) async fn cancel_mfareg(
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let cu_status = state let cu_status = state
.qe_r_ref .qe_r_ref
@ -268,7 +276,7 @@ pub(crate) async fn remove_alt_creds(
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let cu_status = state let cu_status = state
.qe_r_ref .qe_r_ref
@ -286,7 +294,7 @@ pub(crate) async fn remove_unixcred(
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let cu_status = state let cu_status = state
.qe_r_ref .qe_r_ref
@ -309,7 +317,7 @@ pub(crate) async fn remove_totp(
jar: CookieJar, jar: CookieJar,
Form(totp): Form<TOTPRemoveData>, Form(totp): Form<TOTPRemoveData>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let cu_status = state let cu_status = state
.qe_r_ref .qe_r_ref
@ -332,7 +340,7 @@ pub(crate) async fn remove_passkey(
jar: CookieJar, jar: CookieJar,
Form(passkey): Form<PasskeyRemoveData>, Form(passkey): Form<PasskeyRemoveData>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let cu_status = state let cu_status = state
.qe_r_ref .qe_r_ref
@ -355,7 +363,7 @@ pub(crate) async fn finish_passkey(
jar: CookieJar, jar: CookieJar,
Form(passkey_create): Form<PasskeyCreateForm>, Form(passkey_create): Form<PasskeyCreateForm>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token = get_cu_session(jar).await?; let cu_session_token = get_cu_session(&jar).await?;
match serde_json::from_str(passkey_create.creation_data.as_str()) { match serde_json::from_str(passkey_create.creation_data.as_str()) {
Ok(creation_data) => { Ok(creation_data) => {
@ -393,7 +401,7 @@ pub(crate) async fn view_new_passkey(
jar: CookieJar, jar: CookieJar,
Form(init_form): Form<PasskeyInitForm>, Form(init_form): Form<PasskeyInitForm>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token = get_cu_session(jar).await?; let cu_session_token = get_cu_session(&jar).await?;
let cu_req = match init_form.class { let cu_req = match init_form.class {
PasskeyClass::Any => CURequest::PasskeyInit, PasskeyClass::Any => CURequest::PasskeyInit,
PasskeyClass::Attested => CURequest::AttestedPasskeyInit, PasskeyClass::Attested => CURequest::AttestedPasskeyInit,
@ -445,7 +453,7 @@ pub(crate) async fn view_new_totp(
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token = get_cu_session(jar).await?; let cu_session_token = get_cu_session(&jar).await?;
let push_url = HxPushUrl(Uri::from_static("/ui/reset/add_totp")); let push_url = HxPushUrl(Uri::from_static("/ui/reset/add_totp"));
let cu_status = state let cu_status = state
@ -497,7 +505,7 @@ pub(crate) async fn add_totp(
jar: CookieJar, jar: CookieJar,
new_totp_form: Form<NewTotp>, new_totp_form: Form<NewTotp>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token = get_cu_session(jar).await?; let cu_session_token = get_cu_session(&jar).await?;
let check_totpcode = u32::from_str(&new_totp_form.check_totpcode).unwrap_or_default(); let check_totpcode = u32::from_str(&new_totp_form.check_totpcode).unwrap_or_default();
@ -569,7 +577,7 @@ pub(crate) async fn view_new_pwd(
jar: CookieJar, jar: CookieJar,
opt_form: Option<Form<NewPassword>>, opt_form: Option<Form<NewPassword>>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let swapped_handler_trigger = let swapped_handler_trigger =
HxResponseTrigger::after_swap([HxEvent::new("addPasswordSwapped".to_string())]); HxResponseTrigger::after_swap([HxEvent::new("addPasswordSwapped".to_string())]);
@ -679,10 +687,9 @@ fn add_cu_cookie(
state: &ServerState, state: &ServerState,
cu_session_token: CUSessionToken, cu_session_token: CUSessionToken,
) -> CookieJar { ) -> CookieJar {
let mut token_cookie = Cookie::new(COOKIE_CU_SESSION_TOKEN, cu_session_token.token); let mut token_cookie =
token_cookie.set_secure(state.secure_cookies); cookies::make_unsigned(state, COOKIE_CU_SESSION_TOKEN, cu_session_token.token, "/");
token_cookie.set_same_site(SameSite::Strict); token_cookie.set_same_site(SameSite::Strict);
token_cookie.set_http_only(true);
jar.add(token_cookie) jar.add(token_cookie)
} }
@ -694,7 +701,7 @@ pub(crate) async fn view_set_unixcred(
jar: CookieJar, jar: CookieJar,
opt_form: Option<Form<NewPassword>>, opt_form: Option<Form<NewPassword>>,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
let cu_session_token: CUSessionToken = get_cu_session(jar).await?; let cu_session_token: CUSessionToken = get_cu_session(&jar).await?;
let swapped_handler_trigger = let swapped_handler_trigger =
HxResponseTrigger::after_swap([HxEvent::new("addPasswordSwapped".to_string())]); HxResponseTrigger::after_swap([HxEvent::new("addPasswordSwapped".to_string())]);
@ -781,7 +788,7 @@ pub(crate) async fn view_reset_get(
| OperationError::InvalidState, | OperationError::InvalidState,
) => { ) => {
// If our previous credential update session expired we want to see the reset form again. // If our previous credential update session expired we want to see the reset form again.
jar = jar.remove(Cookie::from(COOKIE_CU_SESSION_TOKEN)); jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN);
if let Some(token) = params.token { if let Some(token) = params.token {
let token_uri_string = format!("{}?token={}", Urls::CredReset, token); let token_uri_string = format!("{}?token={}", Urls::CredReset, token);
@ -917,7 +924,7 @@ fn get_cu_response(
} }
} }
async fn get_cu_session(jar: CookieJar) -> Result<CUSessionToken, Response> { async fn get_cu_session(jar: &CookieJar) -> Result<CUSessionToken, Response> {
let cookie = jar.get(COOKIE_CU_SESSION_TOKEN); let cookie = jar.get(COOKIE_CU_SESSION_TOKEN);
if let Some(cookie) = cookie { if let Some(cookie) = cookie {
let cu_session_token = cookie.value(); let cu_session_token = cookie.value();

View file

@ -147,7 +147,8 @@
<div class="mt-2 pt-2 border-top"> <div class="mt-2 pt-2 border-top">
<button class="btn btn-danger" <button class="btn btn-danger"
hx-post="/ui/api/cu_cancel" hx-post="/ui/api/cu_cancel"
hx-target="#main">Discard Changes</button> hx-boost="false"
>Discard Changes</button>
<span class="d-inline-block" tabindex="0" <span class="d-inline-block" tabindex="0"
data-bs-toggle="tooltip" data-bs-toggle="tooltip"
data-bs-title="Unresolved warnings"> data-bs-title="Unresolved warnings">