Skip to content
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

[WIP - Help wanted!] Ensure preservation of return_to through OAuth login flow #3762

Closed
wants to merge 1 commit into from

Conversation

jywarren
Copy link
Member

Fixes #3384

I think this is only the beginning -- i'm not setting the session[:return_to] in the right places, nor am I setting up the redirects thoroughly... and it could probably use some refinement to not have redundant code.

@jywarren jywarren added the help wanted requires help by anyone willing to contribute label Oct 24, 2018
@ghost ghost assigned jywarren Oct 24, 2018
@ghost ghost added the in progress label Oct 24, 2018
@plotsbot
Copy link
Collaborator

1 Message
📖 @jywarren Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

@Radhikadua123
Copy link
Contributor

@jywarren Wow, this seems to be quite interesting issue. I was thinking about approach to it.

What about this:

  1. All the third party login icons will point to /redirect?from=#{url}&to=/auth/#{provider}.
  2. In that GET view:
    2.1 if user is already logged in, redirect user to from url. (handling edge case)
    2.2 if user is not logged in, just get the existing session or create new one. In that session, save the return_to url and redirect user to to url from get parameters.
  3. When callback from third party login is called, on success, we check if return_to is set in session or not. If it is set, then just redirect user to that url.

I'm not sure how well would it address the issue. I might have missed something.

@jywarren
Copy link
Member Author

jywarren commented Oct 30, 2018 via email

@Radhikadua123
Copy link
Contributor

Sure, I'll try it out tomorrow. Thanks!

@jywarren
Copy link
Member Author

jywarren commented Nov 2, 2018

Deferring to #3879

@jywarren jywarren closed this Nov 2, 2018
@ghost ghost removed the in progress label Nov 2, 2018
@emilyashley emilyashley deleted the oauth-return-to branch January 15, 2020 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted requires help by anyone willing to contribute
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants