Skip to content

Add the possibility of having a sequence of strings as scopes#146

Closed
naumazeredo wants to merge 1 commit intoramosbugs:mainfrom
naumazeredo:main
Closed

Add the possibility of having a sequence of strings as scopes#146
naumazeredo wants to merge 1 commit intoramosbugs:mainfrom
naumazeredo:main

Conversation

@naumazeredo
Copy link
Copy Markdown

Some OAuth2 APIs have a sequence of strings as scope in the client credentials response (e.g.: Twitch), which is not supported in the current oauth2-rs state.

This PR just adds the deserialization to this case.
The easiest approach was to create an abstraction enum to encapsulate both options during deserialization (and avoid touching Option parsing and Visitor design altogether). The change only affects the deserialization helper.

Not part of the PR:
To be honest, I had a tough time debugging this. Passing a slice, instead of a string, to serde deserialization makes it very annoying to debug since all parsing errors prints the characters as u8 instead of the correct JSON string and makes the debugging way more cumbersome. In case I keep using this crate I'll probably submit some other PRs to both add examples for move parts (had to reverse engineer the client credentials part to both know what to use and where this bug was) and better error messages.

Thanks for the work on this crate. Saved me quite some time.

@ramosbugs
Copy link
Copy Markdown
Owner

Hey, thanks for the PR!

Based on my reading of RFC 6749, this behavior doesn't quite follow the spec, unless there's a newer standard that allows a JSON array of strings instead of a space-delimited list of strings. I would prefer to keep StandardTokenResponse (and similar structs) strictly standards-adherent, but this crate exports the TokenResponse trait (and similar traits) specifically to allow users to provide their own implementations that deviate from the standards. I think that would be the best solution for this case.

Not part of the PR:
To be honest, I had a tough time debugging this. Passing a slice, instead of a string, to serde deserialization makes it very annoying to debug since all parsing errors prints the characters as u8 instead of the correct JSON string and makes the debugging way more cumbersome. In case I keep using this crate I'll probably submit some other PRs to both add examples for move parts (had to reverse engineer the client credentials part to both know what to use and where this bug was) and better error messages.

This is useful feedback, thank you! I've also run into similar debugging pain. The reason this crate uses from_slice to deserialize the body is that the response body is a plain byte array. In order to use serde_json::from_str, we'd first need to UTF-8 decode the body, which would introduce a new error variant (containing a Utf8Error). I originally thought having a single error variant would be better, but in retrospect I think that was the wrong decision. I'd welcome a PR to split up the error cases, but that would be a breaking change that would need to wait for a 5.0 release. In the meantime, callers can do their own UTF-8 decoding when handling the error to print a string instead of the raw bytes.

@ramosbugs
Copy link
Copy Markdown
Owner

See also #52

@naumazeredo
Copy link
Copy Markdown
Author

Didn't check if Twitch was just breaking the RFC, sorry.
I'll try the TokenResponse approach, thanks for the info.

Closing.

@paulm17
Copy link
Copy Markdown

paulm17 commented Feb 19, 2025

@naumazeredo did you resolve this issue? I've exhausted all the AI's and they can't seem to resolve this issue. Totally stuck.

@bekriebel bekriebel mentioned this pull request Sep 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants