From d0e57442d206f6dae6fddd79987a8ff12db8e6c9 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Mon, 15 Jul 2024 16:16:24 +1000 Subject: [PATCH] Tidy up replication poll interval (#2883) --- proto/src/constants.rs | 11 +++++++++++ server/core/src/repl/config.rs | 22 ++++++++++++++++------ server/lib/src/constants/mod.rs | 5 ----- server/lib/src/idm/account.rs | 2 +- server/lib/src/idm/oauth2.rs | 4 ++-- server/lib/src/idm/server.rs | 4 ++-- server/lib/src/idm/serviceaccount.rs | 4 ++-- server/lib/src/plugins/session.rs | 8 ++++---- 8 files changed, 38 insertions(+), 22 deletions(-) diff --git a/proto/src/constants.rs b/proto/src/constants.rs index d3b477fa8..07e14d8be 100644 --- a/proto/src/constants.rs +++ b/proto/src/constants.rs @@ -2,6 +2,8 @@ //! pub mod uri; +use std::time::Duration; + /// The default location for the `kanidm` CLI tool's token cache. pub const CLIENT_TOKEN_CACHE: &str = "~/.cache/kanidm_tokens"; @@ -41,6 +43,15 @@ pub const DEFAULT_LDAP_LOCALHOST: &str = "localhost:636"; pub const DEFAULT_REPLICATION_ADDRESS: &str = "127.0.0.1:8444"; pub const DEFAULT_REPLICATION_ORIGIN: &str = "repl://localhost:8444"; +/// Default replication poll window in seconds. +pub const DEFAULT_REPL_TASK_POLL_INTERVAL: u64 = 15; + +/// Default grace window for authentication tokens. This allows a token to be +/// validated by another replica before the backing database session has been +/// replicated to the partner. If replication stalls until this point then +/// the token will be considered INVALID. +pub const AUTH_TOKEN_GRACE_WINDOW: Duration = Duration::from_secs(5 * 60); + // IF YOU CHANGE THESE VALUES YOU BREAK EVERYTHING pub const ATTR_ACCOUNT_EXPIRE: &str = "account_expire"; pub const ATTR_ACCOUNT_VALID_FROM: &str = "account_valid_from"; diff --git a/server/core/src/repl/config.rs b/server/core/src/repl/config.rs index 4bac6b7a1..e8150f44e 100644 --- a/server/core/src/repl/config.rs +++ b/server/core/src/repl/config.rs @@ -1,6 +1,9 @@ use kanidm_lib_crypto::prelude::X509; use kanidm_lib_crypto::serialise::x509b64; -use kanidm_proto::constants::{DEFAULT_REPLICATION_ADDRESS, DEFAULT_REPLICATION_ORIGIN}; +use kanidm_proto::constants::{ + AUTH_TOKEN_GRACE_WINDOW, DEFAULT_REPLICATION_ADDRESS, DEFAULT_REPLICATION_ORIGIN, + DEFAULT_REPL_TASK_POLL_INTERVAL, +}; use serde::Deserialize; use std::collections::BTreeMap; use std::net::SocketAddr; @@ -43,7 +46,9 @@ pub struct ReplicationConfiguration { pub origin: Url, /// Defaults to [kanidm_proto::constants::DEFAULT_REPLICATION_ADDRESS] pub bindaddress: SocketAddr, - /// Number of seconds between running a replication event + /// Number of seconds between running a replication event. Defaults to + /// [kanidm_proto::constants::DEFAULT_REPL_TASK_POLL_INTERVAL] but may + /// not exceed [kanidm_proto::constants::AUTH_TOKEN_GRACE_WINDOW]. pub task_poll_interval: Option, #[serde(flatten)] @@ -69,14 +74,19 @@ impl Default for ReplicationConfiguration { } } -const DEFAULT_REPL_TASK_POLL_INTERVAL: u64 = 15; - impl ReplicationConfiguration { /// Get the task poll interval, or the default if not set. pub(crate) fn get_task_poll_interval(&self) -> core::time::Duration { - core::time::Duration::from_secs( + let config_poll = core::time::Duration::from_secs( self.task_poll_interval .unwrap_or(DEFAULT_REPL_TASK_POLL_INTERVAL), - ) + ); + + // Don't allow the replication poll to exceed gracewindow. + if config_poll > AUTH_TOKEN_GRACE_WINDOW { + AUTH_TOKEN_GRACE_WINDOW + } else { + config_poll + } } } diff --git a/server/lib/src/constants/mod.rs b/server/lib/src/constants/mod.rs index 4159230ef..d9bb71096 100644 --- a/server/lib/src/constants/mod.rs +++ b/server/lib/src/constants/mod.rs @@ -137,11 +137,6 @@ pub const DEFAULT_AUTH_SESSION_LIMITED_EXPIRY: u32 = 3600; // Default - oauth refresh tokens last for 16 hours. pub const OAUTH_REFRESH_TOKEN_EXPIRY: u64 = 3600 * 16; -// The time that a token can be used before session -// status is enforced. This needs to be longer than -// replication delay/cycle. -pub const GRACE_WINDOW: Duration = Duration::from_secs(300); - /// How long access tokens should last. This is NOT the length /// of the refresh token, which is bound to the issuing session. pub const OAUTH2_ACCESS_TOKEN_EXPIRY: u32 = 15 * 60; diff --git a/server/lib/src/idm/account.rs b/server/lib/src/idm/account.rs index b58f07986..49beecb52 100644 --- a/server/lib/src/idm/account.rs +++ b/server/lib/src/idm/account.rs @@ -723,7 +723,7 @@ impl Account { } } } else { - let grace = uat.issued_at + GRACE_WINDOW; + let grace = uat.issued_at + AUTH_TOKEN_GRACE_WINDOW; let current = time::OffsetDateTime::UNIX_EPOCH + ct; trace!(%grace, %current); if current >= grace { diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs index ed4c19298..280fc97bb 100644 --- a/server/lib/src/idm/oauth2.rs +++ b/server/lib/src/idm/oauth2.rs @@ -3971,7 +3971,7 @@ mod tests { assert!(intr_response.active); // Grace window passed, it will now be invalid. - let ct = ct + GRACE_WINDOW; + let ct = ct + AUTH_TOKEN_GRACE_WINDOW; let intr_response = idms_prox_read .check_oauth2_token_introspect(&client_authz, &intr_request, ct) .expect("Failed to inspect token"); @@ -6230,7 +6230,7 @@ mod tests { assert!(idms_prox_write.commit().is_ok()); // Now must be invalid. - let ct = ct + GRACE_WINDOW; + let ct = ct + AUTH_TOKEN_GRACE_WINDOW; let mut idms_prox_read = idms.proxy_read().await; let intr_request = AccessTokenIntrospectRequest { diff --git a/server/lib/src/idm/server.rs b/server/lib/src/idm/server.rs index 9472b2146..36b226d0e 100644 --- a/server/lib/src/idm/server.rs +++ b/server/lib/src/idm/server.rs @@ -513,7 +513,7 @@ pub trait IdmServerTransaction<'a> { // We enforce both sessions are present in case of inconsistency // that may occur with replication. - let grace_valid = ct < (Duration::from_secs(iat as u64) + GRACE_WINDOW); + let grace_valid = ct < (Duration::from_secs(iat as u64) + AUTH_TOKEN_GRACE_WINDOW); let oauth2_session = entry .get_ava_as_oauth2session_map(Attribute::OAuth2Session) @@ -3506,7 +3506,7 @@ mod tests { let ct = duration_from_epoch_now(); - let post_grace = ct + GRACE_WINDOW + Duration::from_secs(1); + let post_grace = ct + AUTH_TOKEN_GRACE_WINDOW + Duration::from_secs(1); let expiry = ct + Duration::from_secs(DEFAULT_AUTH_SESSION_EXPIRY as u64 + 1); // Assert that our grace time is less than expiry, so we know the failure is due to diff --git a/server/lib/src/idm/serviceaccount.rs b/server/lib/src/idm/serviceaccount.rs index 78afc7e4e..7841ecccf 100644 --- a/server/lib/src/idm/serviceaccount.rs +++ b/server/lib/src/idm/serviceaccount.rs @@ -110,7 +110,7 @@ impl ServiceAccount { security_info!("A valid session value exists for this token"); true } else { - let grace = apit.issued_at + GRACE_WINDOW; + let grace = apit.issued_at + AUTH_TOKEN_GRACE_WINDOW; let current = time::OffsetDateTime::UNIX_EPOCH + ct; trace!(%grace, %current); if current >= grace { @@ -441,7 +441,7 @@ mod tests { _idms_delayed: &mut IdmServerDelayed, ) { let ct = Duration::from_secs(TEST_CURRENT_TIME); - let past_grc = Duration::from_secs(TEST_CURRENT_TIME + 1) + GRACE_WINDOW; + let past_grc = Duration::from_secs(TEST_CURRENT_TIME + 1) + AUTH_TOKEN_GRACE_WINDOW; let exp = Duration::from_secs(TEST_CURRENT_TIME + 6000); let post_exp = Duration::from_secs(TEST_CURRENT_TIME + 6010); let mut idms_prox_write = idms.proxy_write(ct).await; diff --git a/server/lib/src/plugins/session.rs b/server/lib/src/plugins/session.rs index 1ba9e2082..d0edec337 100644 --- a/server/lib/src/plugins/session.rs +++ b/server/lib/src/plugins/session.rs @@ -159,7 +159,7 @@ impl SessionConsistency { None } else { // Can't find the parent. Are we within grace window - if session.issued_at + GRACE_WINDOW <= curtime_odt { + if session.issued_at + AUTH_TOKEN_GRACE_WINDOW <= curtime_odt { info!(%o2_session_id, parent_id = ?session.parent, "Removing orphaned oauth2 session"); Some(PartialValue::Refer(*o2_session_id)) } else { @@ -320,7 +320,7 @@ mod tests { let cred_id = cred.uuid; // Set exp to gracewindow. - let exp_curtime = curtime + GRACE_WINDOW; + let exp_curtime = curtime + AUTH_TOKEN_GRACE_WINDOW; let exp_curtime_odt = OffsetDateTime::UNIX_EPOCH + exp_curtime; // Create a user @@ -491,7 +491,7 @@ mod tests { async fn test_session_consistency_oauth2_removed_by_parent(server: &QueryServer) { let curtime = duration_from_epoch_now(); let curtime_odt = OffsetDateTime::UNIX_EPOCH + curtime; - let exp_curtime = curtime + GRACE_WINDOW; + let exp_curtime = curtime + AUTH_TOKEN_GRACE_WINDOW; let p = CryptoPolicy::minimum(); let cred = Credential::new_password_only(&p, "test_password").unwrap(); @@ -665,7 +665,7 @@ mod tests { let curtime_odt = OffsetDateTime::UNIX_EPOCH + curtime; // Set exp to gracewindow. - let exp_curtime = curtime + GRACE_WINDOW; + let exp_curtime = curtime + AUTH_TOKEN_GRACE_WINDOW; // let exp_curtime_odt = OffsetDateTime::UNIX_EPOCH + exp_curtime; // Create a user