-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Keep returnTo when using successReturnToOrRedirect #941
base: master
Are you sure you want to change the base?
Keep returnTo when using successReturnToOrRedirect #941
Conversation
@jaredhanson Can we get this fix in? |
@jaredhanson please fix it, so then there will be no need of a workaround by using keepSessionInfo flag as returnTo will be preserved. |
@shashank686 The work around did not work for me. Did it work for you? So right now the return too is not perserved and my app is failing with that behavior if i upgrade. |
Is this going to be merged? |
I don't think so. Seems like this repo is dead and no longer maintained. |
That sucks to hear. Do you know an alternative package that does the same job? |
It's still maintained. As a general rule, I don't merge pull requests that lack corresponding tests. If this PR is updated with test cases, I will merge and publish. Otherwise, it'll wait until I have the time to write the tests. Thanks. |
5fd7393
to
fd41914
Compare
We patch passport with the code from https://github .com/jaredhanson/passport/pull/941, which excludes session.returnTo from reset on login. Fixes #4466 Co-authored-by: Graham White <graham_alton@hotmail.com> Signed-off-by: David Mehren <git@herrmehren.de>
We patch passport with the code from https://github .com/jaredhanson/passport/pull/941, which excludes session.returnTo from reset on login. Fixes #4466 Co-authored-by: Graham White <graham_alton@hotmail.com> Signed-off-by: David Mehren <git@herrmehren.de>
We patch passport with the code from https://github .com/jaredhanson/passport/pull/941, which excludes session.returnTo from reset on login. Fixes #4466 Co-authored-by: Graham White <graham_alton@hotmail.com> Signed-off-by: David Mehren <git@herrmehren.de>
This is a small patch to fix the behaviour of the
successReturnToOrRedirect
option as illustrated in #919 and #281As I understand it, the
successReturnToOrRedirect
option is supposed to, upon success, return or redirect to the string specified insession.returnTo
or ifsession.returnTo
is not specified then return/redirect to the string specified in thesuccessReturnToOrRedirect
option.The problem with the situation as-is, is that
session.returnTo
is wiped out includingsession.returnTo
and thus the user is always routed towards the default as specified insuccessReturnToOrRedirect
. This behaviour is unexpected and one would expect that when you have set areturnTo
value in the session and also specified thesuccessReturnToOrRedirect
option that the user would be directed towards the value specified in the session. As described in #919, there is a workaround by setting thekeepSessionInfo
option to true but this workaround involves keeping all of the previous session values rather than the minimal set required forsuccessReturnToOrRedirect
to operated as expected.This patch ensures that if
successReturnToOrRedirect
is being used and the session contains areturnTo
value that thesession.returnTo
value is maintained and thus the expected behaviour is implemented.Checklist
$ make test
) executes successfully.$ make lint
) executes successfully.