-
Notifications
You must be signed in to change notification settings - Fork 462
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
Labels
Comments
Jay,
client_secrets.json currently holds the full client_id.
Currently, the client_id is truncated when it's read from client_secrets.json and written to oauth2.txt.
Should the config option control that truncation or should the full value always be written to oauth2.txt and the config option controls truncation when the value from oauth2.txt is read on each command?
Ross
----
Ross Scroggs
***@***.***
… On Apr 22, 2024, at 1:19 PM, Jay Lee ***@***.***> wrote:
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:
Configure GAM to send the full client ID to Google every time by default.
Start storing the full client ID in oauth2.txt and client_secrets.json always.
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 <https://github.com/taers232c> fyi
—
Reply to this email directly, view it on GitHub <#1686>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCTYL377PBXRWNI7SCELVTY6VWETAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TOMZZGE3TEOA>.
You are receiving this because you were mentioned.
|
I'd say we should always store the full value. If anyone sees the need to
re-enable truncation that's easy enough to do on read.
…On Mon, Apr 22, 2024, 5:27 PM Ross Scroggs ***@***.***> wrote:
Jay,
client_secrets.json currently holds the full client_id.
Currently, the client_id is truncated when it's read from
client_secrets.json and written to oauth2.txt.
Should the config option control that truncation or should the full value
always be written to oauth2.txt and the config option controls truncation
when the value from oauth2.txt is read on each command?
Ross
----
Ross Scroggs
***@***.***
> On Apr 22, 2024, at 1:19 PM, Jay Lee ***@***.***> wrote:
>
>
> 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:
>
> Configure GAM to send the full client ID to Google every time by
default.
> Start storing the full client ID in oauth2.txt and client_secrets.json
always.
> 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 <https://github.com/taers232c> fyi
>
> —
> Reply to this email directly, view it on GitHub <
#1686>, or unsubscribe <
https://github.com/notifications/unsubscribe-auth/ACCTYL377PBXRWNI7SCELVTY6VWETAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TOMZZGE3TEOA>.
> You are receiving this because you were mentioned.
>
—
Reply to this email directly, view it on GitHub
<#1686 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDIZMEFHZYCPC47GE74GZDY6V6DLAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZQHE4DAOJWGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
With the existing code (a truncated client_id in oauth2.txt) the decoded_id_token shows the full client_id in "aud" and "azp".
Maybe we're already sending the full client_id.?
----
Ross Scroggs
***@***.***
… On Apr 22, 2024, at 1:19 PM, Jay Lee ***@***.***> wrote:
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:
Configure GAM to send the full client ID to Google every time by default.
Start storing the full client ID in oauth2.txt and client_secrets.json always.
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 <https://github.com/taers232c> fyi
—
Reply to this email directly, view it on GitHub <#1686>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCTYL377PBXRWNI7SCELVTY6VWETAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TOMZZGE3TEOA>.
You are receiving this because you were mentioned.
|
No, we definitely truncate it and use the truncated version in the auth URL
and on refresh.
…On Mon, Apr 22, 2024, 5:43 PM Ross Scroggs ***@***.***> wrote:
With the existing code (a truncated client_id in oauth2.txt) the
decoded_id_token shows the full client_id in "aud" and "azp".
Maybe we're already sending the full client_id.?
----
Ross Scroggs
***@***.***
> On Apr 22, 2024, at 1:19 PM, Jay Lee ***@***.***> wrote:
>
>
> 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:
>
> Configure GAM to send the full client ID to Google every time by
default.
> Start storing the full client ID in oauth2.txt and client_secrets.json
always.
> 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 <https://github.com/taers232c> fyi
>
> —
> Reply to this email directly, view it on GitHub <
#1686>, or unsubscribe <
https://github.com/notifications/unsubscribe-auth/ACCTYL377PBXRWNI7SCELVTY6VWETAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TOMZZGE3TEOA>.
> You are receiving this because you were mentioned.
>
—
Reply to this email directly, view it on GitHub
<#1686 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDIZME5B5B6WWPB3YAGWFDY6V767AVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAYDEMRYHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
If I try to truncate on read I get:
AttributeError: property 'client_id' of 'Credentials' object has no setter
----
Ross Scroggs
***@***.***
… On Apr 22, 2024, at 2:57 PM, Jay Lee ***@***.***> wrote:
No, we definitely truncate it and use the truncated version in the auth URL
and on refresh.
On Mon, Apr 22, 2024, 5:43 PM Ross Scroggs ***@***.***> wrote:
> With the existing code (a truncated client_id in oauth2.txt) the
> decoded_id_token shows the full client_id in "aud" and "azp".
> Maybe we're already sending the full client_id.?
> ----
> Ross Scroggs
> ***@***.***
>
>
>
> > On Apr 22, 2024, at 1:19 PM, Jay Lee ***@***.***> wrote:
> >
> >
> > 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:
> >
> > Configure GAM to send the full client ID to Google every time by
> default.
> > Start storing the full client ID in oauth2.txt and client_secrets.json
> always.
> > 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 <https://github.com/taers232c> fyi
> >
> > —
> > Reply to this email directly, view it on GitHub <
> #1686>, or unsubscribe <
> https://github.com/notifications/unsubscribe-auth/ACCTYL377PBXRWNI7SCELVTY6VWETAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TOMZZGE3TEOA>.
>
> > You are receiving this because you were mentioned.
> >
>
> —
> Reply to this email directly, view it on GitHub
> <#1686 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABDIZME5B5B6WWPB3YAGWFDY6V767AVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAYDEMRYHA>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub <#1686 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCTYLZOQUCU6QQVORQDKA3Y6WBVNAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAZDAMJUGE>.
You are receiving this because you were mentioned.
|
Jay,
I figured out what to change and uploaded a PR
Rebuild your oauth2.txt; the full client_id should be present
gam oauth create
Truncate
gam config truncate_client_id true info customer
No truncate, this is the default
gam config truncate_client_id false info customer
Ross
…----
Ross Scroggs
***@***.***
On Apr 22, 2024, at 2:57 PM, Jay Lee ***@***.***> wrote:
No, we definitely truncate it and use the truncated version in the auth URL
and on refresh.
On Mon, Apr 22, 2024, 5:43 PM Ross Scroggs ***@***.***> wrote:
> With the existing code (a truncated client_id in oauth2.txt) the
> decoded_id_token shows the full client_id in "aud" and "azp".
> Maybe we're already sending the full client_id.?
> ----
> Ross Scroggs
> ***@***.***
>
>
>
> > On Apr 22, 2024, at 1:19 PM, Jay Lee ***@***.***> wrote:
> >
> >
> > 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:
> >
> > Configure GAM to send the full client ID to Google every time by
> default.
> > Start storing the full client ID in oauth2.txt and client_secrets.json
> always.
> > 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 <https://github.com/taers232c> fyi
> >
> > —
> > Reply to this email directly, view it on GitHub <
> #1686>, or unsubscribe <
> https://github.com/notifications/unsubscribe-auth/ACCTYL377PBXRWNI7SCELVTY6VWETAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGI2TOMZZGE3TEOA>.
>
> > You are receiving this because you were mentioned.
> >
>
> —
> Reply to this email directly, view it on GitHub
> <#1686 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABDIZME5B5B6WWPB3YAGWFDY6V767AVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAYDEMRYHA>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub <#1686 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCTYLZOQUCU6QQVORQDKA3Y6WBVNAVCNFSM6AAAAABGTOKWNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZRGAZDAMJUGE>.
You are receiving this because you were mentioned.
|
FYI, a PR (pull request) goes through a review before being committed to the source, you did a direct commit here: for future reference, PRs and noting the issue # in the PR make it easier to follow the issue, PR and fix status. |
actually the real meat of the change was this commit: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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: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:
oauth2.txt
andclient_secrets.json
always.@taers232c fyi
The text was updated successfully, but these errors were encountered: