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

Version 4.5.0 doesn't work with Facebook Limited Login #848

Closed
Tenzer opened this issue Nov 2, 2023 · 2 comments
Closed

Version 4.5.0 doesn't work with Facebook Limited Login #848

Tenzer opened this issue Nov 2, 2023 · 2 comments

Comments

@Tenzer
Copy link
Contributor

Tenzer commented Nov 2, 2023

Expected behaviour

After upgrading to version 4.5.0, logins using the Facebook Limited Login method should continue to work.

Actual behaviour

All login attempts instead fail with an AuthTokenError exception because the JWT coming from Facebook doesn't specify the at_hash claim and thus fails the new validation in 4.5.0.

What are the steps to reproduce this issue?

Input clear steps to reproduce the issue for a maintainer.

  1. Set up a site using Facebook Limited Login
  2. Attempt to log in using social-core 4.5.0

(It's a bit difficult to make this easier to reproduce without disclosing actual user information :/)

Any logs, error output, etc?

A heavily redacted example of the data Facebook Limited Login generates, note the lack of the at_hash field:

{
  "iss": "https://www.facebook.com",
  "aud": "1436880...",
  "sub": "35002149...",
  "iat": 1698925551,
  "exp": 1698929151,
  "jti": "5W6a.1598fbf937fa89654c1090d04a41...",
  "nonce": "AEB2DAD9-0347-46F3-...",
  "email": "<redacted>",
  "given_name": "<redacted>",
  "family_name": "<redacted>",
  "name": "<redacted>",
  "picture": "https://platform-lookaside.fbsbx.com/platform/profilepic/..."
}

Any other comments?

According to the OpenID Connect spec the at_hash field is optional, but it seems to always be checked inside OpenIdConnectAuth.validate_and_return_id_token. Is that correct?
I wonder if this could be solved with a relatively small change:

--- social_core/backends/open_id_connect.py
+++ social_core/backends/open_id_connect.py
@@ -236,7 +236,7 @@ class OpenIdConnectAuth(BaseOAuth2):

         # pyjwt does not validate OIDC claims
         # see https://github.com/jpadilla/pyjwt/pull/296
-        if claims.get("at_hash") != self.calc_at_hash(access_token, key["alg"]):
+        if "at_hash" in claims and claims.get("at_hash") != self.calc_at_hash(access_token, key["alg"]):
             raise AuthTokenError(self, "Invalid access token")

         self.validate_claims(claims)
@nijel
Copy link
Member

nijel commented Nov 2, 2023

This was introduced in #819. Looking at the code there are other backends which include JWT_DECODE_OPTIONS = {"verify_at_hash": False}, which will now do nothing (it will trigger RemovedInPyjwt3Warning).

So this should be addressed better:

  • Facebook Limited backend should get a test-case so that such error is detected earlier
  • JWT_DECODE_OPTIONS should be removed (they were used for jose and PyJWT API differs, so it's anyway unlikely that will work)
  • Introduce separate class attribute for backends to make at_hash validation optional

@Tenzer
Copy link
Contributor Author

Tenzer commented Jan 12, 2024

I can see my exact suggested change was added in #852 so I guess this is solved now and should work in version 4.5.1.

@Tenzer Tenzer closed this as completed Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants