Better support for passing up errors when OmniAuth fails after auth

This commit is contained in:
Robin Ward 2015-06-24 12:12:43 -04:00
parent f18098fd9b
commit b4960d48b4
5 changed files with 56 additions and 34 deletions

View File

@ -34,14 +34,18 @@ class Users::OmniauthCallbacksController < ApplicationController
authenticator = self.class.find_authenticator(params[:provider]) authenticator = self.class.find_authenticator(params[:provider])
@data = authenticator.after_authenticate(auth) @auth_result = authenticator.after_authenticate(auth)
@data.authenticator_name = authenticator.name
complete_response_data if @auth_result.failed?
flash[:error] = @auth_result.failed_reason.html_safe
respond_to do |format| return render('failure')
format.html else
format.json { render json: @data.to_client_hash } @auth_result.authenticator_name = authenticator.name
complete_response_data
respond_to do |format|
format.html
format.json { render json: @auth_result.to_client_hash }
end
end end
end end
@ -69,35 +73,35 @@ class Users::OmniauthCallbacksController < ApplicationController
protected protected
def complete_response_data def complete_response_data
if @data.user if @auth_result.user
user_found(@data.user) user_found(@auth_result.user)
elsif SiteSetting.invite_only? elsif SiteSetting.invite_only?
@data.requires_invite = true @auth_result.requires_invite = true
else else
session[:authentication] = @data.session_data session[:authentication] = @auth_result.session_data
end end
end end
def user_found(user) def user_found(user)
# automatically activate any account if a provider marked the email valid # automatically activate any account if a provider marked the email valid
if !user.active && @data.email_valid if !user.active && @auth_result.email_valid
user.toggle(:active).save user.toggle(:active).save
end end
if ScreenedIpAddress.should_block?(request.remote_ip) if ScreenedIpAddress.should_block?(request.remote_ip)
@data.not_allowed_from_ip_address = true @auth_result.not_allowed_from_ip_address = true
elsif ScreenedIpAddress.block_admin_login?(user, request.remote_ip) elsif ScreenedIpAddress.block_admin_login?(user, request.remote_ip)
@data.admin_not_allowed_from_ip_address = true @auth_result.admin_not_allowed_from_ip_address = true
elsif Guardian.new(user).can_access_forum? && user.active # log on any account that is active with forum access elsif Guardian.new(user).can_access_forum? && user.active # log on any account that is active with forum access
log_on_user(user) log_on_user(user)
Invite.invalidate_for_email(user.email) # invite link can't be used to log in anymore Invite.invalidate_for_email(user.email) # invite link can't be used to log in anymore
session[:authentication] = nil # don't carry around old auth info, perhaps move elsewhere session[:authentication] = nil # don't carry around old auth info, perhaps move elsewhere
@data.authenticated = true @auth_result.authenticated = true
else else
if SiteSetting.must_approve_users? && !user.approved? if SiteSetting.must_approve_users? && !user.approved?
@data.awaiting_approval = true @auth_result.awaiting_approval = true
else else
@data.awaiting_activation = true @auth_result.awaiting_activation = true
end end
end end
end end

View File

@ -22,7 +22,7 @@
<p><%=t "login.close_window" %></p> <p><%=t "login.close_window" %></p>
<script type="text/javascript"> <script type="text/javascript">
window.opener.Discourse.authenticationComplete(<%=@data.to_client_hash.to_json.html_safe%>); window.opener.Discourse.authenticationComplete(<%=@auth_result.to_client_hash.to_json.html_safe%>);
window.close(); window.close();
</script> </script>
</div> </div>

View File

@ -1,11 +1,20 @@
<div id="simple-container"> <html>
<%if flash[:error]%> <head>
<div class='alert alert-error'> <%= render partial: "layouts/head" %>
<%=flash[:error]%> <%= render partial: "common/special_font_face" %>
<%= render partial: "common/discourse_stylesheet" %>
</head>
<body>
<div id="simple-container">
<%if flash[:error].present? %>
<div class='alert alert-error'>
<%=flash[:error]%>
</div>
<%else%>
<div class='alert alert-error'>
<%= t 'login.omniauth_error_unknown' %>
</div>
<%end%>
</div> </div>
<%else%> </body>
<div class='alert alert-error'> </html>
<% t 'login.omniauth_error_unknown' %>
</div>
<%end%>
</div>

View File

@ -5,15 +5,24 @@ class Auth::Result
:requires_invite, :not_allowed_from_ip_address, :requires_invite, :not_allowed_from_ip_address,
:admin_not_allowed_from_ip_address :admin_not_allowed_from_ip_address
attr_accessor :failed,
:failed_reason
def initialize
@failed = false
end
def failed?
!!@failed
end
def session_data def session_data
{ { email: email,
email: email,
username: username, username: username,
email_valid: email_valid, email_valid: email_valid,
name: name, name: name,
authenticator_name: authenticator_name, authenticator_name: authenticator_name,
extra_data: extra_data extra_data: extra_data }
}
end end
def to_client_hash def to_client_hash

View File

@ -13,7 +13,7 @@ describe "users/omniauth_callbacks/complete.html.erb" do
result = Auth::Result.new result = Auth::Result.new
result.user = User.new result.user = User.new
assign(:data, result) assign(:auth_result, result)
render render
@ -28,7 +28,7 @@ describe "users/omniauth_callbacks/complete.html.erb" do
result.email = "xxx@xxx.com" result.email = "xxx@xxx.com"
result.authenticator_name = "CAS" result.authenticator_name = "CAS"
assign(:data, result) assign(:auth_result, result)
render render