Skip to content
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.

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

GAM should always send the full OAuth Client ID #1686

Open
jay0lee opened this issue Apr 22, 2024 · 8 comments
Open

GAM should always send the full OAuth Client ID #1686

jay0lee opened this issue Apr 22, 2024 · 8 comments
Assignees
Labels

Comments

@jay0lee
Copy link
Member

jay0lee commented Apr 22, 2024

Historically, GAM has truncated the OAuth 2.0 client id by removing .apps.googleusercontent.com when storing it to client_secrets.json and oauth2.txt because:

  • OAuth authorization URLs end up being very long due to the number of scopes GAM uses
  • it still works.

However, the recent GAM outage occurred because GAM was truncating the client ID in this way. That's why authoriztion started throwing 403 and 500 errors for GAM but other OAuth apps did not see issues.

At this point, I don't think we have OAuth authorization issues with long URLs to the point that .apps.googleusercontent.com is significant and we should avoid doing non-standard / undocumented things like client ID truncation wherever possible to avoid breaking things like this in the future.

Lets:

  1. Configure GAM to send the full client ID to Google every time by default.
  2. Start storing the full client ID in oauth2.txt and client_secrets.json always.
  3. Add a config option to use truncated client IDs. This is a "just in case" we need to revert to current behavior without admins needing to upgrade.

@taers232c fyi

@jay0lee jay0lee added the bug label Apr 22, 2024
@jay0lee jay0lee self-assigned this Apr 22, 2024
@taers232c
Copy link
Contributor

taers232c commented Apr 22, 2024 via email

@jay0lee
Copy link
Member Author

jay0lee commented Apr 22, 2024 via email

@taers232c
Copy link
Contributor

taers232c commented Apr 22, 2024 via email

@jay0lee
Copy link
Member Author

jay0lee commented Apr 22, 2024 via email

@taers232c
Copy link
Contributor

taers232c commented Apr 22, 2024 via email

@taers232c
Copy link
Contributor

taers232c commented Apr 23, 2024 via email

@jay0lee
Copy link
Member Author

jay0lee commented Apr 23, 2024

FYI, a PR (pull request) goes through a review before being committed to the source, you did a direct commit here:

b4b9bd2

for future reference, PRs and noting the issue # in the PR make it easier to follow the issue, PR and fix status.

@jay0lee
Copy link
Member Author

jay0lee commented Apr 23, 2024

actually the real meat of the change was this commit:

b384bdb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants