mozilla / PyFxA

Python library for interacting with the Firefox Accounts ecosystem
Other
30 stars 20 forks source link

"at+JWT" case mismatch between mozilla/fxa and pyfxa #84

Closed jackyzy823 closed 4 years ago

jackyzy823 commented 4 years ago

When i using kinto-fxa. I encountered some issues. After some digging , i found that it may caused by PyFxa.

"at+JWT" in https://github.com/mozilla/fxa/blob/e440dae101bbd5d90e26dfcf60bea3edde8f362a/packages/fxa-auth-server/lib/oauth/jwt_access_token.js#L13 do not match in case https://github.com/mozilla/PyFxA/blob/68e576338118c0d8265f65b8003a65a77c507ef1/fxa/oauth.py#L236 which raise TrustError

should be

if jwt.get_unverified_header(token).get('typ').lower() != 'at+jwt': 
rfk commented 4 years ago

Thanks! Could you please try out the proposed fix in https://github.com/mozilla/PyFxA/pull/85 and let me know if it works for you?

jackyzy823 commented 4 years ago

Thanks for fix. And there may be an another question https://github.com/mozilla/PyFxA/blob/68e576338118c0d8265f65b8003a65a77c507ef1/fxa/oauth.py#L308-L310

in scope_matches, authorized_scope is assumed to be a list or tuple . However which i get from self hosting FxA v1.178.0. authorized_scope is a string. so scope_matches iterates every single char of authorized_scope to match scope which obviously comes to a fail.

Reference for fxa: https://github.com/mozilla/fxa/blob/46f02be0f2e8871503401efa6b1bb44c5fdb7967/packages/fxa-auth-server/lib/oauth/jwt_access_token.js#L39

I'm not sure whether this question belongs to server or client.

Update: i thinks it's PyFxA 's question.

Previously PyFxA use /v1/verify to verify token. and fxa returns scope: ScopeSet.fromString(t.scope),

reference : https://github.com/mozilla/fxa/blob/46f02be0f2e8871503401efa6b1bb44c5fdb7967/packages/fxa-auth-server/lib/oauth/token.js#L46

Now PyFxA use /v1/jwks and it return scope as a string.

since /v1/verify is a fallback method. scope_matches should handle both types of scope.

for string type , split the scope string by whitespace into list.

rfk commented 4 years ago

Yes, good call; I've added this fix as part of #85.

jackyzy823 commented 4 years ago

Works like a charm. Thanks. LGTM :thumbsup:

rfk commented 4 years ago

Thanks again for your help digging into these @jackyzy823!