From c5d27d5490acf69840916514b91e4613676e91db Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Thu, 23 Feb 2023 11:19:55 -0500 Subject: [PATCH 1/8] deault to token expiry in 1 hour if none is provided --- src/types.rs | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/types.rs b/src/types.rs index 59e2112..4d99b09 100644 --- a/src/types.rs +++ b/src/types.rs @@ -36,7 +36,7 @@ impl Token { Token { inner: Arc::new(InnerToken { access_token, - expires_at: None, + expires_at: default_token_expiry(), }), } } @@ -55,11 +55,11 @@ impl fmt::Debug for Token { struct InnerToken { access_token: String, #[serde( - default, + default = "default_token_expiry", deserialize_with = "deserialize_time", rename(deserialize = "expires_in") )] - expires_at: Option, + expires_at: OffsetDateTime, } impl Token { @@ -68,12 +68,7 @@ impl Token { /// This takes an additional 30s margin to ensure the token can still be reasonably used /// instead of expiring right after having checked. pub fn has_expired(&self) -> bool { - self.inner - .expires_at - .map(|expiration_time| { - expiration_time - Duration::seconds(30) <= OffsetDateTime::now_utc() - }) - .unwrap_or(false) + self.inner.expires_at - Duration::seconds(30) <= OffsetDateTime::now_utc() } /// Get str representation of the token. @@ -82,7 +77,7 @@ impl Token { } /// Get expiry of token, if available - pub fn expires_at(&self) -> Option { + pub fn expires_at(&self) -> OffsetDateTime { self.inner.expires_at } } @@ -140,14 +135,16 @@ impl fmt::Debug for Signer { } } -fn deserialize_time<'de, D>(deserializer: D) -> Result, D::Error> +fn default_token_expiry() -> OffsetDateTime { + OffsetDateTime::now_utc() + Duration::seconds(3600) +} + +fn deserialize_time<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, { - let s: Option = Deserialize::deserialize(deserializer)?; - let s = - s.map(|seconds_from_now| OffsetDateTime::now_utc() + Duration::seconds(seconds_from_now)); - Ok(s) + let seconds_from_now: i64 = Deserialize::deserialize(deserializer)?; + Ok(OffsetDateTime::now_utc() + Duration::seconds(seconds_from_now)) } pub(crate) fn client() -> HyperClient { @@ -171,7 +168,7 @@ mod tests { let token = Token { inner: Arc::new(InnerToken { access_token: "abc123".to_string(), - expires_at: Some(OffsetDateTime::from_unix_timestamp(123).unwrap()), + expires_at: OffsetDateTime::from_unix_timestamp(123).unwrap(), }), }; let s = serde_json::to_string(&token).unwrap(); @@ -191,7 +188,7 @@ mod tests { assert_eq!(token.as_str(), "abc123"); // Testing time is always racy, give it 1s leeway. - let expires_at = token.expires_at().unwrap(); + let expires_at = token.expires_at(); assert!(expires_at < expires + Duration::seconds(1)); assert!(expires_at > expires - Duration::seconds(1)); } @@ -200,8 +197,9 @@ mod tests { fn test_deserialise_no_time() { let s = r#"{"access_token":"abc123"}"#; let token: Token = serde_json::from_str(s).unwrap(); + let expires = OffsetDateTime::now_utc() + Duration::seconds(3600); - assert_eq!(token.as_str(), "abc123"); - assert!(token.expires_at().is_none()); + assert!(token.expires_at() < expires + Duration::seconds(1)); + assert!(token.expires_at() > expires - Duration::seconds(1)); } } From f1af7fb02f019379ac8621a52de48727c0d6255f Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Thu, 23 Feb 2023 11:21:47 -0500 Subject: [PATCH 2/8] replace test assertion --- src/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/types.rs b/src/types.rs index 4d99b09..725760a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -199,6 +199,7 @@ mod tests { let token: Token = serde_json::from_str(s).unwrap(); let expires = OffsetDateTime::now_utc() + Duration::seconds(3600); + assert_eq!(token.as_str(), "abc123"); assert!(token.expires_at() < expires + Duration::seconds(1)); assert!(token.expires_at() > expires - Duration::seconds(1)); } From bf3f4f2fb7c3c764152ba73c29dc158af510b031 Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Thu, 23 Feb 2023 17:43:08 -0500 Subject: [PATCH 3/8] bump version to 0.7.6 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 30a43e9..1f7d015 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "gcp_auth" -version = "0.7.5" +version = "0.7.6" repository = "https://github.com/hrvolapeter/gcp_auth" description = "Google cloud platform (GCP) authentication using default and custom service accounts" documentation = "https://docs.rs/gcp_auth/" From c2ca082277d79db39c789be04f2ed6a6a30d50ee Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Fri, 24 Feb 2023 10:46:02 -0500 Subject: [PATCH 4/8] remove default expiry for deserialized tokens --- src/types.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/types.rs b/src/types.rs index 725760a..9c32c7c 100644 --- a/src/types.rs +++ b/src/types.rs @@ -36,7 +36,7 @@ impl Token { Token { inner: Arc::new(InnerToken { access_token, - expires_at: default_token_expiry(), + expires_at: OffsetDateTime::now_utc() + Duration::seconds(3600), }), } } @@ -55,7 +55,6 @@ impl fmt::Debug for Token { struct InnerToken { access_token: String, #[serde( - default = "default_token_expiry", deserialize_with = "deserialize_time", rename(deserialize = "expires_in") )] @@ -135,10 +134,6 @@ impl fmt::Debug for Signer { } } -fn default_token_expiry() -> OffsetDateTime { - OffsetDateTime::now_utc() + Duration::seconds(3600) -} - fn deserialize_time<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -194,12 +189,13 @@ mod tests { } #[test] - fn test_deserialise_no_time() { - let s = r#"{"access_token":"abc123"}"#; - let token: Token = serde_json::from_str(s).unwrap(); + fn test_token_from_string() { + let s = String::from("abc123"); + let token = Token::from_string(s); let expires = OffsetDateTime::now_utc() + Duration::seconds(3600); assert_eq!(token.as_str(), "abc123"); + assert!(!token.has_expired()); assert!(token.expires_at() < expires + Duration::seconds(1)); assert!(token.expires_at() > expires - Duration::seconds(1)); } From d089f342be2b806d45bacc8fa05a7ae875edb2a6 Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Mon, 27 Feb 2023 10:03:53 -0500 Subject: [PATCH 5/8] make default token duration a parameter to token fetch --- src/gcloud_authorized_user.rs | 11 ++++++----- src/types.rs | 13 +++++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/gcloud_authorized_user.rs b/src/gcloud_authorized_user.rs index 7446d64..19becaa 100644 --- a/src/gcloud_authorized_user.rs +++ b/src/gcloud_authorized_user.rs @@ -3,12 +3,13 @@ use std::process::Command; use std::sync::RwLock; use async_trait::async_trait; +use time::Duration; use which::which; use crate::authentication_manager::ServiceAccount; use crate::error::Error; use crate::error::Error::{GCloudError, GCloudNotFound, GCloudParseError}; -use crate::types::HyperClient; +use crate::types::{HyperClient, DEFAULT_TOKEN_DURATION}; use crate::Token; #[derive(Debug)] @@ -31,10 +32,10 @@ impl GCloudAuthorizedUser { } fn token(gcloud: &Path) -> Result { - Ok(Token::from_string(run( - gcloud, - &["auth", "print-access-token", "--quiet"], - )?)) + Ok(Token::from_string( + run(gcloud, &["auth", "print-access-token", "--quiet"])?, + Duration::seconds(DEFAULT_TOKEN_DURATION), + )) } } diff --git a/src/types.rs b/src/types.rs index 9c32c7c..76b90b2 100644 --- a/src/types.rs +++ b/src/types.rs @@ -13,6 +13,11 @@ use time::{Duration, OffsetDateTime}; use crate::Error; +/// The default number of seconds that it takes for a Google Cloud auth token to expire. +/// This appears to be the default from practical testing, but we have not found evidence +/// that this will always be the default duration. +pub(crate) const DEFAULT_TOKEN_DURATION: i64 = 3600; + /// Represents an access token. All access tokens are Bearer tokens. /// /// Tokens should not be cached, the [`AuthenticationManager`] handles the correct caching @@ -32,11 +37,11 @@ pub struct Token { } impl Token { - pub(crate) fn from_string(access_token: String) -> Self { + pub(crate) fn from_string(access_token: String, expires_in: Duration) -> Self { Token { inner: Arc::new(InnerToken { access_token, - expires_at: OffsetDateTime::now_utc() + Duration::seconds(3600), + expires_at: OffsetDateTime::now_utc() + expires_in, }), } } @@ -191,8 +196,8 @@ mod tests { #[test] fn test_token_from_string() { let s = String::from("abc123"); - let token = Token::from_string(s); - let expires = OffsetDateTime::now_utc() + Duration::seconds(3600); + let token = Token::from_string(s, Duration::seconds(DEFAULT_TOKEN_DURATION)); + let expires = OffsetDateTime::now_utc() + Duration::seconds(DEFAULT_TOKEN_DURATION); assert_eq!(token.as_str(), "abc123"); assert!(!token.has_expired()); From 18087842c97cbe72a4a89c719f8f2524a3619d1f Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Mon, 27 Feb 2023 12:39:17 -0500 Subject: [PATCH 6/8] change type of default duration const --- src/gcloud_authorized_user.rs | 3 +-- src/types.rs | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/gcloud_authorized_user.rs b/src/gcloud_authorized_user.rs index 19becaa..563e3fd 100644 --- a/src/gcloud_authorized_user.rs +++ b/src/gcloud_authorized_user.rs @@ -3,7 +3,6 @@ use std::process::Command; use std::sync::RwLock; use async_trait::async_trait; -use time::Duration; use which::which; use crate::authentication_manager::ServiceAccount; @@ -34,7 +33,7 @@ impl GCloudAuthorizedUser { fn token(gcloud: &Path) -> Result { Ok(Token::from_string( run(gcloud, &["auth", "print-access-token", "--quiet"])?, - Duration::seconds(DEFAULT_TOKEN_DURATION), + DEFAULT_TOKEN_DURATION, )) } } diff --git a/src/types.rs b/src/types.rs index 76b90b2..d6c059b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -16,7 +16,7 @@ use crate::Error; /// The default number of seconds that it takes for a Google Cloud auth token to expire. /// This appears to be the default from practical testing, but we have not found evidence /// that this will always be the default duration. -pub(crate) const DEFAULT_TOKEN_DURATION: i64 = 3600; +pub(crate) const DEFAULT_TOKEN_DURATION: Duration = Duration::seconds(3600); /// Represents an access token. All access tokens are Bearer tokens. /// @@ -196,8 +196,8 @@ mod tests { #[test] fn test_token_from_string() { let s = String::from("abc123"); - let token = Token::from_string(s, Duration::seconds(DEFAULT_TOKEN_DURATION)); - let expires = OffsetDateTime::now_utc() + Duration::seconds(DEFAULT_TOKEN_DURATION); + let token = Token::from_string(s, DEFAULT_TOKEN_DURATION); + let expires = OffsetDateTime::now_utc() + DEFAULT_TOKEN_DURATION; assert_eq!(token.as_str(), "abc123"); assert!(!token.has_expired()); From 4beae16d30e2a465dd73f08ca4883e0aeb66cf34 Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Tue, 7 Mar 2023 09:51:45 -0500 Subject: [PATCH 7/8] Move test for Token::from_string to the gcloud auth'd user because they are tightly coupled --- src/gcloud_authorized_user.rs | 27 ++++++++++++++++++++++++++- src/types.rs | 12 ------------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/gcloud_authorized_user.rs b/src/gcloud_authorized_user.rs index 563e3fd..84892ef 100644 --- a/src/gcloud_authorized_user.rs +++ b/src/gcloud_authorized_user.rs @@ -73,6 +73,8 @@ fn run(gcloud: &Path, cmd: &[&str]) -> Result { #[cfg(test)] mod tests { + use time::{Duration, OffsetDateTime}; + use super::*; #[tokio::test] @@ -80,6 +82,29 @@ mod tests { async fn gcloud() { let gcloud = GCloudAuthorizedUser::new().await.unwrap(); println!("{:?}", gcloud.project_id); - println!("{:?}", gcloud.get_token(&[""])); + if let Some(t) = gcloud.get_token(&[""]) { + let expires = OffsetDateTime::now_utc() + DEFAULT_TOKEN_DURATION; + println!("{:?}", t); + assert!(!t.has_expired()); + assert!(t.expires_at() < expires + Duration::seconds(1)); + assert!(t.expires_at() > expires - Duration::seconds(1)); + } else { + panic!("GCloud Authorized User failed to get a token"); + } + } + + /// `gcloud_authorized_user` is the only user type to get a token that isn't deserialized from + /// JSON, and that doesn't include an expiry time. As such, the default token expiry time + /// functionality is tested here. + #[test] + fn test_token_from_string() { + let s = String::from("abc123"); + let token = Token::from_string(s, DEFAULT_TOKEN_DURATION); + let expires = OffsetDateTime::now_utc() + DEFAULT_TOKEN_DURATION; + + assert_eq!(token.as_str(), "abc123"); + assert!(!token.has_expired()); + assert!(token.expires_at() < expires + Duration::seconds(1)); + assert!(token.expires_at() > expires - Duration::seconds(1)); } } diff --git a/src/types.rs b/src/types.rs index d6c059b..e4f2e0d 100644 --- a/src/types.rs +++ b/src/types.rs @@ -192,16 +192,4 @@ mod tests { assert!(expires_at < expires + Duration::seconds(1)); assert!(expires_at > expires - Duration::seconds(1)); } - - #[test] - fn test_token_from_string() { - let s = String::from("abc123"); - let token = Token::from_string(s, DEFAULT_TOKEN_DURATION); - let expires = OffsetDateTime::now_utc() + DEFAULT_TOKEN_DURATION; - - assert_eq!(token.as_str(), "abc123"); - assert!(!token.has_expired()); - assert!(token.expires_at() < expires + Duration::seconds(1)); - assert!(token.expires_at() > expires - Duration::seconds(1)); - } } From 589742cbe46f60eeade55a55e006551392b2089a Mon Sep 17 00:00:00 2001 From: Matthew Helmer Date: Mon, 13 Mar 2023 11:27:43 -0400 Subject: [PATCH 8/8] move default time to GAU and replace the no-time deserialization test --- src/gcloud_authorized_user.rs | 17 ++++++++++++++++- src/types.rs | 6 ------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/gcloud_authorized_user.rs b/src/gcloud_authorized_user.rs index 84892ef..33473db 100644 --- a/src/gcloud_authorized_user.rs +++ b/src/gcloud_authorized_user.rs @@ -3,14 +3,20 @@ use std::process::Command; use std::sync::RwLock; use async_trait::async_trait; +use time::Duration; use which::which; use crate::authentication_manager::ServiceAccount; use crate::error::Error; use crate::error::Error::{GCloudError, GCloudNotFound, GCloudParseError}; -use crate::types::{HyperClient, DEFAULT_TOKEN_DURATION}; +use crate::types::HyperClient; use crate::Token; +/// The default number of seconds that it takes for a Google Cloud auth token to expire. +/// This appears to be the default from practical testing, but we have not found evidence +/// that this will always be the default duration. +pub(crate) const DEFAULT_TOKEN_DURATION: Duration = Duration::seconds(3600); + #[derive(Debug)] pub(crate) struct GCloudAuthorizedUser { gcloud: PathBuf, @@ -107,4 +113,13 @@ mod tests { assert!(token.expires_at() < expires + Duration::seconds(1)); assert!(token.expires_at() > expires - Duration::seconds(1)); } + + #[test] + fn test_deserialise_no_time() { + let s = r#"{"access_token":"abc123"}"#; + let result = serde_json::from_str::(s) + .expect_err("Deserialization from JSON should fail when no expiry_time is included"); + + assert!(result.is_data()); + } } diff --git a/src/types.rs b/src/types.rs index e4f2e0d..04b11c0 100644 --- a/src/types.rs +++ b/src/types.rs @@ -12,12 +12,6 @@ use serde::{Deserialize, Serialize}; use time::{Duration, OffsetDateTime}; use crate::Error; - -/// The default number of seconds that it takes for a Google Cloud auth token to expire. -/// This appears to be the default from practical testing, but we have not found evidence -/// that this will always be the default duration. -pub(crate) const DEFAULT_TOKEN_DURATION: Duration = Duration::seconds(3600); - /// Represents an access token. All access tokens are Bearer tokens. /// /// Tokens should not be cached, the [`AuthenticationManager`] handles the correct caching