Skip to content

Conversation

@nstapelbroek
Copy link
Contributor

Changes

Heya 👋, This PR will introduce backwards incompatible changes I left out in #135. Let me know what you think.

Concrete changes:

Result is that brand new sessions that start on a /login will now work because the redirect response returned wil have set-cookie headers.

References

#135

Testing

Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. If this library has unit and/or integration testing, tests should be added for new functionality and existing tests should complete without errors.

[ ] This change has been tested on the latest version Laravel

Checklist

[x] I have read the Auth0 general contribution guidelines

[x] I have read the Auth0 Code of Conduct

nstapelbroek and others added 9 commits July 21, 2019 21:00
Also, by binding them both to your container you can extend or overwrite the behaviour a bit more easily. Which should help with testing or implemting different auth flows.
Use LaravelSessionStore in the SessionStateHandler.
Explicitly save session
Since I broke that in my previous commit
Since we now let the Framework exit the session and cookie middleware can
take care of that again
@joshcanhelp
Copy link
Contributor

@nstapelbroek - Thanks for this! Instead of recreating all the behavior in the SDK, do you think it would be more helpful to us to add a method to the SDK that just builds out the authorization URL and hands it back? I can use the same method in the core login() one to do the redirection, then we won't break anything. I'm working on a minor release for the SDK now and can include that as part of it.

@joshcanhelp joshcanhelp requested review from joshcanhelp and removed request for a team September 23, 2019 15:10
@joshcanhelp joshcanhelp self-assigned this Sep 23, 2019
@joshcanhelp joshcanhelp removed their request for review September 23, 2019 15:11
@joshcanhelp
Copy link
Contributor

@nstapelbroek - Alright, I put through a PR on the PHP SDK that should help with this:

auth0/auth0-PHP#371

You get use that new \Auth0\SDK\Auth0::getLoginUrl() method in that Auth0Service::login()method to get the authorize URL exactly the same way then add the return new RedirectResponse($url); just below and you should be good to go. You should also bump auth0/auth0-php in the composer file to ^5.6.0 to make sure that method is available. Happy to put through that PR myself if you don't have enough time.

Probably best to back all the session changes out of this one and keep #135 as-is. That way we can get all this working with no backwards-incompatible changes! Feel free to provide any feedback you have on the PR linked above, otherwise we'll get that reviewed/merged/released ASAP.

Thank you once again!

@joshcanhelp joshcanhelp added this to the 5.3.0 milestone Sep 24, 2019
@nstapelbroek
Copy link
Contributor Author

No longer needed because we found a better way in #135

@joshcanhelp joshcanhelp removed this from the 5.3.0 milestone Sep 26, 2019
@nstapelbroek nstapelbroek deleted the redirect-using-laravel-framework branch September 27, 2019 07:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants