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

oauth2 not possible with google-oauth-java-client #164

Closed
n0xena opened this issue Jul 13, 2020 · 11 comments
Closed

oauth2 not possible with google-oauth-java-client #164

n0xena opened this issue Jul 13, 2020 · 11 comments

Comments

@n0xena
Copy link

n0xena commented Jul 13, 2020

please see: googleapis/google-oauth-java-client#487
possible cause of issue: token endpoint return scopes as array with single space separated string

"scope": ["scopes"]

but google-oauth-lib actually expects just space separated string without the array brackets around it

"scope": "scopes"

don't know if root cause of issue is caused by bad json implementation in googles lib or on twitch reply

@n0xena
Copy link
Author

n0xena commented Jul 14, 2020

please see: https://tools.ietf.org/html/rfc6749#section-3.3
according to the rfc the value of the "scope" parameter has to be only a string separated list - hence twitch "violates" the rfc by wrapping it in an array - please fix this so any rfc conforming oauth lib (like google) could work with your endpoint

@n0xena
Copy link
Author

n0xena commented Jul 14, 2020

here's a side by side comparison

https://accounts.google.com/o/oauth2/auth?response_type=code&redirect_uri=urn%3Aietf%3Awg%3Aoauth%3A2.0%3Aoob&client_id=redacted&scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutube+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutube.channel-memberships.creator+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutube.force-ssl+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutube.readonly+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutube.upload+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutubepartner+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fyoutubepartner-channel-audit

{
	"access_token": "redacted",
	"expires_in": 3599,
	"refresh_token": "redacted",
	"scope": "https://www.googleapis.com/auth/youtube.upload https://www.googleapis.com/auth/youtube.force-ssl https://www.googleapis.com/auth/youtubepartner-channel-audit https://www.googleapis.com/auth/youtube.readonly https://www.googleapis.com/auth/youtubepartner https://www.googleapis.com/auth/youtube.channel-memberships.creator https://www.googleapis.com/auth/youtube",
	"token_type": "Bearer"
}
https://id.twitch.tv/oauth2/authorize?response_type=code&redirect_uri=redacted&client_id=redacted&scope=analytics%3Aread%3Aextensions+analytics%3Aread%3Agames+bits%3Aread+channel%3Aedit%3Acommercial+channel%3Aread%3Ahype_train+channel%3Aread%3Asubscriptions+clips%3Aedit+user%3Aedit+user%3Aedit%3Abroadcast+user%3Aedit%3Afollows+user%3Aread%3Abroadcast+user%3Aread%3Aemail+channel%3Aread%3Astream_key+channel_check_subscription+channel_commercial+channel_editor+channel_feed_edit+channel_feed_read+channel_read+channel_stream+channel_subscriptions+collections_edit+communities_edit+communities_moderate+openid+user_blocks_edit+user_blocks_read+user_follows_edit+user_read+user_subscriptions+viewing_activity_read+channel%3Amoderate+chat%3Aedit+chat%3Aread+whispers%3Aread+whispers%3Aedit

{
	"access_token":"redacted",
	"expires_in":15634,
	"id_token":"redacted",
	"refresh_token":"redactred",
	"scope":["analytics:read:extensions","analytics:read:games","bits:read","channel:edit:commercial","channel:moderate","channel:read:hype_train","channel:read:stream_key","channel:read:subscriptions","channel_check_subscription","channel_commercial","channel_editor","channel_feed_edit","channel_feed_read","channel_read","channel_stream","channel_subscriptions","chat:edit","chat:read","clips:edit","collections_edit","communities_edit","communities_moderate","openid","user:edit","user:edit:broadcast","user:edit:follows","user:read:broadcast","user:read:email","user_blocks_edit","user_blocks_read","user_follows_edit","user_read","user_subscriptions","viewing_activity_read","whispers:edit","whispers:read"],
	"token_type":"bearer"
}

This clearly shows how twitch token endpoint fails to follow RFC

The value of the scope parameter is expressed as a list of space-delimited, case-sensitive strings.  The strings are defined by the authorization server.  If the value contains multiple space-delimited strings, their order does not matter, and each string adds an additional access range to the requested scope.

     scope       = scope-token *( SP scope-token )
     scope-token = 1*( %x21 / %x23-5B / %x5D-7E )

That's the reason why googles oauth lib fails to parse the scope correctly - cause twitch serves it wrong. This has to be fixed!
More severly: Every other client used twitch oauth until now and until this gets finally fixed had to come up with its own parser instead of use working libs. This will solve quite a lot of issues with custom parsers as devs now finally can rely on tested and proper implemented libs. Why was it done this way in the first place? Just to troll google? Violation of RFC-defined protocols wasn't your best idea ... just another reason why twitch suxx when compared to youtube ... along with low quality (as by support: cause we focus on user-base - which most of them use mobile devices and limited bandwidth) and no transcoding for non-partners (which youtube does offer).

