From d4e6fc5ff2acd0f0e789639e3117e9a233eab4e6 Mon Sep 17 00:00:00 2001
From: Nick Muerdter <nick.muerdter@nrel.gov>
Date: Sun, 19 Feb 2017 21:06:50 -0700
Subject: [PATCH] Fix some email verification issues with external login
 providers.

- 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).
---
 .../admins/omniauth_callbacks_controller.rb   | 29 ++++++++++++++-----
 .../web-app/config/initializers/devise.rb     |  3 +-
 .../admin_ui/login/test_external_providers.rb |  9 +++---
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/api-umbrella/web-app/app/controllers/admin/admins/omniauth_callbacks_controller.rb b/src/api-umbrella/web-app/app/controllers/admin/admins/omniauth_callbacks_controller.rb
index c001ab71e..6dab0bd59 100644
--- a/src/api-umbrella/web-app/app/controllers/admin/admins/omniauth_callbacks_controller.rb
+++ b/src/api-umbrella/web-app/app/controllers/admin/admins/omniauth_callbacks_controller.rb
@@ -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
@@ -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
@@ -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
diff --git a/src/api-umbrella/web-app/config/initializers/devise.rb b/src/api-umbrella/web-app/config/initializers/devise.rb
index 32dc6906c..807f14bf4 100644
--- a/src/api-umbrella/web-app/config/initializers/devise.rb
+++ b/src/api-umbrella/web-app/config/initializers/devise.rb
@@ -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,
diff --git a/test/admin_ui/login/test_external_providers.rb b/test/admin_ui/login/test_external_providers.rb
index 9eae152cd..dbcde5bac 100644
--- a/test/admin_ui/login/test_external_providers.rb
+++ b/test/admin_ui/login/test_external_providers.rb
@@ -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,
@@ -172,7 +171,7 @@ def assert_login_nonexistent_admin(options)
     LazyHash.add(omniauth_data, options.fetch(:username_path), "noadmin@example.com")
 
     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
 
@@ -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
 
@@ -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