Skip to content

fix: report auth errors instead of silently proceeding unauthenticated #446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anshul-garg27 wants to merge 1 commit into
base: main
Choose a base branch
from
+47 −7
Open

fix: report auth errors instead of silently proceeding unauthenticated #446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-auth-error-consistency.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

fix: report auth errors instead of silently proceeding unauthenticated
7 changes: 6 additions & 1 deletion src/helpers/calendar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ TIPS:
let scopes_str: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scopes_str).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Calendar auth failed: {e}")))
}
};
Comment on lines 152 to 160
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This new authentication handling logic is a great improvement. However, this exact match block has been duplicated across multiple helper files (calendar.rs, chat.rs, docs.rs, drive.rs, script.rs, and sheets.rs), and a similar pattern already exists in gmail.rs.

To improve maintainability and avoid potential inconsistencies in the future, consider abstracting this logic into a single, shared helper function. This would centralize the authentication flow, making it easier to update if needed.

For example, you could create a function like this in a shared module (e.g., a new src/helpers/auth_utils.rs or within src/auth.rs):

pub async fn get_auth_token_or_fail(
    scopes: &[&str],
    matches: &clap::ArgMatches,
    service_name: &str,
) -> Result<(Option<String>, crate::executor::AuthMethod), crate::error::GwsError> {
    match crate::auth::get_token(scopes).await {
        Ok(t) => Ok((Some(t), crate::executor::AuthMethod::OAuth)),
        Err(_) if matches.get_flag("dry-run") => Ok((None, crate::executor::AuthMethod::None)),
        Err(e) => Err(crate::error::GwsError::Auth(format!(
            "{} auth failed: {}",
            service_name, e
        ))),
    }
}

Then, each call site could be simplified to a single line:

let (token, auth_method) = crate::helpers::auth_utils::get_auth_token_or_fail(&scopes_str, matches, "Calendar").await?;

Copy link
Contributor Author

@anshul-garg27 anshul-garg27 Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed the pattern is duplicated. extracting a shared helper is a good follow-up but would touch 7+ files — keeping this PR scoped to fixing the silent failure. noted for a future refactor


let events_res = doc.resources.get("events").ok_or_else(|| {
Expand Down
7 changes: 6 additions & 1 deletion src/helpers/chat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ TIPS:
let scope_strs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scope_strs).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Chat auth failed: {e}")))
}
};

// Method: spaces.messages.create
Expand Down
7 changes: 6 additions & 1 deletion src/helpers/docs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ TIPS:
let scope_strs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scope_strs).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Docs auth failed: {e}")))
}
};

// Method: documents.batchUpdate
Expand Down
7 changes: 6 additions & 1 deletion src/helpers/drive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,12 @@ TIPS:
let scopes: Vec<&str> = create_method.scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scopes).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Drive auth failed: {e}")))
}
};

executor::execute_method(
Expand Down
7 changes: 6 additions & 1 deletion src/helpers/script.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,12 @@ TIPS:
let scopes: Vec<&str> = update_method.scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scopes).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Script auth failed: {e}")))
}
};

let params = json!({
Expand Down
14 changes: 12 additions & 2 deletions src/helpers/sheets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ TIPS:
let scope_strs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scope_strs).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Sheets auth failed: {e}")))
}
};

let spreadsheets_res = doc.resources.get("spreadsheets").ok_or_else(|| {
Expand Down Expand Up @@ -166,7 +171,12 @@ TIPS:
let scope_strs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
let (token, auth_method) = match auth::get_token(&scope_strs).await {
Ok(t) => (Some(t), executor::AuthMethod::OAuth),
Err(_) => (None, executor::AuthMethod::None),
Err(_) if matches.get_flag("dry-run") => {
(None, executor::AuthMethod::None)
}
Err(e) => {
return Err(GwsError::Auth(format!("Sheets auth failed: {e}")))
}
};

executor::execute_method(
Expand Down
Loading