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.

Sign up for GitHub

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

Jump to bottom

Ensure preservation of return_to through OAuth login flow #3884

Merged
merged 1 commit into from
Nov 9, 2018
Merged

Ensure preservation of return_to through OAuth login flow #3884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions app/controllers/user_sessions_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def create
def handle_social_login_flow(auth)
# Find an identity here
@identity = UserTag.find_with_omniauth(auth)
return_to = request.env['omniauth.origin'] || root_url

if signed_in?
if @identity.nil?
Expand All @@ -27,27 +28,27 @@ def handle_social_login_flow(auth)
# associate the identity
@identity.user = current_user
@identity.save
redirect_to root_url, notice: "Successfully linked to your account!"
redirect_to return_to, notice: "Successfully linked to your account!"
elsif @identity.user == current_user
# User is signed in so they are trying to link an identity with their
# account. But we found the identity and the user associated with it
# is the current user. So the identity is already associated with
# this user. So let's display an error message.
redirect_to root_url, notice: "Already linked to your account!"
redirect_to return_to, notice: "Already linked to your account!"
else
# User is signed in so they are trying to link an identity with their
# account. But we found the identity and a different user associated with it
# ,which is not the current user. So the identity is already associated with
# that user. So let's display an error message.
redirect_to root_url, notice: "Already linked to another account!"
redirect_to return_to, notice: "Already linked to another account!"
end
else # not signed in
if @identity&.user.present?
# The identity we found had a user associated with it so let's
# just log them in here
@user = @identity.user
@user_session = UserSession.create(@identity.user)
redirect_to root_url, notice: "Signed in!"
redirect_to return_to, notice: "Signed in!"
else # identity does not exist so we need to either create a user with identity OR link identity to existing user
if User.where(email: auth["info"]["email"]).empty?
# Create a new user as email provided is not present in PL database
Expand All @@ -59,7 +60,7 @@ def handle_social_login_flow(auth)
@user = user
# send key to user email
PasswordResetMailer.reset_notify(user, key).deliver_now unless user.nil? # respond the same to both successes and failures; security
redirect_to root_url, notice: "You have successfully signed in. Please change your password via a link sent to you via e-mail"
redirect_to return_to, notice: "You have successfully signed in. Please change your password via a link sent to you via e-mail"
else # email exists so link the identity with existing user and log in the user
user = User.where(email: auth["info"]["email"])
# If no identity was found, create a brand new one here
Expand All @@ -70,7 +71,7 @@ def handle_social_login_flow(auth)
@user = user
# log in them
@user_session = UserSession.create(@identity.user)
redirect_to root_url, notice: "Successfully linked to your account!"
redirect_to return_to, notice: "Successfully linked to your account!"
end
end
end
Expand Down
45 changes: 3 additions & 42 deletions app/views/layouts/_header.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
<% end %>
</h5>
</div>

<li><a href="/profile/<%= current_user.username %>"><%= t('layout._header.profile') %></a></li>
<li><a href="/profile/edit"><%= t('layout._header.edit_profile') %></a></li>
<li><a href="/settings" >Email Settings</a></li>
Expand All @@ -121,48 +121,9 @@
<li><a href="/logoutRemotely">Logout from all devices</a></li>
</ul>
<% else %>
<a class="hidden-sm" href="#" class="dropdown-toggle" data-toggle="dropdown">
<%= t('layout._header.login.login_title') %>
<a href="/login?return_to=<%= params[:return_to] || request.path %>">
<%= t('layout._header.login.login_title') %>
</a>
<ul id="login-dropdown" class="dropdown-menu" style="width:245px; color: white;">
Copy link
Member

@SidharthBansal SidharthBansal Nov 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have deleted the form by mistake I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it's w.r.t. this comment : #3879 (comment)

Though in case you feel we need it, I can try adding it back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

When Jeff and I implemented this OAuth system we made this working on the localhost, unstable, stable and on the production.
So, once you deploy this onto the unstable and check the functionality for all the providers, ping me. I will check it.

Thanks for the awesome hard work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I wasn't sure how to get those credentials for local setup. So, I tried it on unstable only.

