fxa-client: add has_scope() method.#7459
Conversation
mhammond
force-pushed
the
push-wxpsknpymslz
branch
from
d65c334 to
8a36dfb
Compare
mhammond
force-pushed
the
push-wxpsknpymslz
branch
from
8a36dfb to
49e7705
Compare
jonalmeida
left a comment
jonalmeida
left a comment
There was a problem hiding this comment.
Ha, nice thank you for this! I left a question on usage expectations, but it's a non-blocker.
| scopes: ["profile", "sync"].iter().map(|s| s.to_string()).collect(), | ||
| }); | ||
| assert!(fxa.has_scope("profile")); | ||
| assert!(fxa.has_scope("sync profile")); |
There was a problem hiding this comment.
Is this an accurate expectation? In the persisted state, we have an array of individual scopes, so a space-delimited list of scopes wouldn't work there.
Our access tokens are based on the grouped scopes, so that tracks if we were looking at that.
There was a problem hiding this comment.
I don't fully understand your question. The API using space sep'd scopes is odd, but I went with that to match how get_access_token() works - the impl splits it. So roughly, this is saying "if I pass this exact string of scopes to get_access_token(), is it going to think the refresh token can handle this without an exchange".
So my expectation is that passing all of ".../oldsync profile", "profile .../oldsync" and "profile" would return true for the token in your screenshot.
I'm open to changing the api to be a vec, and that's probably what should have been done earlier - but IMO the consistency with get_access_token() seems the lesser of 2 evils. Or maybe that's not what you are referring to?
2 participants
Pull Request checklist
[ci full]to the PR title.