mart-w/kanidm
1
0
Fork 0
mirror of https://github.com/kanidm/kanidm.git synced 2025-02-23 12:37:00 +01:00
Code Issues Actions 16 Packages Projects Releases Wiki Activity

Limit OAuth2 resumption to session (#3296)

Browse source 
OAuth2 session resumption was accidentally made a permanent cookie
which led to continuing issues with it causing invalid redirections
after login. Make this a session only cookie.
This commit is contained in:
Firstyear 2024-12-17 11:37:16 +10:00 committed by GitHub
parent 6c3b8500a2
commit 7e9c33ab03
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 28 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
proto/src/internal/error.rs
View file

@ -268,6 +268,7 @@ pub enum OperationError {
// Web UI
UI0001ChallengeSerialisation,
UI0002InvalidState,
UI0003InvalidOauth2Resume,
// Unixd Things
KU001InitWhileSessionActive,
@ -476,6 +477,7 @@ impl OperationError {
Self::UI0001ChallengeSerialisation => Some("The WebAuthn challenge was unable to be serialised.".into()),
Self::UI0002InvalidState => Some("The credential update process returned an invalid state transition.".into()),
Self::UI0003InvalidOauth2Resume => Some("The server attemped to resume OAuth2, but no OAuth2 session is in progress.".into()),
Self::VL0001ValueSshPublicKeyString => None,
Self::VS0001IncomingReplSshPublicKey => None,
Self::VS0002CertificatePublicKeyDigest |

9
server/core/src/https/views/cookies.rs
View file

@ -10,11 +10,6 @@ pub fn destroy(jar: CookieJar, ck_id: &str) -> CookieJar {
if let Some(ck) = jar.get(ck_id) {
let mut ck = ck.clone();
ck.make_removal();
/*
if let Some(path) = ck.path().cloned() {
ck.set_path(&path);
}
*/
jar.add(ck)
} else {
jar
@ -37,8 +32,6 @@ pub fn make_unsigned<'a>(
// then webauthn won't work anyway!
token_cookie.set_domain(state.domain.clone());
token_cookie.set_path(path);
// These last forever.
token_cookie.make_permanent();
token_cookie
}
@ -71,8 +64,6 @@ pub fn make_signed<'a, T: Serialize>(
token_cookie.set_http_only(true);
token_cookie.set_path(path);
token_cookie.set_domain(state.domain.clone());
// These last forever, we have our own internal expiration handling.
token_cookie.make_permanent();
Some(token_cookie)
}

7
server/core/src/https/views/login.rs
View file

@ -926,22 +926,25 @@ async fn view_login_step(
// Important - this can be make unsigned as token_str has it's own
// signatures.
let bearer_cookie = cookies::make_unsigned(
let mut bearer_cookie = cookies::make_unsigned(
&state,
COOKIE_BEARER_TOKEN,
token_str.clone(),
"/",
);
// Important - can be permanent as the token has its own expiration time internally
bearer_cookie.make_permanent();
jar = if session_context.remember_me {
// Important - can be unsigned as username is just for remember
// me and no other purpose.
let username_cookie = cookies::make_unsigned(
let mut username_cookie = cookies::make_unsigned(
&state,
COOKIE_USERNAME,
session_context.username.clone(),
Urls::Login.as_ref(),
);
username_cookie.make_permanent();
jar.add(username_cookie)
} else {
jar

6
server/core/src/https/views/oauth2.rs
View file

@ -112,7 +112,7 @@ async fn oauth2_auth_req(
return (
jar,
UnrecoverableErrorView {
err_code: OperationError::InvalidState,
err_code: OperationError::UI0003InvalidOauth2Resume,
operation_id: kopid.eventid,
},
)
@ -176,6 +176,10 @@ async fn oauth2_auth_req(
cookies::make_signed(&state, COOKIE_OAUTH2_REQ, &auth_req, Urls::Ui.as_ref())
.map(|mut cookie| {
cookie.set_same_site(SameSite::Strict);
// Expire at the end of the session.
cookie.set_expires(None);
// Could experiment with this to a shorter value, but session should be enough.
cookie.set_max_age(None);
jar.add(cookie)
})
.ok_or(OperationError::InvalidSessionState);