Repository: sebadob/rauthy (language: rust)

Issue:
auth_time in id_token does not conform to OIDC standard
I've been having some trouble with the package [oidc-client-ts](url)

I've been receiving the error "" upon renew of the token. The package expects upon token renew the auth_time of the new id_token to be the exact same as the auth_time of the first issued id_token as per spec: [https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse](https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse)

It seems Rauthy sends the time of authentication as the auth_time instead of the time of initial authentication when SESSION_LIFETIME is not defined.
https://github.com/sebadob/rauthy/blob/bd47f42d1d0b97ee4760efbb26ebbf22af5aee94/src/service/src/token_set.rs#L228

For reference, I've been using the Admin account on version 24.1 and 25.0 sqlite docker image.

How much work would it be to implement the 'more accurate time' as describe in code comments for the id token creation?


Discussion / hints:
Thanks for the issue. 

I am currently in the middle of a big cache migration under the hood, but I had a quick look. Yes it's true, that it will always re-calculate instead of re-using the original `auth_time` when it's a refresh. I will fix this.

The amount of work is not too big. I will probably finish the cache migration today (hopefully) and then can fix this.

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31e3dcd59..03ad69a27 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -121,7 +121,13 @@ fine, but it has some issues I wanted to get rid of.
 I started the [Hiqlite](https://github.com/sebadob/hiqlite) project some time ago to get rid of these things and have
 additional features. It is outsourced to make it generally usable in other contexts as well.
 
-[]()
+[0919767](https://github.com/sebadob/rauthy/commit/09197670e6491f83a8b739c0f195d4b842abe771)
+
+### Bugfix
+
+- The `refresh_token` grant type on the `/token` endpoint did not set the original `auth_time` for the `id_token`, but
+  instead calculated it from `now()` each time.
+  []()
 
 ## v0.25.0
 
diff --git a/src/api/src/fed_cm.rs b/src/api/src/fed_cm.rs
index 3d2047cd6..8d3cf4ee7 100644
--- a/src/api/src/fed_cm.rs
+++ b/src/api/src/fed_cm.rs
@@ -1,6 +1,7 @@
 use actix_web::http::header;
 use actix_web::http::header::{HeaderName, HeaderValue};
 use actix_web::{get, post, web, HttpRequest, HttpResponse};
+use chrono::Utc;
 use rauthy_api_types::clients::EphemeralClientRequest;
 use rauthy_api_types::fed_cm::{FedCMAssertionRequest, FedCMClientMetadataRequest};
 use rauthy_common::constants::{
@@ -19,7 +20,7 @@ use rauthy_models::entity::fed_cm::{
 use rauthy_models::entity::sessions::Session;
 use rauthy_models::entity::users::User;
 use rauthy_models::ListenScheme;
-use rauthy_service::token_set::{AuthCodeFlow, DeviceCodeFlow, TokenNonce, TokenSet};
+use rauthy_service::token_set::{AuthCodeFlow, AuthTime, DeviceCodeFlow, TokenNonce, TokenSet};
 use tracing::{debug, error, warn};
 
 const HEADER_ALLOW_CREDENTIALS: (&str, &str) = ("access-control-allow-credentials", "true");
@@ -294,6 +295,7 @@ pub async fn post_fed_cm_token(
         &user,
         &data,
         &client,
+        AuthTime::given(user.last_login.unwrap_or_else(|| Utc::now().timestamp())),
         None,
         payload.nonce.map(TokenNonce),
         // TODO add something like `fedcm` to the scopes? Maybe depending on new allowed flow?
diff --git a/src/models/src/entity/jwk.rs b/src/models/src/entity/jwk.rs
index 1126c3a95..02354bd1f 100644
--- a/src/models/src/entity/jwk.rs
+++ b/src/models/src/entity/jwk.rs
@@ -1031,6 +1031,7 @@ mod tests {
                 azp: "some_azp".to_string(),
                 typ: JwtTokenType::Refresh,
                 uid: "user_id_13337".to_string(),
+                auth_time: None,
                 cnf: None,
                 did: None,
             },
diff --git a/src/models/src/lib.rs b/src/models/src/lib.rs
index 268673e52..7e07403c3 100644
--- a/src/models/src/lib.rs
+++ b/src/models/src/lib.rs
@@ -236,6 +236,8 @@ pub struct JwtRefreshClaims {
     pub typ: JwtTokenType,
     pub uid: String,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub auth_time: Option<i64>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub cnf: Option<JktClaim>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub did: Option<String>,
diff --git a/src/service/src/oidc/grant_types/authorization_code.rs b/src/service/src/oidc/grant_types/authorization_code.rs
index d25e0a53c..8acbc57e9 100644
--- a/src/service/src/oidc/grant_types/authorization_code.rs
+++ b/src/service/src/oidc/grant_types/authorization_code.rs
@@ -1,8 +1,9 @@
 use crate::token_set::{
-    AuthCodeFlow, DeviceCodeFlow, DpopFingerprint, TokenNonce, TokenScopes, TokenSet,
+    AuthCodeFlow, AuthTime, DeviceCodeFlow, DpopFingerprint, TokenNonce, TokenScopes, TokenSet,
 };
 use actix_web::http::header::{HeaderName, HeaderValue};
 use actix_web::{web, HttpRequest};
+use chrono::Utc;
 use rauthy_api_types::oidc::TokenRequest;
 use rauthy_common::constants::HEADER_DPOP_NONCE;
 use rauthy_common::utils::{base64_url_encode, real_ip_from_req};
@@ -19,7 +20,10 @@ use std::str::FromStr;
 use time::OffsetDateTime;
 use tracing::warn;
 
-#[tracing::instrument(skip_all, fields(client_id = req_data.client_id, username = req_data.username))]
+#[tracing::instrument(
+    skip_all,
+    fields(client_id = req_data.client_id, username = req_data.username)
+)]
 pub async fn grant_type_authorization_code(
     data: &web::Data<AppState>,
     req: HttpRequest,
@@ -145,6 +149,7 @@ pub async fn grant_type_authorization_code(
         &user,
         data,
         &client,
+        AuthTime::given(user.last_login.unwrap_or_else(|| Utc::now().timestamp())),
         dpop_fingerprint,
         code.nonce.clone().map(TokenNonce),
         Some(TokenScopes(code.scopes.join(" "))),
diff --git a/src/service/src/oidc/grant_types/device_code.rs b/src/service/src/oidc/grant_types/device_code.rs
index abc4b05f5..8db279da4 100644
--- a/src/service/src/oidc/grant_types/device_code.rs
+++ b/src/service/src/oidc/grant_types/device_code.rs
@@ -1,4 +1,4 @@
-use crate::token_set::{AuthCodeFlow, DeviceCodeFlow, TokenScopes, TokenSet};
+use crate::token_set::{AuthCodeFlow, AuthTime, DeviceCodeFlow, TokenScopes, TokenSet};
 use actix_web::{web, HttpResponse};
 use chrono::Utc;
 use rauthy_api_types::oidc::{OAuth2ErrorResponse, OAuth2ErrorTypeResponse, TokenRequest};
@@ -164,6 +164,7 @@ pub async fn grant_type_device_code(
             &user,
             data,
             &client,
+            AuthTime::now(),
             None,
             None,
             code.scopes.map(TokenScopes),
diff --git a/src/service/src/oidc/grant_types/password.rs b/src/service/src/oidc/grant_types/password.rs
index 99befea94..9e265201a 100644
--- a/src/service/src/oidc/grant_types/password.rs
+++ b/src/service/src/oidc/grant_types/password.rs
@@ -1,4 +1,4 @@
-use crate::token_set::{AuthCodeFlow, DeviceCodeFlow, DpopFingerprint, TokenSet};
+use crate::token_set::{AuthCodeFlow, AuthTime, DeviceCodeFlow, DpopFingerprint, TokenSet};
 use actix_web::http::header::{HeaderName, HeaderValue};
 use actix_web::{web, HttpRequest};
 use rauthy_api_types::oidc::TokenRequest;
@@ -97,6 +97,7 @@ pub async fn grant_type_password(
                 &user,
                 data,
                 &client,
+                AuthTime::now(),
                 dpop_fingerprint,
                 None,
                 None,
diff --git a/src/service/src/oidc/validation.rs b/src/service/src/oidc/validation.rs
index 413cce050..35b97306b 100644
--- a/src/service/src/oidc/validation.rs
+++ b/src/service/src/oidc/validation.rs
@@ -1,4 +1,6 @@
-use crate::token_set::{AuthCodeFlow, DeviceCodeFlow, DpopFingerprint, TokenScopes, TokenSet};
+use crate::token_set::{
+    AuthCodeFlow, AuthTime, DeviceCodeFlow, DpopFingerprint, TokenScopes, TokenSet,
+};
 use actix_web::http::header::{HeaderName, HeaderValue};
 use actix_web::{web, HttpRequest};
 use jwt_simple::claims;
@@ -194,30 +196,28 @@ pub async fn validate_refresh_token(
     user.last_login = Some(OffsetDateTime::now_utc().unix_timestamp());
     user.save(data, None, None).await?;
 
-    let ts = if let Some(s) = rt_scope {
-        TokenSet::from_user(
-            &user,
-            data,
-            &client,
-            dpop_fingerprint,
-            None,
-            Some(TokenScopes(s)),
-            AuthCodeFlow::No,
-            DeviceCodeFlow::No,
-        )
-        .await
+    let auth_time = if let Some(ts) = claims.custom.auth_time {
+        AuthTime::given(ts)
     } else {
-        TokenSet::from_user(
-            &user,
-            data,
-            &client,
-            dpop_fingerprint,
-            None,
-            None,
-            AuthCodeFlow::No,
-            DeviceCodeFlow::No,
-        )
-        .await
-    }?;
+        // TODO this is not 100% correct but will make the migration from older to new refresh
+        // tokens smooth. In a future release, the `auth_time` can be set to required in the claims.
+        // Optional for now to still accept older tokens.
+        // As soon as no old refresh tokens exist anymore, this branch will never be used anyway.
+        AuthTime::now()
+    };
+
+    let ts = TokenSet::from_user(
+        &user,
+        data,
+        &client,
+        auth_time,
+        dpop_fingerprint,
+        None,
+        rt_scope.map(TokenScopes),
+        AuthCodeFlow::No,
+        DeviceCodeFlow::No,
+    )
+    .await?;
+
     Ok((ts, dpop_nonce))
 }
diff --git a/src/service/src/token_set.rs b/src/service/src/token_set.rs
index ec7298241..ff74521c1 100644
--- a/src/service/src/token_set.rs
+++ b/src/service/src/token_set.rs
@@ -6,7 +6,6 @@ use jwt_simple::prelude::{coarsetime, UnixTimeStamp};
 use rauthy_api_types::oidc::JktClaim;
 use rauthy_common::constants::{
     DEVICE_GRANT_REFRESH_TOKEN_LIFETIME, ENABLE_SOLID_AUD, ENABLE_WEB_ID, REFRESH_TOKEN_LIFETIME,
-    SESSION_LIFETIME,
 };
 use rauthy_common::utils::base64_url_no_pad_encode;
 use rauthy_error::{ErrorResponse, ErrorResponseType};
@@ -79,6 +78,23 @@ pub enum AuthCodeFlow {
     No,
 }
 
+#[derive(Debug, Clone)]
+pub struct AuthTime(i64);
+
+impl AuthTime {
+    pub fn now() -> Self {
+        Self(Utc::now().timestamp())
+    }
+
+    pub fn given(ts: i64) -> Self {
+        Self(ts)
+    }
+
+    pub fn get(&self) -> i64 {
+        self.0
+    }
+}
+
 #[derive(Debug, Clone, PartialEq)]
 pub enum DeviceCodeFlow {
     Yes(String),
@@ -199,6 +215,7 @@ impl TokenSet {
         user: &User,
         data: &web::Data<AppState>,
         client: &Client,
+        auth_time: AuthTime,
         dpop_fingerprint: Option<DpopFingerprint>,
         at_hash: AtHash,
         lifetime: i64,
@@ -207,33 +224,10 @@ impl TokenSet {
         scope_customs: Option<(Vec<&Scope>, &Option<HashMap<String, Vec<u8>>>)>,
         auth_code_flow: AuthCodeFlow,
     ) -> Result<String, ErrorResponse> {
-        let now_ts = Utc::now().timestamp();
-
-        // TODO the `auth_time` here is a bit inaccurate currently. The accuracy could be improved
-        // with future DB migrations by adding something like a `last_auth` column for each user.
-        // It is unclear right now, if we even need it right now.
-        let (amr, auth_time) = match user.has_webauthn_enabled() {
-            true => {
-                if auth_code_flow == AuthCodeFlow::Yes {
-                    // With active MFA, the auth_time is always 'now', because it must be re-validated each time
-                    (JwtAmrValue::Mfa.to_string(), now_ts)
-                } else {
-                    (
-                        JwtAmrValue::Pwd.to_string(),
-                        now_ts - *SESSION_LIFETIME as i64,
-                    )
-                }
-            }
-            false => {
-                if auth_code_flow == AuthCodeFlow::Yes {
-                    (JwtAmrValue::Pwd.to_string(), now_ts)
-                } else {
-                    (
-                        JwtAmrValue::Pwd.to_string(),
-                        now_ts - *SESSION_LIFETIME as i64,
-                    )
-                }
-            }
+        let amr = if user.has_webauthn_enabled() && auth_code_flow == AuthCodeFlow::Yes {
+            JwtAmrValue::Mfa.to_string()
+        } else {
+            JwtAmrValue::Pwd.to_string()
         };
 
         let webid =
@@ -243,7 +237,14 @@ impl TokenSet {
             azp: client.id.clone(),
             typ: JwtTokenType::Id,
             amr: vec![amr],
-            auth_time,
+            // TODO the `auth_time` here is a bit inaccurate currently. The accuracy could be improved
+            // with future DB migrations by adding something like a `last_auth` column for each user.
+            // It is unclear right now, if we even need it right now.
+            // A bit incorrect value might exist here in case a user does a refresh with an existing
+            // session with just a password or the need to refresh WebAuthn.
+            // -> add a new column to the `users` table with the Hiqlite migration to keep track
+            // of the 100% exact value.
+            auth_time: auth_time.get(),
             at_hash: at_hash.0,
             preferred_username: user.email.clone(),
             email: None,
@@ -368,6 +369,7 @@ impl TokenSet {
         data: &web::Data<AppState>,
         dpop_fingerprint: Option<DpopFingerprint>,
         client: &Client,
+        auth_time: AuthTime,
         access_token_lifetime: i64,
         scope: Option<TokenScopes>,
         is_mfa: bool,
@@ -383,6 +385,7 @@ impl TokenSet {
             azp: client.id.clone(),
             typ: JwtTokenType::Refresh,
             uid: user.id.clone(),
+            auth_time: Some(auth_time.get()),
             cnf: dpop_fingerprint.map(|jkt| JktClaim { jkt: jkt.0 }),
             did: did.clone(),
         };
@@ -473,6 +476,7 @@ impl TokenSet {
         user: &User,
         data: &web::Data<AppState>,
         client: &Client,
+        auth_time: AuthTime,
         dpop_fingerprint: Option<DpopFingerprint>,
         nonce: Option<TokenNonce>,
         scopes: Option<TokenScopes>,
@@ -583,6 +587,7 @@ impl TokenSet {
             user,
             data,
             client,
+            auth_time.clone(),
             dpop_fingerprint.clone(),
             at_hash,
             lifetime,
@@ -599,6 +604,7 @@ impl TokenSet {
                     data,
                     dpop_fingerprint,
                     client,
+                    auth_time,
                     lifetime,
                     scopes.map(TokenScopes),
                     user.has_webauthn_enabled(),


Test patch:
diff --git a/src/bin/tests/handler_auth.rs b/src/bin/tests/handler_auth.rs
index a8a69a07d..0a42cb8be 100644
--- a/src/bin/tests/handler_auth.rs
+++ b/src/bin/tests/handler_auth.rs
@@ -14,7 +14,9 @@ use rauthy_api_types::oidc::{
 use rauthy_common::constants::{
     APPLICATION_JSON, DPOP_TOKEN_ENDPOINT, HEADER_DPOP_NONCE, TOKEN_DPOP,
 };
-use rauthy_common::utils::{base64_encode, base64_url_encode, base64_url_no_pad_encode, get_rand};
+use rauthy_common::utils::{
+    base64_encode, base64_url_encode, base64_url_no_pad_decode, base64_url_no_pad_encode, get_rand,
+};
 use rauthy_error::{ErrorResponse, ErrorResponseType};
 use rauthy_models::entity::dpop_proof::{DPoPClaims, DPoPHeader};
 use rauthy_models::entity::jwk::{JWKSPublicKey, JwkKeyPairType, JWKS};
@@ -475,6 +477,8 @@ async fn test_concurrent_logins() -> Result<(), Box<dyn Error>> {
 
 #[tokio::test]
 async fn test_password_flow() -> Result<(), Box<dyn Error>> {
+    let before = Utc::now().timestamp();
+
     let url = format!("{}/oidc/token", get_backend_url());
     let mut body = TokenRequest {
         grant_type: "password".to_string(),
@@ -528,8 +532,16 @@ async fn test_password_flow() -> Result<(), Box<dyn Error>> {
     };
     validate_token(&ts.access_token, payload).await?;
 
+    // make sure `auth_time` is handled properly
+    let auth_time_refresh = auth_time_from_token(ts.refresh_token.as_ref().unwrap());
+    assert!(before <= auth_time_refresh);
+    assert!(auth_time_refresh <= Utc::now().timestamp());
+
+    let auth_time_orig = auth_time_from_token(ts.id_token.as_ref().unwrap());
+    assert_eq!(auth_time_refresh, auth_time_orig);
+
     // refresh it
-    time::sleep(Duration::from_secs(1)).await;
+    time::sleep(Duration::from_secs(2)).await;
     let req = TokenRequest {
         grant_type: "refresh_token".to_string(),
         code: None,
@@ -557,6 +569,13 @@ async fn test_password_flow() -> Result<(), Box<dyn Error>> {
     assert_ne!(ts.access_token, new_ts.access_token);
     assert_ne!(ts.id_token, new_ts.id_token);
 
+    let auth_time = auth_time_from_token(new_ts.id_token.as_ref().unwrap());
+    assert_eq!(auth_time_orig, auth_time);
+
+    let auth_time_refresh_new = auth_time_from_token(ts.refresh_token.as_ref().unwrap());
+    // the `auth_time` for the refresh token must always stay the original one
+    assert_eq!(auth_time_orig, auth_time_refresh_new);
+
     Ok(())
 }
 
@@ -1033,6 +1052,16 @@ async fn test_token_introspection() -> Result<(), Box<dyn Error>> {
     Ok(())
 }
 
+fn auth_time_from_token(id_token: &str) -> i64 {
+    let (_, rest) = id_token.split_once('.').unwrap_or(("", ""));
+    let (claims_b64, _) = rest.split_once('.').unwrap_or(("", ""));
+    let bytes = base64_url_no_pad_decode(claims_b64).unwrap();
+    let s = String::from_utf8_lossy(&bytes);
+    let claims = serde_json::from_str::<serde_json::Value>(s.as_ref()).unwrap();
+    let at = claims.get("auth_time").unwrap();
+    at.as_i64().unwrap()
+}
+
 async fn validate_token(
     access_token: &str,
     payload: TokenValidationRequest,