Replace API calls to /userinfo with payload from id_token
#235
Conversation
lib/ueberauth/strategy/auth0.ex
Outdated
|
|
||
| {:error, %Response{body: body}} -> | ||
| set_errors!(conn, [error("OAuth2", body)]) | ||
| [_header, payload, _signature] = String.split(id_token, ".") |
There was a problem hiding this comment.
Is it possible to validate the JWT instead of manually parsing it ?
It doesn't look like a good idea to skip signature and header checks.
Be sure to validate ID tokens before using the information it contains.
You can add one of these three libs: https://jwt.io/libraries?language=Elixir
I would suggest using joken as it's the most complete and lightweight given the task.
There was a problem hiding this comment.
heya @achedeuzot 100%
This is just WIP, I am currently working on adding the JWT validation! This means there will be a breaking change in Auth0 as we have to start requiring all devs to use the .pem signing keyfrom Auth0
There was a problem hiding this comment.
@achedeuzot can you have a look now? :)
There was a problem hiding this comment.
@taj Thanks so much for your contributions. I'm a bit overwhelmed with tasks lately but will have a look very soon (and if everything goes well, will be able to release a new package).
There was a problem hiding this comment.
@achedeuzot no worries, take your time! We have forked this repo and have been using this branch in our production applications already and everything seems to be working correctly.
Let me know if I need to provide any further info or support!
test/strategy/auth0_test.exs
Outdated
| assert conn.resp_body =~ ~s|scope=openid+profile+email| | ||
| assert conn.resp_body =~ ~s|state=#{conn.private[:ueberauth_state_param]}| | ||
| end | ||
| describe "handle_request!" do |
There was a problem hiding this comment.
Could you split the test refactoring into 2 separate commits or, even better, into two separate pull requests ? This will make the reviewing easier by first moving the tests into describe blocks with no changes and then add tests for the current PR. Right now I'm having some trouble to find what has really changed from what is a simple refactor ? Thanks :)
There was a problem hiding this comment.
I've actually refactored the existing tests which are on the master branch as well as fixed the CI tests. If you rebase it should be quite fast to update your branch :)
taj
force-pushed
the
sc-21571/fix-ueberauth-auth0-rate-limit
branch
2 times, most recently
from
ceba09c to
bb88f23
Compare
| @@ -3,7 +3,7 @@ use Mix.Config | |||
| config :ueberauth, Ueberauth, | |||
| json_library: Jason, | |||
| providers: [ | |||
| auth0: {Ueberauth.Strategy.Auth0, []} | |||
| auth0: {Ueberauth.Strategy.Auth0, [signer_module: SpecSignerModule]} | |||
taj
force-pushed
the
sc-21571/fix-ueberauth-auth0-rate-limit
branch
from
bb88f23 to
c4ef00d
Compare
|
BTW, have you seen the #175 PR ? It could give you some ideas about your feature ;) |
|
@taj In ran the CI but it's red because of a few things :) Have you checked the PR of the previous comment ? It could help ;) |
Hey @achedeuzot, sorry was on holiday for the past week, I will try to fix everything this week! :) |
We are going above Auth0's rate limits on a daily basis.
After some investigation we realised that the
ueberauth_auth0was making API calls to the/userinfoendpoint every time someone logs in (oncallback).We actually don't need to do this as the user info is returned in the
id_token.The
id_tokenfollows the the JWT standard.