Skip to content

Commit

Permalink
Fix some email verification issues with external login providers.
Browse files Browse the repository at this point in the history
- Needed to add explicit config to get "verified" information back from
  Facebook.
- GitHub provider no longer needs explicit verified checks.
- Improve error message when the email address is unverified (to
  provide the email address in the error message and distinguish it from
  the unauthorized error).
  • Loading branch information
GUI committed Feb 20, 2017
1 parent f45b13b commit d4e6fc5
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,19 @@ def cas
end

def facebook
if(request.env["omniauth.auth"]["info"]["verified"])
@username = request.env["omniauth.auth"]["info"]["email"]
@username = request.env["omniauth.auth"]["info"]["email"]
if(!request.env["omniauth.auth"]["info"]["verified"])
return email_unverified_error
end

login
end

def github
if(request.env["omniauth.auth"]["info"]["email_verified"])
@username = request.env["omniauth.auth"]["info"]["email"]
end
# omniauth-github only returns verified emails by default (so no explicit
# verification check is needed):
# https://github.com/intridea/omniauth-github/pull/48
@username = request.env["omniauth.auth"]["info"]["email"]

login
end
Expand All @@ -51,8 +53,9 @@ def gitlab
end

def google_oauth2
if(request.env["omniauth.auth"]["extra"]["raw_info"]["email_verified"])
@username = request.env["omniauth.auth"]["info"]["email"]
@username = request.env["omniauth.auth"]["info"]["email"]
if(!request.env["omniauth.auth"]["extra"]["raw_info"]["email_verified"])
return email_unverified_error
end

login
Expand Down Expand Up @@ -94,6 +97,18 @@ def login
end
end

def email_unverified_error
flash[:error] = ActionController::Base.helpers.safe_join([
"The email address '",
@username,
"' is not verified. Please ",
ActionController::Base.helpers.content_tag(:a, "contact us", :href => ApiUmbrellaConfig[:contact_url]),
" for further assistance.",
])

redirect_to new_admin_session_path
end

def after_omniauth_failure_path_for(scope)
new_admin_session_path
end
Expand Down
3 changes: 2 additions & 1 deletion src/api-umbrella/web-app/config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@
config.omniauth :facebook,
ApiUmbrellaConfig[:web][:admin][:auth_strategies][:facebook][:client_id],
ApiUmbrellaConfig[:web][:admin][:auth_strategies][:facebook][:client_secret],
:scope => "email"
:scope => "email",
:info_fields => "name,email,verified"
when "cas"
require "omniauth-cas"
config.omniauth :cas,
Expand Down
9 changes: 4 additions & 5 deletions test/admin_ui/login/test_external_providers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ def test_no_password_field_on_admin_forms
:provider => :github,
:login_button_text => "Sign in with GitHub",
:username_path => "info.email",
:verified_path => "info.email_verified",
},
{
:provider => :gitlab,
Expand Down Expand Up @@ -172,7 +171,7 @@ def assert_login_nonexistent_admin(options)
LazyHash.add(omniauth_data, options.fetch(:username_path), "[email protected]")

mock_omniauth(omniauth_data) do
assert_login_forbidden(options.fetch(:login_button_text))
assert_login_forbidden(options.fetch(:login_button_text), "not authorized")
end
end

Expand All @@ -183,7 +182,7 @@ def assert_login_unverified_email_login(options)
LazyHash.add(omniauth_data, options.fetch(:verified_path), false)

mock_omniauth(omniauth_data) do
assert_login_forbidden(options.fetch(:login_button_text))
assert_login_forbidden(options.fetch(:login_button_text), "not verified")
end
end

Expand All @@ -193,10 +192,10 @@ def assert_login_permitted(login_button_text, admin)
assert_link("my_account_nav_link", :href => /#{admin.id}/, :visible => :all)
end

def assert_login_forbidden(login_button_text)
def assert_login_forbidden(login_button_text, error_text)
visit "/admin/"
trigger_click_link(login_button_text)
assert_text("not authorized")
assert_text(error_text)
refute_link("my_account_nav_link")
end

Expand Down

0 comments on commit d4e6fc5

Please sign in to comment.