Get this straight - and fix a lot of crap you caused by it ...

@jbulava
Copy link
Member

jbulava commented Aug 20, 2020

Thanks for your feedback regarding the scope format. While we strive to follow RFCs as closely as possible, occasionally deviations do occur. Changing our scope value from an array to a space-separated string would be a significant breaking change for existing applications. Making this change to be inline with the RFC does not outweigh the disruption it will cause, so we will not be updating the format in the current implementation. This may be something to consider in a future implementation, but as there is no action at this time, this issue will be closed.

@jbulava jbulava closed this as completed Aug 20, 2020
@deefdragon
Copy link

deefdragon commented Aug 26, 2020

@jbulava given this breaks basically every oauth library, and as you said, a full switch is not reasonable for compatibility, would it be possible to bodge it?

Most, if not all libraries have a way to set the scopes requested, and I was wondering if it would be possible to check for a specific new scope "space_seperated_scopes" for example. In the case where the scope exists, the scopes are returned as a space separated string following the RFC. Anyone who doesn't want to switch, doesn't have to, and anyone who wants to use a library still can.

Theoretically this could also allow for an easier transition period to using the rfc implementation at some point in the future as well.

@jbulava
Copy link
Member

jbulava commented Aug 26, 2020

Thanks for the follow up. I'll bring this idea to the team that oversees authentication.

@deefdragon
Copy link

@jbulava any update on this? I wrote an in-between service that handles things, but I would rather not need to host an entire extra program if not needed.

@ghost
Copy link

ghost commented Jan 31, 2021

Is it possible to get an update on this?

@0x0BEE
Copy link

0x0BEE commented Dec 12, 2021

Almost a year later, can we get an update on this? This is a pretty critical issue for most people who use an IdP broker. Obviously a breaking change would be bad, but you already use a non standard &claims= parameters, perhaps we could add something like &space_separated_scopes?

@FFY00
Copy link

FFY00 commented Feb 20, 2023

Making this change to be inline with the RFC does not outweigh the disruption it will cause, so we will not be updating the format in the current implementation. This may be something to consider in a future implementation, but as there is no action at this time, this issue will be closed.

There are multiple ways for this to be implemented in a backwards compatible way, as described already.

Having a non-standard OAuth2 makes almost all language-specifc libraries break. This is a major pain point, as our options are to either fork, or play whack-a-mole trying to implement workarounds for all OAuth2 libraries... if this kind of workarounds are in line with the libraries' philosophy in the first place.

If there is an internal reason why this can't be implemented, I'd understand, but limiting the fix to a disruptive implementation just means that Twitch does not deem it financially worth it to spend the developer time to support a nondisruptive fix. I'd like to charitable, but this was reported 2.5 years ago, and several people suggested suitable fixes, and like I said, this is a major pain point in the Twitch API.

@jbulava any updates?

@ramosbugs
Copy link

Having a non-standard OAuth2 makes almost all language-specifc libraries break.

As the maintainer of two such Rust libraries (https://github.com/ramosbugs/oauth2-rs and https://github.com/ramosbugs/openidconnect-rs), I think it would be nice for Twitch and other companies who fail to follow the RFCs to at least chip in and sponsor the libraries that have to regularly respond to these issues. I've had at least 4 issues/PRs filed by Twitch users alone. 🙄

The cavalier attitude demonstrated by "occasionally deviations do occur" while completely externalizing the impacts of these engineering mistakes to third-party maintainers and devs trying to build integrations is offensive, honestly.

@n0xena
Copy link
Author

n0xena commented Jun 11, 2024

Just found the reply while cleaning my mail server - 4 years and NOTHING has happend? This is just ridiculous.
Yes, of course a hard transition would break existing stuff - but only because you did it wrong in the first place and anyone had to follow your crap cause rfc-compliant libs failed.
There were even some options posted for a smooth transition period so every active dev could have enough time to switch to a rfc-complaint lib to fix this whole mess.

@jbulava Are there ANY updates since? How about pitching this issue again? Oh, btw - as this issue is still present - how about re-opening it? Closing github issues means the problem was resolved - which this here clearly is not!
Just because you (Twitch) think "Oh, since amazon bought us we're too big to fix this now" doesn't mean it's the right thing to do just to ignore it - it would just require a longer transition period.

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

No branches or pull requests

6 participants