fix(auth): add and enforce token_type, bind refresh to access, derive live perms

crates/auth/src/api/handlers/auth.rs

@@ -13,11 +13,25 @@ use tracing::{debug, error, info, warn};
 use validator::Validate;
 
 use crate::api::handlers::AuthUiStaticFiles;
+use crate::auth::token::Claims;
 use crate::auth::validation::{sanitize_identifier, sanitize_string, ValidatedJson};
 use crate::server::AppState;
 use crate::storage::models::Key;
 use crate::AuthError;
 
+/// Expiry leeway applied by the refresh endpoint's "access token must already
+/// be expired" policy. Matches jsonwebtoken's default `Validation::leeway` so
+/// the two notions of "expired" cannot disagree.
+const JWT_EXPIRY_LEEWAY_SECS: u64 = 60;
+
+/// Whether an access token and a refresh token belong to the same subject.
+///
+/// Enforces finding #3: the refresh endpoint must reject a request that pairs a
+/// refresh token with an access token issued to a different subject.
+fn tokens_share_subject(access: &Claims, refresh: &Claims) -> bool {
+    access.sub == refresh.sub
+}
+
 // Common response type used by all helper functions
 type ApiResponse = (StatusCode, HeaderMap, Json<serde_json::Value>);
 
@@ -351,32 +365,40 @@ pub async fn refresh_token_handler(
     headers: HeaderMap,
     ValidatedJson(refresh_request): ValidatedJson<RefreshTokenRequest>,
 ) -> impl IntoResponse {
-    // First check if access token is still valid
-    match state
+    // Decode the access token exactly once, skipping only expiry enforcement:
+    // the refresh flow must read the claims of an already-expired token.
+    // Signature validity and the Access token type are still enforced
+    // (finding #1), so a refresh token in the access slot is rejected here.
+    let access_claims = match state
         .0
         .token_generator
-        .verify_token(&refresh_request.access_token)
+        .decode_access_claims_ignore_expiry(&refresh_request.access_token)
         .await
     {
-        Ok(_) => {
-            return error_response(StatusCode::UNAUTHORIZED, "Access token still valid", None);
-        }
+        Ok(claims) => claims,
         Err(err) => {
-            if !matches!(err, AuthError::TokenExpired) {
-                return error_response(
-                    StatusCode::UNAUTHORIZED,
-                    format!("Invalid access token: {err}"),
-                    None,
-                );
-            }
+            return error_response(
+                StatusCode::UNAUTHORIZED,
+                format!("Invalid access token: {err}"),
+                None,
+            );
         }
     };
 
-    // Verify the refresh token and extract claims
+    // Refresh policy: only an expired access token may be exchanged. The leeway
+    // mirrors jsonwebtoken's default `Validation::leeway`, so a token this
+    // service would still accept as a bearer credential cannot simultaneously
+    // be declared expired here.
+    let now = u64::try_from(chrono::Utc::now().timestamp()).unwrap_or_default();
+    if access_claims.exp.saturating_add(JWT_EXPIRY_LEEWAY_SECS) >= now {
+        return error_response(StatusCode::UNAUTHORIZED, "Access token still valid", None);
+    }
+
+    // Verify the refresh token and extract claims (must be a refresh token).
     let refresh_claims = match state
         .0
         .token_generator
-        .verify_token(&refresh_request.refresh_token)
+        .verify_refresh_token(&refresh_request.refresh_token)
         .await
     {
         Ok(claims) => claims,
@@ -389,6 +411,21 @@ pub async fn refresh_token_handler(
         }
     };
 
+    // Bind access <-> refresh: both must belong to the same subject (finding #3).
+    // Without this, any valid refresh token could be paired with an unrelated
+    // expired access token to mint a fresh pair for the refresh token's subject.
+    if !tokens_share_subject(&access_claims, &refresh_claims) {
+        warn!(
+            "Refresh rejected: access/refresh subject mismatch ({} != {})",
+            access_claims.sub, refresh_claims.sub
+        );
+        return error_response(
+            StatusCode::UNAUTHORIZED,
+            "Access and refresh tokens do not belong to the same subject",
+            None,
+        );
+    }
+
     // Check node URL if token has node information
     if let Some(token_node_url) = &refresh_claims.node_url {
         if let Err(error_msg) = state
@@ -1047,6 +1084,45 @@ pub async fn mock_token_handler(
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::auth::token::TokenType;
+
+    fn claims_for(sub: &str) -> Claims {
+        Claims {
+            sub: sub.to_string(),
+            iss: "calimero-test".to_string(),
+            aud: "calimero-test".to_string(),
+            exp: 0,
+            iat: 0,
+            jti: "jti".to_string(),
+            token_type: TokenType::Access,
+            permissions: vec![],
+            node_url: None,
+        }
+    }
+
+    #[test]
+    fn tokens_share_subject_accepts_same_subject() {
+        // finding #3: matching subjects bind the access/refresh pair.
+        assert!(tokens_share_subject(
+            &claims_for("user-a"),
+            &claims_for("user-a")
+        ));
+    }
+
+    #[test]
+    fn tokens_share_subject_rejects_mismatched_subject() {
+        // finding #3: a refresh token must not be paired with an access token
+        // issued to a different subject.
+        assert!(!tokens_share_subject(
+            &claims_for("user-a"),
+            &claims_for("user-b")
+        ));
+    }
+}
+
 #[cfg(test)]
 mod callback_xss_tests {
     use super::{safe_callback_js, DEFAULT_CALLBACK};