omniauth / omniauth

OmniAuth is a flexible authentication system utilizing Rack middleware.
MIT License
7.92k stars 976 forks source link

Should we memoize #OmniAuth::Strategy#credentials? #999

Open zinosama opened 4 years ago

zinosama commented 4 years ago

Expected Behavior

#credentials to be called once per callback request.

Actual Behavior

#credentials is called twice per callback request.

Question

Is there any reason OmniAuth::Strategy#credentials is not memoized given that OmniAuth::Strategy#auth_hash always invokes it twice in a row:

# OmniAuth::Strategy#auth_hash 
hash.credentials = credentials if credentials

This results in all providers' credentials block to be evaluated twice per callback request.

I'm happy to open a fix for this if maintainers are open to it. Thoughts? @BobbyMcWho

BobbyMcWho commented 4 years ago

I'm open to a PR, I'm not intimate with that section of code in particular