From 14d3fd87be1dce2c97f1f365d303b91d8f2cfbf7 Mon Sep 17 00:00:00 2001 From: Csongor Vogel <15221068+gerfalcon@users.noreply.github.com> Date: Tue, 17 Mar 2026 20:41:44 +0400 Subject: [PATCH 1/4] fix(auth): enforce readonly scopes by revoking stale tokens and adding client-side guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user previously authenticated with full scopes, `gws auth login --readonly` did not actually enforce read-only access. The underlying refresh token retained the original consent grant, and Google's token endpoint ignores the scope parameter on refresh requests — so the cached token silently carried write access. This commit fixes the issue with a layered approach: - Persist the configured scope set to scopes.json on login so scope changes can be detected across sessions. - When scopes change, revoke the old refresh token via Google's revocation endpoint and clear local credential/cache files before starting the new OAuth flow. - Add a client-side scope guard that rejects write-scope API requests when the session is readonly. - Show scope_mode and configured_scopes in `gws auth status`. - Clean up scopes.json on logout. Fixes #168 --- .changeset/fix-readonly-scope-enforcement.md | 7 + src/auth_commands.rs | 197 ++++++++++++++++++- src/main.rs | 5 + 3 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-readonly-scope-enforcement.md diff --git a/.changeset/fix-readonly-scope-enforcement.md b/.changeset/fix-readonly-scope-enforcement.md new file mode 100644 index 00000000..e35b967a --- /dev/null +++ b/.changeset/fix-readonly-scope-enforcement.md @@ -0,0 +1,7 @@ +--- +"@googleworkspace/cli": patch +--- + +fix(auth): enforce readonly scopes by revoking stale tokens on scope change and adding client-side guard + +Fixes #168 diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 47d2d4e2..eb76e6b3 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -128,6 +128,56 @@ fn token_cache_path() -> PathBuf { config_dir().join("token_cache.json") } +fn scopes_path() -> PathBuf { + config_dir().join("scopes.json") +} + +/// Save the configured scope set so scope changes can be detected across sessions. +fn save_scopes(scopes: &[String]) { + if let Ok(json) = serde_json::to_string_pretty(scopes) { + let _ = crate::fs_util::atomic_write(&scopes_path(), json.as_bytes()); + } +} + +/// Load the previously saved scope set, if any. +pub fn load_saved_scopes() -> Option> { + let data = std::fs::read_to_string(scopes_path()).ok()?; + serde_json::from_str(&data).ok() +} + +/// Returns true if the saved scopes are all read-only. +pub fn is_readonly_session() -> bool { + load_saved_scopes() + .map(|scopes| { + scopes.iter().all(|s| { + s.ends_with(".readonly") + || s == "openid" + || s.starts_with("https://www.googleapis.com/auth/userinfo.") + || s == "email" + }) + }) + .unwrap_or(false) +} + +/// Check if a requested scope is compatible with the current session. +/// +/// In a readonly session, write-scope requests are rejected with a clear error. +pub fn check_scope_allowed(scope: &str) -> Result<(), GwsError> { + if is_readonly_session() + && !scope.ends_with(".readonly") + && scope != "openid" + && !scope.starts_with("https://www.googleapis.com/auth/userinfo.") + && scope != "email" + { + return Err(GwsError::Auth(format!( + "This operation requires scope '{}' (write access), but the current session \ + uses read-only scopes. Run `gws auth login` (without --readonly) to upgrade.", + scope + ))); + } + Ok(()) +} + /// Handle `gws auth `. pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { const USAGE: &str = concat!( @@ -267,6 +317,34 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { ..Default::default() }; + // If scopes changed from the previous login, revoke the old refresh token + // so Google removes the prior consent grant. Without revocation, Google's + // consent screen shows previously-granted scopes pre-checked and the user + // may unknowingly re-grant broad access. + if let Some(prev_scopes) = load_saved_scopes() { + let prev_set: HashSet<&str> = prev_scopes.iter().map(|s| s.as_str()).collect(); + let new_set: HashSet<&str> = scopes.iter().map(|s| s.as_str()).collect(); + if prev_set != new_set { + // Best-effort revocation: load the old refresh token and revoke it. + if let Ok(creds_str) = credential_store::load_encrypted() { + if let Ok(creds) = serde_json::from_str::(&creds_str) { + if let Some(rt) = creds.get("refresh_token").and_then(|v| v.as_str()) { + let client = reqwest::Client::new(); + let _ = client + .post("https://oauth2.googleapis.com/revoke") + .form(&[("token", rt)]) + .send() + .await; + } + } + } + // Clear local credential and cache files regardless of revocation result. + let _ = std::fs::remove_file(credential_store::encrypted_credentials_path()); + let _ = std::fs::remove_file(token_cache_path()); + eprintln!("Scopes changed — revoked previous credentials."); + } + } + // Ensure openid + email scopes are always present so we can identify the user // via the userinfo endpoint after login. let identity_scopes = ["openid", "https://www.googleapis.com/auth/userinfo.email"]; @@ -343,6 +421,10 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { let enc_path = credential_store::save_encrypted(&creds_str) .map_err(|e| GwsError::Auth(format!("Failed to encrypt credentials: {e}")))?; + // Persist the configured scope set for scope-change detection and + // client-side guard enforcement. + save_scopes(&scopes); + // Clean up temp file let _ = std::fs::remove_file(&temp_path); @@ -1141,6 +1223,16 @@ async fn handle_status() -> Result<(), GwsError> { } } // end !cfg!(test) + // Show configured scope mode from scopes.json (independent of network) + if let Some(saved) = load_saved_scopes() { + output["configured_scopes"] = json!(saved); + output["scope_mode"] = json!(if is_readonly_session() { + "readonly" + } else { + "default" + }); + } + println!( "{}", serde_json::to_string_pretty(&output).unwrap_or_default() @@ -1153,10 +1245,11 @@ fn handle_logout() -> Result<(), GwsError> { let enc_path = credential_store::encrypted_credentials_path(); let token_cache = token_cache_path(); let sa_token_cache = config_dir().join("sa_token_cache.json"); + let scopes_file = scopes_path(); let mut removed = Vec::new(); - for path in [&enc_path, &plain_path, &token_cache, &sa_token_cache] { + for path in [&enc_path, &plain_path, &token_cache, &sa_token_cache, &scopes_file] { if path.exists() { std::fs::remove_file(path).map_err(|e| { GwsError::Validation(format!("Failed to remove {}: {e}", path.display())) @@ -1947,4 +2040,106 @@ mod tests { // Exactly 9 chars — first 4 + last 4 with "..." in between assert_eq!(mask_secret("123456789"), "1234...6789"); } + + // --- Scope persistence and guard tests --- + + #[test] + fn test_save_and_load_scopes_roundtrip() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("scopes.json"); + let scopes = vec![ + "https://www.googleapis.com/auth/gmail.readonly".to_string(), + "openid".to_string(), + ]; + let json = serde_json::to_string_pretty(&scopes).unwrap(); + crate::fs_util::atomic_write(&path, json.as_bytes()).unwrap(); + let loaded: Vec = + serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert_eq!(loaded, scopes); + } + + #[test] + fn test_is_readonly_all_readonly_scopes() { + let scopes = vec![ + "https://www.googleapis.com/auth/drive.readonly", + "https://www.googleapis.com/auth/gmail.readonly", + "openid", + "https://www.googleapis.com/auth/userinfo.email", + "email", + ]; + assert!(scopes.iter().all(|s| { + s.ends_with(".readonly") + || *s == "openid" + || s.starts_with("https://www.googleapis.com/auth/userinfo.") + || *s == "email" + })); + } + + #[test] + fn test_is_readonly_mixed_scopes() { + let scopes = vec![ + "https://www.googleapis.com/auth/drive", + "https://www.googleapis.com/auth/gmail.readonly", + "openid", + ]; + let is_readonly = scopes.iter().all(|s| { + s.ends_with(".readonly") + || *s == "openid" + || s.starts_with("https://www.googleapis.com/auth/userinfo.") + || *s == "email" + }); + assert!(!is_readonly); + } + + #[test] + fn test_check_scope_allowed_blocks_write_in_readonly() { + // Simulate the readonly check logic directly + let saved_scopes = vec![ + "https://www.googleapis.com/auth/gmail.readonly".to_string(), + "openid".to_string(), + ]; + let is_readonly = saved_scopes.iter().all(|s| { + s.ends_with(".readonly") + || s == "openid" + || s.starts_with("https://www.googleapis.com/auth/userinfo.") + || s == "email" + }); + assert!(is_readonly); + + // A write scope should be rejected + let scope = "https://www.googleapis.com/auth/gmail.modify"; + let blocked = is_readonly + && !scope.ends_with(".readonly") + && scope != "openid" + && !scope.starts_with("https://www.googleapis.com/auth/userinfo.") + && scope != "email"; + assert!(blocked); + } + + #[test] + fn test_check_scope_allowed_permits_readonly_in_readonly() { + let scope = "https://www.googleapis.com/auth/gmail.readonly"; + let blocked = !scope.ends_with(".readonly") + && scope != "openid" + && !scope.starts_with("https://www.googleapis.com/auth/userinfo.") + && scope != "email"; + assert!(!blocked); + } + + #[test] + fn test_check_scope_allowed_permits_all_in_default() { + // When saved scopes include write scopes, is_readonly is false + let saved_scopes = vec![ + "https://www.googleapis.com/auth/drive".to_string(), + "https://www.googleapis.com/auth/gmail.modify".to_string(), + ]; + let is_readonly = saved_scopes.iter().all(|s| { + s.ends_with(".readonly") + || s == "openid" + || s.starts_with("https://www.googleapis.com/auth/userinfo.") + || s == "email" + }); + assert!(!is_readonly); + // In non-readonly mode, no scope is blocked + } } diff --git a/src/main.rs b/src/main.rs index a448fec1..2069810c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -233,6 +233,11 @@ async fn run() -> Result<(), GwsError> { // to avoid restrictive scopes like gmail.metadata that block query parameters. let scopes: Vec<&str> = select_scope(&method.scopes).into_iter().collect(); + // Enforce readonly session: reject write scopes if user logged in with --readonly + if let Some(scope) = scopes.first() { + auth_commands::check_scope_allowed(scope)?; + } + // Authenticate: try OAuth, fail with error if credentials exist but are broken let (token, auth_method) = match auth::get_token(&scopes).await { Ok(t) => (Some(t), executor::AuthMethod::OAuth), From 80b47e761893f71ba483854b3329e178983981ad Mon Sep 17 00:00:00 2001 From: Csongor Vogel <15221068+gerfalcon@users.noreply.github.com> Date: Tue, 17 Mar 2026 21:41:33 +0400 Subject: [PATCH 2/4] fix(auth): propagate save_scopes errors instead of silently ignoring Address review feedback: save_scopes now returns Result<(), GwsError> and the call site uses ? to propagate failures. This ensures scopes.json is always consistent with the actual login state. --- src/auth_commands.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 1982ea86..92d13b1e 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -133,10 +133,12 @@ fn scopes_path() -> PathBuf { } /// Save the configured scope set so scope changes can be detected across sessions. -fn save_scopes(scopes: &[String]) { - if let Ok(json) = serde_json::to_string_pretty(scopes) { - let _ = crate::fs_util::atomic_write(&scopes_path(), json.as_bytes()); - } +fn save_scopes(scopes: &[String]) -> Result<(), GwsError> { + let json = serde_json::to_string_pretty(scopes) + .map_err(|e| GwsError::Validation(format!("Failed to serialize scopes: {e}")))?; + crate::fs_util::atomic_write(&scopes_path(), json.as_bytes()) + .map_err(|e| GwsError::Validation(format!("Failed to save scopes file: {e}")))?; + Ok(()) } /// Load the previously saved scope set, if any. @@ -431,7 +433,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { // Persist the configured scope set for scope-change detection and // client-side guard enforcement. - save_scopes(&scopes); + save_scopes(&scopes)?; // Clean up temp file let _ = std::fs::remove_file(&temp_path); From ec96752f929e2b6beb9b2df02de47faac3236cb9 Mon Sep 17 00:00:00 2001 From: Csongor Vogel <15221068+gerfalcon@users.noreply.github.com> Date: Wed, 18 Mar 2026 00:29:31 +0400 Subject: [PATCH 3/4] refactor(auth): extract is_non_write_scope helper to eliminate duplication DRY up the scope classification logic that was duplicated between is_readonly_session and check_scope_allowed. Rewrite tests to call the extracted helper directly instead of re-implementing the logic. --- src/auth_commands.rs | 117 ++++++++----------------------------------- 1 file changed, 22 insertions(+), 95 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 92d13b1e..a67daa88 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -147,17 +147,18 @@ pub fn load_saved_scopes() -> Option> { serde_json::from_str(&data).ok() } +/// Returns true if a scope does not grant write access (identity or .readonly scopes). +fn is_non_write_scope(scope: &str) -> bool { + scope.ends_with(".readonly") + || scope == "openid" + || scope.starts_with("https://www.googleapis.com/auth/userinfo.") + || scope == "email" +} + /// Returns true if the saved scopes are all read-only. pub fn is_readonly_session() -> bool { load_saved_scopes() - .map(|scopes| { - scopes.iter().all(|s| { - s.ends_with(".readonly") - || s == "openid" - || s.starts_with("https://www.googleapis.com/auth/userinfo.") - || s == "email" - }) - }) + .map(|scopes| scopes.iter().all(|s| is_non_write_scope(s))) .unwrap_or(false) } @@ -165,12 +166,7 @@ pub fn is_readonly_session() -> bool { /// /// In a readonly session, write-scope requests are rejected with a clear error. pub fn check_scope_allowed(scope: &str) -> Result<(), GwsError> { - if is_readonly_session() - && !scope.ends_with(".readonly") - && scope != "openid" - && !scope.starts_with("https://www.googleapis.com/auth/userinfo.") - && scope != "email" - { + if is_readonly_session() && !is_non_write_scope(scope) { return Err(GwsError::Auth(format!( "This operation requires scope '{}' (write access), but the current session \ uses read-only scopes. Run `gws auth login` (without --readonly) to upgrade.", @@ -2322,87 +2318,18 @@ mod tests { } #[test] - fn test_is_readonly_all_readonly_scopes() { - let scopes = vec![ - "https://www.googleapis.com/auth/drive.readonly", - "https://www.googleapis.com/auth/gmail.readonly", - "openid", - "https://www.googleapis.com/auth/userinfo.email", - "email", - ]; - assert!(scopes.iter().all(|s| { - s.ends_with(".readonly") - || *s == "openid" - || s.starts_with("https://www.googleapis.com/auth/userinfo.") - || *s == "email" - })); - } - - #[test] - fn test_is_readonly_mixed_scopes() { - let scopes = vec![ - "https://www.googleapis.com/auth/drive", - "https://www.googleapis.com/auth/gmail.readonly", - "openid", - ]; - let is_readonly = scopes.iter().all(|s| { - s.ends_with(".readonly") - || *s == "openid" - || s.starts_with("https://www.googleapis.com/auth/userinfo.") - || *s == "email" - }); - assert!(!is_readonly); - } - - #[test] - fn test_check_scope_allowed_blocks_write_in_readonly() { - // Simulate the readonly check logic directly - let saved_scopes = vec![ - "https://www.googleapis.com/auth/gmail.readonly".to_string(), - "openid".to_string(), - ]; - let is_readonly = saved_scopes.iter().all(|s| { - s.ends_with(".readonly") - || s == "openid" - || s.starts_with("https://www.googleapis.com/auth/userinfo.") - || s == "email" - }); - assert!(is_readonly); - - // A write scope should be rejected - let scope = "https://www.googleapis.com/auth/gmail.modify"; - let blocked = is_readonly - && !scope.ends_with(".readonly") - && scope != "openid" - && !scope.starts_with("https://www.googleapis.com/auth/userinfo.") - && scope != "email"; - assert!(blocked); - } + fn test_is_non_write_scope() { + // Readonly and identity scopes are non-write + assert!(is_non_write_scope("https://www.googleapis.com/auth/drive.readonly")); + assert!(is_non_write_scope("https://www.googleapis.com/auth/gmail.readonly")); + assert!(is_non_write_scope("openid")); + assert!(is_non_write_scope("email")); + assert!(is_non_write_scope("https://www.googleapis.com/auth/userinfo.email")); - #[test] - fn test_check_scope_allowed_permits_readonly_in_readonly() { - let scope = "https://www.googleapis.com/auth/gmail.readonly"; - let blocked = !scope.ends_with(".readonly") - && scope != "openid" - && !scope.starts_with("https://www.googleapis.com/auth/userinfo.") - && scope != "email"; - assert!(!blocked); - } - - #[test] - fn test_check_scope_allowed_permits_all_in_default() { - // When saved scopes include write scopes, is_readonly is false - let saved_scopes = vec![ - "https://www.googleapis.com/auth/drive".to_string(), - "https://www.googleapis.com/auth/gmail.modify".to_string(), - ]; - let is_readonly = saved_scopes.iter().all(|s| { - s.ends_with(".readonly") - || s == "openid" - || s.starts_with("https://www.googleapis.com/auth/userinfo.") - || s == "email" - }); - assert!(!is_readonly); - // In non-readonly mode, no scope is blocked + // Write scopes are not non-write + assert!(!is_non_write_scope("https://www.googleapis.com/auth/drive")); + assert!(!is_non_write_scope("https://www.googleapis.com/auth/gmail.modify")); + assert!(!is_non_write_scope("https://www.googleapis.com/auth/calendar")); + assert!(!is_non_write_scope("https://www.googleapis.com/auth/pubsub")); } } From 14a2dfebbc4c0daee5dc108db72a31bda0382a02 Mon Sep 17 00:00:00 2001 From: Csongor Vogel <15221068+gerfalcon@users.noreply.github.com> Date: Wed, 18 Mar 2026 11:00:35 +0400 Subject: [PATCH 4/4] fix(auth): handle revocation and file removal errors properly Warn the user when token revocation fails (network error or non-200 response) so they know the old token may still be valid server-side. Return errors when credential/cache file removal fails (ignoring NotFound) instead of silently continuing with stale files. --- src/auth_commands.rs | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index a67daa88..ea12de13 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -331,22 +331,51 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { let prev_set: HashSet<&str> = prev_scopes.iter().map(|s| s.as_str()).collect(); let new_set: HashSet<&str> = scopes.iter().map(|s| s.as_str()).collect(); if prev_set != new_set { - // Best-effort revocation: load the old refresh token and revoke it. + // Revoke the old refresh token so Google removes the prior consent grant. if let Ok(creds_str) = credential_store::load_encrypted() { if let Ok(creds) = serde_json::from_str::(&creds_str) { if let Some(rt) = creds.get("refresh_token").and_then(|v| v.as_str()) { let client = reqwest::Client::new(); - let _ = client + match client .post("https://oauth2.googleapis.com/revoke") .form(&[("token", rt)]) .send() - .await; + .await + { + Ok(resp) if resp.status().is_success() => {} + Ok(resp) => { + eprintln!( + "Warning: token revocation returned HTTP {}. \ + The old token may still be valid on Google's side.", + resp.status() + ); + } + Err(e) => { + eprintln!( + "Warning: could not revoke old token ({e}). \ + The old token may still be valid on Google's side." + ); + } + } } } } - // Clear local credential and cache files regardless of revocation result. - let _ = std::fs::remove_file(credential_store::encrypted_credentials_path()); - let _ = std::fs::remove_file(token_cache_path()); + // Clear local credential and cache files to force a fresh login. + let enc_path = credential_store::encrypted_credentials_path(); + if let Err(e) = std::fs::remove_file(&enc_path) { + if e.kind() != std::io::ErrorKind::NotFound { + return Err(GwsError::Auth(format!( + "Failed to remove old credentials file: {e}. Please remove it manually." + ))); + } + } + if let Err(e) = std::fs::remove_file(token_cache_path()) { + if e.kind() != std::io::ErrorKind::NotFound { + return Err(GwsError::Auth(format!( + "Failed to remove old token cache: {e}. Please remove it manually." + ))); + } + } eprintln!("Scopes changed — revoked previous credentials."); } }