Limit OAuth2 resumption to session (#3296)

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.

Cargo.lock

@@ -1092,7 +1092,7 @@ dependencies = [
 
 [[package]]
 name = "daemon"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "clap",
  "clap_complete",
@@ -2831,7 +2831,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm-ipa-sync"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "chrono",
  "clap",
@@ -2855,7 +2855,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm-ldap-sync"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "chrono",
  "clap",
@@ -2880,7 +2880,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_build_profiles"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "base64 0.22.1",
  "gix",
@@ -2891,7 +2891,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_client"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "compact_jwt 0.4.2",
  "http 1.1.0",
@@ -2913,7 +2913,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_device_flow"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "anyhow",
  "base64 0.22.1",
@@ -2928,7 +2928,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_lib_crypto"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "argon2",
  "base64 0.22.1",
@@ -2949,7 +2949,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_lib_file_permissions"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "kanidm_utils_users",
  "whoami",
@@ -2957,7 +2957,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_proto"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "base32",
  "base64 0.22.1",
@@ -2982,7 +2982,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_tools"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "anyhow",
  "clap",
@@ -3013,7 +3013,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_unix_common"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "bytes",
  "csv",
@@ -3032,7 +3032,7 @@ dependencies = [
 
 [[package]]
 name = "kanidm_unix_int"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "async-trait",
  "bytes",
@@ -3073,14 +3073,14 @@ dependencies = [
 
 [[package]]
 name = "kanidm_utils_users"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "libc",
 ]
 
 [[package]]
 name = "kanidmd_core"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "askama",
  "askama_axum",
@@ -3133,7 +3133,7 @@ dependencies = [
 
 [[package]]
 name = "kanidmd_lib"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "base64 0.22.1",
  "base64urlsafedata 0.5.0",
@@ -3186,7 +3186,7 @@ dependencies = [
 
 [[package]]
 name = "kanidmd_lib_macros"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -3195,7 +3195,7 @@ dependencies = [
 
 [[package]]
 name = "kanidmd_testkit"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "assert_cmd",
  "compact_jwt 0.4.2",
@@ -3658,7 +3658,7 @@ dependencies = [
 
 [[package]]
 name = "nss_kanidm"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "kanidm_unix_common",
  "lazy_static",
@@ -4016,7 +4016,7 @@ dependencies = [
 
 [[package]]
 name = "orca"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "async-trait",
  "chrono",
@@ -4062,7 +4062,7 @@ checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39"
 
 [[package]]
 name = "pam_kanidm"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "kanidm_unix_common",
  "libc",
@@ -4936,7 +4936,7 @@ dependencies = [
 
 [[package]]
 name = "scim_proto"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "base64urlsafedata 0.5.0",
  "peg",
@@ -5211,7 +5211,7 @@ dependencies = [
 
 [[package]]
 name = "sketching"
-version = "1.4.3"
+version = "1.4.4"
 dependencies = [
  "gethostname",
  "num_enum",

proto/src/internal/error.rs

@@ -244,6 +244,7 @@ pub enum OperationError {
     // Web UI
     UI0001ChallengeSerialisation,
     UI0002InvalidState,
+    UI0003InvalidOauth2Resume,
 
     // Unixd Things
     KU001InitWhileSessionActive,
@@ -422,9 +423,12 @@ impl OperationError {
             Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled.".into()),
             Self::MG0008SkipUpgradeAttempted => Some("Skip Upgrade Attempted.".into()),
             Self::PL0001GidOverlapsSystemRange => None,
+
             Self::SC0001IncomingSshPublicKey => None,
+
             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 |

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

@@ -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)
 }
 

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

@@ -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

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

@@ -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);