Reuse callback_url between request phase and callback phase#142
Reuse callback_url between request phase and callback phase#142menisy wants to merge 1 commit intoomniauth:masterfrom
Conversation
|
@BobbyMcWho can we get some love here? Please and thank you ❤️ |
|
Sorry, I saw this and gave it some thought, but I'm not entirely sure if this could potentially cause cookie overflows if the redirect url was particularly long. I'd also have to release this PR in its current state in a major version bump since it's a breaking change from how it functioned previously |
|
Okay I also thought about the cookie overflow and you do have a point there. I can change the implementation to remove the I'm also not sure whether we should consider this as a breaking change since the auth provider is (or should) already expect this given that this is the OAuth2 specification. I wouldn't release this in a patch version but I would consider maybe a minor version. |
As in the subject line & perhaps a little easier to manage than omniauth#142.
As explained in #141, the
callback_urlbeing used in the callback phase is different than the one sent to the authorization server during the request phase (TLDR: During the callback phase,stateandcodeare included as query params within theredirect_uri). This does not comply with the spec which is very clear that theredirect_uri(akacallback_url) in the code-token exchange request must match the one sent in the authorization request.According to the spec:
PS: It was a bit tricky to write the tests in a way to test this flow within the
callback_phasemethod given the amount of mocking needed for the happy case (especially for parent class logic), so I opted for testing thebuild_access_tokenmethod.