swarm-nats-auth: give issued user JWTs an exp

The user JWT the callout issues carried no exp, so nats-server armed no
expiry timer and a connection kept its grant until the client dropped it:
a deleted or re-minted agent queue secret, a removed OIDC client or an
expired access token never reached a live connection.

Every issued user JWT now expires at the presented OIDC token's exp
(read from the introspection answer) or an hour after issue, whichever
is sooner; the agent queue-secret path has no exp of its own and gets
the hour. Every queue client reconnects through an auth callback that
re-reads or re-mints its credential, so the forced reconnect presents a
current one.

Fixes #5432

swarm-nats-auth/README.md

@@ -73,6 +73,11 @@ timeout`**, with a test asserting the relation — a responder that answers
   It is one request's whole budget: when the `IdP` refuses the responder's own
   secret, the re-read from the store and the one retry fit inside it, or the
   request is denied and the re-read lands for the next one.
+- **Every issued user JWT expires**, at the presented OIDC token's own `exp`
+  or an hour after issue, whichever is sooner. nats-server closes the
+  connection at that point, and the client's reconnect comes back through
+  this responder. That reconnect is what makes a deleted or re-minted
+  credential reach an open connection.
 
 ## What the tests do and do not cover
 

swarm-nats-auth/src/introspect.rs

@@ -15,8 +15,8 @@
 //!
 //! # `client_id` is modelled, and is not a second admission check
 //!
-//! `sub`/`scope`/`exp` stay unmodelled: modelling a field implies checking
-//! it. `client_id` differs in kind — `active` decides **whether** to admit,
+//! `sub`/`scope` stay unmodelled: modelling a field implies checking it.
+//! `client_id` differs in kind — `active` decides **whether** to admit,
 //! `client_id` decides **as whom**, which is what lets a grant be scoped to
 //! one hive. Admission stays one decision in one place; the identity is
 //! returned rather than a bool so "admitted but unscoped" cannot exist.
@@ -46,7 +46,7 @@ pub const INTROSPECTION_TIMEOUT: Duration = Duration::from_millis(1500);
 #[derive(Debug, PartialEq, Eq)]
 pub enum Answer {
     /// A 2xx: the caller, or `None` for every shape that is not an admission.
-    Caller(Option<String>),
+    Caller(Option<Caller>),
     /// A non-2xx, with its status: the `IdP` did not answer about the token
     /// at all. A 400 or 401 is how it refuses this responder's own
     /// credential, which `oidc_secret` re-reads and retries once before this
@@ -54,8 +54,17 @@ pub enum Answer {
     Refused(reqwest::StatusCode),
 }
 
-/// The two fields this responder acts on — one per decision. Authelia returns
-/// more (`sub`, `scope`, `exp`); modelling them would imply we check them.
+/// Who an introspected token belongs to, and until when.
+#[derive(Debug, PartialEq, Eq)]
+pub struct Caller {
+    /// The client that holds the token.
+    pub client_id: String,
+    /// The token's `exp`, in unix seconds, when the `IdP` names one.
+    pub expires: Option<i64>,
+}
+
+/// The fields this responder acts on. Authelia returns more (`sub`,
+/// `scope`); modelling them would imply we check them.
 #[derive(Debug, Deserialize)]
 struct IntrospectionResponse {
     /// Whether to admit at all.
@@ -66,6 +75,11 @@ struct IntrospectionResponse {
     /// [`IntrospectionResponse::caller`].
     #[serde(default)]
     client_id: Option<String>,
+    /// When the token expires. It admits nothing (`active` alone does) and
+    /// only caps the issued user JWT's `exp`, at which nats-server closes the
+    /// connection.
+    #[serde(default)]
+    exp: Option<i64>,
 }
 
 impl IntrospectionResponse {
@@ -80,17 +94,21 @@ impl IntrospectionResponse {
     /// empty component in a subject the grant is scoped to, and a scope
     /// assembled from a blank is not a narrower permission, it is a different
     /// one nobody reviewed.
-    fn caller(&self) -> Option<&str> {
+    fn caller(&self) -> Option<Caller> {
         if !self.active {
             return None;
         }
-        self.client_id.as_deref().filter(|id| !id.is_empty())
+        let client_id = self.client_id.as_deref().filter(|id| !id.is_empty())?;
+        Some(Caller {
+            client_id: client_id.to_owned(),
+            expires: self.exp,
+        })
     }
 }
 
 /// Ask the `IdP` whether `token` is currently valid, and as whom.
 ///
-/// Returns `Ok(Answer::Caller(Some(client_id)))` **only** on a 2xx whose
+/// Returns `Ok(Answer::Caller(Some(caller)))` **only** on a 2xx whose
 /// body says `active: true` and names the client. Every other 2xx is
 /// `Ok(Answer::Caller(None))` with the reason logged, a non-2xx is
 /// `Ok(Answer::Refused(_))`, and `Err` is a call that could not be made at all
@@ -139,7 +157,7 @@ pub async fn identify_caller(
         // rejected for no visible reason.
         tracing::warn!("introspection said active but named no client; denying");
     }
-    Ok(Answer::Caller(body.caller().map(str::to_owned)))
+    Ok(Answer::Caller(body.caller()))
 }
 
 #[cfg(test)]
@@ -177,7 +195,20 @@ mod tests {
     fn an_active_token_is_admitted_as_the_client_that_holds_it() {
         assert_eq!(
             parse(r#"{"active":true,"client_id":"hive-alpha","scope":"x"}"#).caller(),
-            Some("hive-alpha")
+            Some(Caller {
+                client_id: "hive-alpha".to_owned(),
+                expires: None,
+            })
+        );
+    }
+
+    #[test]
+    fn an_admitted_caller_carries_the_tokens_exp() {
+        assert_eq!(
+            parse(r#"{"active":true,"client_id":"hive-alpha","exp":1700000000}"#)
+                .caller()
+                .and_then(|c| c.expires),
+            Some(1_700_000_000)
         );
     }
 

swarm-nats-auth/src/main.rs

@@ -278,7 +278,9 @@ async fn serve(args: &Args) -> anyhow::Result<()> {
         // normal thing for a client to attempt and an abnormal thing to
         // grant. Neither the store nor introspection is reached until
         // something was presented.
-        let (caller, permissions) = match req
+        // `expires` is the presented credential's own expiry, when it has one:
+        // an agent's queue secret has none.
+        let (caller, permissions, expires) = match req
             .connect_opts
             .auth_token
             .as_deref()
@@ -289,15 +291,16 @@ async fn serve(args: &Args) -> anyhow::Result<()> {
             Some(agent_token::Presented::Agent(token)) => (
                 Some(format!("agent:{}", token.agent)),
                 agent_token::authorize(&policy, &store, &token).await,
+                None,
             ),
             Some(agent_token::Presented::Malformed(e)) => {
                 tracing::warn!(error = %e, "malformed agent token; denying");
-                (None, None)
+                (None, None, None)
             }
             Some(agent_token::Presented::Bearer(token)) => {
                 introspected(&policy, &http, args, &client_secret, token).await
             }
-            None => (None, None),
+            None => (None, None, None),
         };
         // The caller is an identifier, not a credential: a client id, or
         // `agent:<name>` for an agent token. One line per auth request, a connect
@@ -326,6 +329,7 @@ async fn serve(args: &Args) -> anyhow::Result<()> {
                 &req.server_id.id,
                 &req.user_nkey,
                 permissions,
+                expires,
             ),
             None => respond::deny(&issuer, &req.server_id.id, &req.user_nkey),
         };
@@ -439,8 +443,9 @@ async fn ensure_callout_keys(
     Ok(())
 }
 
-/// The OIDC path: who the `IdP` says presented `token`, and what the policy
-/// grants that client. `None` permissions is a denial.
+/// The OIDC path: who the `IdP` says presented `token`, what the policy
+/// grants that client, and when the token expires. `None` permissions is a
+/// denial.
 ///
 /// The caller is an identity or nothing — see `introspect`'s module docs.
 /// There is no "admitted, identity unknown" branch to write here because
@@ -451,7 +456,7 @@ async fn introspected(
     args: &Args,
     client_secret: &oidc_secret::OidcSecret,
     token: &str,
-) -> (Option<String>, Option<policy::Permissions>) {
+) -> (Option<String>, Option<policy::Permissions>, Option<i64>) {
     let deadline = tokio::time::Instant::now() + introspect::INTROSPECTION_TIMEOUT;
     let caller = client_secret
         .introspect(deadline, |secret| async move {
@@ -473,6 +478,7 @@ async fn introspected(
             tracing::warn!(error = ?e, "introspection failed; denying");
             None
         });
+    let (caller, expires) = caller.map_or((None, None), |c| (Some(c.client_id), c.expires));
     // Admission said who; the policy says what. A caller the `IdP`
     // vouches for but no rule matches is denied — see `policy`'s module
     // docs for why that is deny and not "connect with nothing".
@@ -486,7 +492,7 @@ async fn introspected(
             "authenticated client matches no policy rule; denying"
         );
     }
-    (caller, permissions)
+    (caller, permissions, expires)
 }
 
 #[cfg(test)]

swarm-nats-auth/src/oidc_secret.rs

@@ -21,7 +21,7 @@ use swarm_secret_client::client::{DEFAULT_CERT_MOUNT, Settings};
 use swarm_secret_client::path::{Kind, principal_prefix};
 use tokio::time::Instant;
 
-use crate::introspect::Answer;
+use crate::introspect::{Answer, Caller};
 use crate::keystore::KeyStore;
 
 /// The path under the store's KV mount holding `client_id`'s secret.
@@ -99,7 +99,7 @@ impl OidcSecret {
     ///
     /// # Errors
     /// When an attempt could not be made at all; the caller denies.
-    pub async fn introspect<F, Fut>(&self, deadline: Instant, attempt: F) -> Result<Option<String>>
+    pub async fn introspect<F, Fut>(&self, deadline: Instant, attempt: F) -> Result<Option<Caller>>
     where
         F: Fn(String) -> Fut,
         Fut: Future<Output = Result<Answer>>,
@@ -165,7 +165,10 @@ mod tests {
         move |secret| {
             calls.fetch_add(1, Ordering::SeqCst);
             std::future::ready(Ok(if secret == registered {
-                Answer::Caller(Some("hive-alpha".to_owned()))
+                Answer::Caller(Some(Caller {
+                    client_id: "hive-alpha".to_owned(),
+                    expires: None,
+                }))
             } else {
                 Answer::Refused(reqwest::StatusCode::UNAUTHORIZED)
             }))
@@ -203,7 +206,7 @@ mod tests {
             .introspect(budget(), idp("old", &calls))
             .await
             .expect("answered");
-        assert_eq!(caller.as_deref(), Some("hive-alpha"));
+        assert_eq!(caller.map(|c| c.client_id).as_deref(), Some("hive-alpha"));
         assert_eq!(calls.load(Ordering::SeqCst), 1);
         assert_eq!(reads.load(Ordering::SeqCst), 0);
     }
@@ -219,7 +222,7 @@ mod tests {
             .introspect(budget(), idp("new", &calls))
             .await
             .expect("answered");
-        assert_eq!(caller.as_deref(), Some("hive-alpha"));
+        assert_eq!(caller.map(|c| c.client_id).as_deref(), Some("hive-alpha"));
         assert_eq!(calls.load(Ordering::SeqCst), 2);
         assert_eq!(reads.load(Ordering::SeqCst), 1);
     }
@@ -279,7 +282,7 @@ mod tests {
             .introspect(budget(), idp("new", &calls))
             .await
             .expect("answered");
-        assert_eq!(caller.as_deref(), Some("hive-alpha"));
+        assert_eq!(caller.map(|c| c.client_id).as_deref(), Some("hive-alpha"));
         assert_eq!(calls.load(Ordering::SeqCst), 5);
         assert_eq!(reads.load(Ordering::SeqCst), 1);
     }
@@ -307,7 +310,7 @@ mod tests {
             .introspect(budget(), idp("new", &calls))
             .await
             .expect("answered");
-        assert_eq!(next.as_deref(), Some("hive-alpha"));
+        assert_eq!(next.map(|c| c.client_id).as_deref(), Some("hive-alpha"));
         assert_eq!(calls.load(Ordering::SeqCst), 2);
         assert_eq!(reads.load(Ordering::SeqCst), 1);
     }
@@ -326,7 +329,7 @@ mod tests {
             if headers.get(AUTHORIZATION).and_then(|v| v.to_str().ok()) == Some(&*expected) {
                 (
                     StatusCode::OK,
-                    r#"{"active":true,"client_id":"hive-alpha"}"#,
+                    r#"{"active":true,"client_id":"hive-alpha","exp":1700000000}"#,
                 )
             } else {
                 (StatusCode::UNAUTHORIZED, "")
@@ -369,7 +372,13 @@ mod tests {
             })
             .await
             .expect("answered");
-        assert_eq!(caller.as_deref(), Some("hive-alpha"));
+        assert_eq!(
+            caller,
+            Some(Caller {
+                client_id: "hive-alpha".to_owned(),
+                expires: Some(1_700_000_000),
+            })
+        );
         assert_eq!(reads.load(Ordering::SeqCst), 1);
     }
 }

swarm-nats-auth/src/respond.rs

@@ -35,11 +35,23 @@ use sha2::{Digest, Sha256};
 /// equality test would catch it drifting.
 const JWT_HEADER: &str = r#"{"typ":"JWT","alg":"ed25519-nkey"}"#;
 
+/// The longest an issued user JWT lives. At its `exp` nats-server closes the
+/// connection and the client reconnects through this responder, so this is
+/// how long a deleted or re-minted queue secret keeps a live connection.
+/// An hour matches the lifetime of the OIDC access tokens the other clients
+/// present, which caps their JWTs anyway; the cost is one reconnect per
+/// client per hour.
+const MAX_USER_JWT_LIFETIME: std::time::Duration = std::time::Duration::from_hours(1);
+
 /// A NATS JWT claim set, with `aud` — which the callout response needs and
 /// `nats-jwt`'s own `Claims` lacks.
 #[derive(Serialize)]
 struct Claims<T: Serialize> {
     iat: i64,
+    /// Unix seconds. nats-server closes a connection admitted with this JWT
+    /// when it passes.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    exp: Option<i64>,
     iss: String,
     jti: String,
     sub: String,
@@ -108,6 +120,7 @@ fn response(
     sign(
         Claims {
             iat: now,
+            exp: None,
             iss: issuer.public_key(),
             jti: String::new(),
             sub: user_nkey.to_owned(),
@@ -205,16 +218,25 @@ impl ResponsePermission {
 /// `account` is the account **name** from the server's `accounts` block (the
 /// module's `clientAccount`), not a public key — in config mode the server
 /// resolves `aud` against its own config rather than against a key.
+///
+/// It expires at `credential_expires` (the presented credential's own
+/// expiry, unix seconds) or [`MAX_USER_JWT_LIFETIME`] after `now`, whichever
+/// is sooner.
 fn user_jwt(
     now: i64,
     issuer: &KeyPair,
     account: &str,
     user_nkey: &str,
     permissions: &crate::policy::Permissions,
+    credential_expires: Option<i64>,
 ) -> String {
+    let latest = now.saturating_add(
+        i64::try_from(MAX_USER_JWT_LIFETIME.as_secs()).expect("an hour fits in an i64"),
+    );
     sign(
         Claims {
             iat: now,
+            exp: Some(credential_expires.map_or(latest, |t| t.min(latest))),
             iss: issuer.public_key(),
             jti: String::new(),
             sub: user_nkey.to_owned(),
@@ -242,7 +264,8 @@ fn user_jwt(
 }
 
 /// Grant: mint a user JWT placing the client in `account`, scoped to
-/// `permissions`, and wrap it.
+/// `permissions` and expiring no later than `credential_expires` (see
+/// [`user_jwt`]), and wrap it.
 ///
 /// Taking the permissions by value rather than defaulting them is the point:
 /// there is no way to call this and get an unscoped grant by omission, so a
@@ -253,9 +276,17 @@ pub fn grant(
     server_id: &str,
     user_nkey: &str,
     permissions: &crate::policy::Permissions,
+    credential_expires: Option<i64>,
 ) -> String {
     let now = now_secs();
-    let jwt = user_jwt(now, issuer, account, user_nkey, permissions);
+    let jwt = user_jwt(
+        now,
+        issuer,
+        account,
+        user_nkey,
+        permissions,
+        credential_expires,
+    );
     response(now, issuer, server_id, user_nkey, Ok(jwt))
 }
 
@@ -337,6 +368,7 @@ mod tests {
         let mine = sign(
             Claims {
                 iat: decoded["iat"].as_i64().expect("iat"),
+                exp: None,
                 iss: account.public_key(),
                 jti: String::new(),
                 sub: user.public_key(),
@@ -383,13 +415,68 @@ mod tests {
         }
     }
 
+    /// The `exp` of a user JWT minted at `now` for a credential expiring at
+    /// `credential_expires`.
+    fn user_exp(now: i64, credential_expires: Option<i64>) -> serde_json::Value {
+        let account = KeyPair::new_account();
+        let jwt = user_jwt(
+            now,
+            &account,
+            "APP",
+            "UCLIENT",
+            &perms(),
+            credential_expires,
+        );
+        decode_claims(&jwt)["exp"].clone()
+    }
+
+    #[test]
+    fn the_issued_user_jwt_expires_with_a_credential_that_expires_sooner() {
+        let now = 1_700_000_000;
+        assert_eq!(user_exp(now, Some(now + 60)), now + 60);
+    }
+
+    #[test]
+    fn the_issued_user_jwt_expires_after_the_max_lifetime_otherwise() {
+        let now = 1_700_000_000;
+        let latest = now + 60 * 60;
+        assert_eq!(user_exp(now, None), latest);
+        assert_eq!(user_exp(now, Some(latest + 1)), latest);
+    }
+
+    #[test]
+    fn a_granted_user_jwt_carries_an_exp_after_its_iat() {
+        let account = KeyPair::new_account();
+        let wrapper = decode_claims(&grant(
+            &account,
+            "APP",
+            "NSERVER",
+            "UCLIENT",
+            &perms(),
+            None,
+        ));
+        let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
+        let (iat, exp) = (user["iat"].as_i64(), user["exp"].as_i64());
+        assert!(
+            iat.zip(exp).is_some_and(|(iat, exp)| exp > iat),
+            "iat {iat:?}, exp {exp:?}"
+        );
+    }
+
     #[test]
     fn the_issued_user_jwt_carries_the_permissions_it_was_given() {
         // The scoping is only real if it survives into the minted token. A
         // policy that computes the right subjects and a grant that drops them
         // look identical from every test that stops at the policy.
         let account = KeyPair::new_account();
-        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &perms()));
+        let wrapper = decode_claims(&grant(
+            &account,
+            "APP",
+            "NSERVER",
+            "UCLIENT",
+            &perms(),
+            None,
+        ));
         let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
         assert_eq!(user["nats"]["pub"]["allow"][0], "$KV.hive-status.alpha");
         // An empty or absent allow-list is NATS' "everything": the one shape
@@ -410,14 +497,21 @@ mod tests {
             subscribe: Some(vec!["_INBOX.agent.atlas.>".to_owned()]),
             ..perms()
         };
-        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &scoped));
+        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &scoped, None));
         let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
         assert_eq!(
             user["nats"]["sub"]["allow"],
             serde_json::json!(["_INBOX.agent.atlas.>"])
         );
 
-        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &perms()));
+        let wrapper = decode_claims(&grant(
+            &account,
+            "APP",
+            "NSERVER",
+            "UCLIENT",
+            &perms(),
+            None,
+        ));
         let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
         assert!(user["nats"]["sub"].is_null(), "{}", user["nats"]);
     }
@@ -429,7 +523,9 @@ mod tests {
             responses: Some(std::time::Duration::from_mins(31)),
             ..perms()
         };
-        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &answering));
+        let wrapper = decode_claims(&grant(
+            &account, "APP", "NSERVER", "UCLIENT", &answering, None,
+        ));
         let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
         // The names and units nats-server's `jwt/v2` reads: `max` messages,
         // `ttl` as a Go duration in nanoseconds.
@@ -438,7 +534,14 @@ mod tests {
             serde_json::json!({ "max": 1, "ttl": 31 * 60 * 1_000_000_000_i64 })
         );
 
-        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &perms()));
+        let wrapper = decode_claims(&grant(
+            &account,
+            "APP",
+            "NSERVER",
+            "UCLIENT",
+            &perms(),
+            None,
+        ));
         let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
         assert!(user["nats"]["resp"].is_null(), "{}", user["nats"]);
     }
@@ -446,7 +549,14 @@ mod tests {
     #[test]
     fn a_grant_carries_a_user_jwt_and_no_error() {
         let account = KeyPair::new_account();
-        let claims = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &perms()));
+        let claims = decode_claims(&grant(
+            &account,
+            "APP",
+            "NSERVER",
+            "UCLIENT",
+            &perms(),
+            None,
+        ));
         assert!(claims["nats"]["error"].is_null());
         assert!(
             claims["nats"]["jwt"]
@@ -470,7 +580,14 @@ mod tests {
     #[test]
     fn the_issued_user_jwt_names_its_account_by_aud_and_sets_no_issuer_account() {
         let account = KeyPair::new_account();
-        let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT", &perms()));
+        let wrapper = decode_claims(&grant(
+            &account,
+            "APP",
+            "NSERVER",
+            "UCLIENT",
+            &perms(),
+            None,
+        ));
         let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt"));
 
         // In server-config mode the account is named by `aud`...

swarm-nats-auth/src/xkey.rs

@@ -254,6 +254,7 @@ pub(crate) mod tests {
             &req.server_id.id,
             &req.user_nkey,
             &permissions,
+            None,
         );
         let reply = seal(&own, &server_key, token.as_bytes()).expect("sealed");
         assert_ne!(reply, token.as_bytes(), "the reply leaves sealed");