Yeah sure, I'll ping you once it's deployed. Build seems to be failed due to some reason and I don't have permission to start it manually from there. May be I'll push it again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might help you
https://github.com/publiclab/plots2/blob/master/doc/OMNIAUTH.md

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check for all providers on the development. Just create anyone say github and check it on local.
#2856
On unstable we need to check it on all providers.
My insights says that your solution will work. I did the same solution in February.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/publiclab/plots2/blob/master/doc/OMNIAUTH.md

Thanks, I understand the process now!

On unstable we need to check it on all providers.

Oh cool

My insights says that your solution will work. I did the same solution in February.

Oh, so did it get dropped mistakenly in refactoring or something ?

cool, I'll try it out.

<li style="padding:6px;">
<%= form_for :user_session, :as => :user_session, :url => { :controller => "user_sessions", :action => "create" }, :html => {:class => "form"} do |f| %>
<div class="form-group">
<% if Rails.env == "production" %>
<label style="display: block; text-align: center;">Log In with</label>
<div class="loginbutton" style="float:left">
<%= render :partial => "layouts/social_icons" %>
</div>
<br style="clear:both;"/>
<label style="display: block; text-align: center; margin-top:13px;">OR</label>
<% end %>
</div>
<form>
<div class="form-group">
<label for="username"><%= t('layout._header.login.username') %></label>
<%= f.text_field :username, { tabindex: 1, class: 'form-control', id: 'username-login' } %>
</div>
<div class="form-group">
<label for="password"><%= t('layout._header.login.password') %></label>
<%= f.password_field :password, { tabindex: 2, class: 'form-control', id: 'password-login' } %>
</div>
<input type="hidden" name="return_to" value="<%= params[:return_to] || request.path %>" />
<div class="form-check float-right">
<label>
<%= f.check_box :remember_me %> <%= t('layout._header.login.remember_me') %>
</label>
</div>
<div class="form-group">
<button class="btn btn-primary" type="submit" tabindex="3"><%= t('layout._header.login.login_title') %></button>
<button class="btn btn-default" href="/signup" tabindex="4"><%= t('layout._header.login.sign_up') %></button>
</div>
<div>
<a href="/reset/"><%= t('layout._header.login.reset_password') %></a>
</div>
</form>
<% end %>
</li>
</ul>
<% end %>
<% end %>
</li>
Expand Down
24 changes: 20 additions & 4 deletions app/views/layouts/_social_icons.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
<a href="/auth/google_oauth2" id="connect-google"><span class="btn btn-default" style="margin-right:2px;background-color: #d34836;"><i class="fa fa-google fa-fw" style="font-size:20px;color:white;"></i></span></a>
<a href="/auth/github" id="connect-github"><span class="btn btn-default" style="margin-left:2px;background-color: #333;"><i class="fa fa-github fa-fw" style="font-size:20px;color:white;"></i></span></a>
<a href="https://publiclab.org/auth/twitter" id="connect-twitter"><span class="btn btn-default" style="margin-left:2px;background-color: #1da1f2;"><i class="fa fa-twitter fa-fw" style="font-size:20px;color:white;margin-left:2px;"></i></span></a>
<a href="/auth/facebook" id="connect-facebook"><span class="btn btn-default" style="margin-left:2px;background-color: #3b5998"><i class="fa fa-facebook fa-fw" style="font-size:20px;color:white;"></i></span></a>
<a href="/auth/google_oauth2?origin=<%= params[:return_to] || root_url %>" id="connect-google">
<span class="btn btn-default" style="margin-right:2px;background-color: #d34836;">
<i class="fa fa-google fa-fw" style="font-size:20px;color:white;"></i>
</span>
</a>
<a href="/auth/github?origin=<%= params[:return_to] || root_url %>" id="connect-github">
<span class="btn btn-default" style="margin-left:2px;background-color: #333;">
<i class="fa fa-github fa-fw" style="font-size:20px;color:white;"></i>
</span>
</a>
<a href="/auth/twitter?origin=<%= params[:return_to] || root_url %>" id="connect-twitter">
<span class="btn btn-default" style="margin-left:2px;background-color: #1da1f2;">
<i class="fa fa-twitter fa-fw" style="font-size:20px;color:white;margin-left:2px;"></i>
</span>
</a>
<a href="/auth/facebook?origin=<%= params[:return_to] || root_url %>" id="connect-facebook">
<span class="btn btn-default" style="margin-left:2px;background-color: #3b5998">
<i class="fa fa-facebook fa-fw" style="font-size:20px;color:white;"></i>
</span>
</a>