Improve login-time error message for invalid Login.gov account (#2076)

Author: sanason

.env.test

@@ -7,10 +7,10 @@ AWS_SES_REGION=
 
 INDEX_URL=/index
 
-LOGIN_GOV_CLIENT_ID=
-LOGIN_GOV_IDP_BASE_URL=
+LOGIN_GOV_CLIENT_ID=test
+LOGIN_GOV_IDP_BASE_URL=https://idp.int.identitysandbox.gov
 LOGIN_GOV_PRIVATE_KEY=
-LOGIN_GOV_REDIRECT_URI=
+LOGIN_GOV_REDIRECT_URI=http://localhost:3000/users/auth/login_dot_gov/callback
 
 NEW_RELIC_KEY=
 

app/controllers/users/omniauth_callbacks_controller.rb

@@ -4,12 +4,17 @@ module Users
   class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     def login_dot_gov
       @kind = 'Login.gov'
-      all_emails = auth_hash['info']['all_emails']
-      if all_emails.present?
-        @email = auth_hash['info']['all_emails'].find { |email| email.end_with?(".gov") || email.end_with?(".mil") } if auth_hash && auth_hash['info']['email_verified']
-      else
-        @email = auth_hash['info']['email'] if auth_hash && auth_hash['info']['email_verified']
+      if auth_hash['info']['all_emails'].present?
+        @email = auth_hash['info']['all_emails'].find { |email| email.end_with?('.gov', '.mil') } if auth_hash['info']['email_verified']
+      elsif auth_hash['info']['email'].present?
+        @email = auth_hash['info']['email'] if auth_hash['info']['email'].end_with?('.gov', '.mil') && auth_hash['info']['email_verified']
       end
+
+      if @email.blank?
+        Event.log_event(Event.names[:user_authentication_failure], 'Event::Generic', 1, "Login.gov account with no verified .gov/.mil email attempted to log in on #{Date.today}")
+        redirect_to index_path, alert: t('home.invalid_login_account') and return
+      end
+
       login
     end
 
@@ -34,15 +39,9 @@ def auth_hash
     def login
       Event.log_event(Event.names[:user_authentication_attempt], 'Event::Generic', 1, "Email #{@email} attempted to authenticate on #{Date.today}")
 
-      @user = User.from_omniauth(auth_hash, @email) if @email.present?
+      @user = User.from_omniauth(auth_hash, @email)
 
-      # If user exists
-      # Else, if valid email and no user, we create an account.
-      if @user.blank?
-        message = "Email #{@email} failed to authenticate on #{Date.today} via #{@kind}"
-        Event.log_event(Event.names[:user_authentication_failure], 'Event::Generic', 1, message)
-        redirect_to index_path, alert: message
-      elsif @user.errors.blank?
+      if @user.errors.blank?
         Event.log_event(Event.names[:user_authentication_successful], 'User', @user.id, "User #{@user.email} successfully authenticated on #{Date.today}", @user.id)
         sign_in_and_redirect(:user, @user)
       elsif @user.errors.present?

config/initializers/omniauth.rb

@@ -23,7 +23,7 @@
     client_id: ENV.fetch('LOGIN_GOV_CLIENT_ID'),
     idp_base_url: ENV.fetch('LOGIN_GOV_IDP_BASE_URL'),
     ial: 1,
-    private_key: OpenSSL::PKey::RSA.new(ENV.fetch('LOGIN_GOV_PRIVATE_KEY').gsub('\\n', "\n")),
+    private_key: Rails.env.test? ? 'dummy_key' : OpenSSL::PKey::RSA.new(ENV.fetch('LOGIN_GOV_PRIVATE_KEY').gsub('\\n', "\n")),
     redirect_uri: ENV.fetch('LOGIN_GOV_REDIRECT_URI'),
   }
 end

config/locales/en.yml

@@ -177,6 +177,7 @@ en:
     sign_up_text: Any federal employee with a .gov or .mil email address can use Touchpoints. Your account will be automatically created the first time you sign in.
     sign_in_text: Touchpoints uses Login.gov to handle user accounts. Once you authenticate with Login.gov, you will be signed in and redirected back to Touchpoints.
     sign_in_with: Sign in with
+    invalid_login_account: "This Login.gov account cannot be used to access Touchpoints because it is not associated with a verified .gov or .mil email address. If you are a government employee, please follow <a href='https://login.gov/help/manage-your-account/change-your-email-address/'>these instructions</a> to add your work email to this account, or sign in with a different Login.gov account that uses your work email."
   the_feedback_and_analytics_team: " the Feedback and Analytics Team"
   characters_allowed: "characters allowed"
   characters_left: "characters left"

spec/controllers/users/omniauth_callbacks_controller_spec.rb

@@ -0,0 +1,277 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Users::OmniauthCallbacksController, type: :controller do
+  before do
+    @request.env['devise.mapping'] = Devise.mappings[:user]
+    OmniAuth.config.test_mode = true
+  end
+
+  after do
+    OmniAuth.config.test_mode = false
+  end
+
+  let!(:example_organization_gov) { FactoryBot.create(:organization, domain: 'example.gov') }
+  let!(:example_organization_mil) { FactoryBot.create(:organization, domain: 'army.mil') }
+
+  describe 'GET #login_dot_gov' do
+    context 'with verified .gov email in all_emails array' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12345',
+          info: {
+            all_emails: ['user@example.gov', 'personal@gmail.com'],
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/forms'
+      end
+
+      it 'successfully authenticates the new user' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_attempt], 'Event::Generic', 1, anything)
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_successful], 'User', anything, anything, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to('/admin/forms')
+        user = User.find_by(email: 'user@example.gov')
+        expect(controller.current_user).to eq(user)
+      end
+    end
+
+    context 'with verified .mil email in all_emails array' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12346',
+          info: {
+            all_emails: ['soldier@army.mil', 'personal@gmail.com'],
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/forms'
+      end
+
+      it 'successfully authenticates the new user' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_attempt], 'Event::Generic', 1, anything)
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_successful], 'User', anything, anything, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to('/admin/forms')
+        user = User.find_by(email: 'soldier@army.mil')
+        expect(controller.current_user).to eq(user)
+      end
+    end
+
+    context 'with verified .gov email in email field' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12347',
+          info: {
+            email: 'user@example.gov',
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/profile'
+      end
+
+      it 'successfully authenticates the new user' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_attempt], 'Event::Generic', 1, anything)
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_successful], 'User', anything, anything, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to('/profile')
+        user = User.find_by(email: 'user@example.gov')
+        expect(controller.current_user).to eq(user)
+      end
+    end
+
+    context 'with no verified .gov or .mil email' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12348',
+          info: {
+            all_emails: ['personal@gmail.com'],
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/forms'
+      end
+
+      it 'redirects with error message' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_failure], 'Event::Generic', 1, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to(index_path)
+        expect(flash[:alert]).to include('This Login.gov account cannot be used')
+      end
+    end
+
+    context 'with unverified email' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12349',
+          info: {
+            all_emails: ['user@example.gov'],
+            email_verified: false
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/forms'
+      end
+
+      it 'redirects with error message' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_failure], 'Event::Generic', 1, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to(index_path)
+        expect(flash[:alert]).to include('This Login.gov account cannot be used')
+      end
+    end
+
+    context 'with no email provided' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12350',
+          info: {}
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/forms'
+      end
+
+      it 'redirects with error message' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_failure], 'Event::Generic', 1, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to(index_path)
+        expect(flash[:alert]).to include('This Login.gov account cannot be used')
+      end
+    end
+
+    context 'when user creation fails' do
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12351',
+          info: {
+            email: 'user@agency.gov',
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/forms'
+      end
+
+      it 'redirects with error message' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_attempt], 'Event::Generic', 1, anything)
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_failure], 'Event::Generic', 1, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to(index_path)
+        expect(flash[:alert]).to include("Organization 'agency.gov' has not yet been configured for Touchpoints")
+      end
+    end
+
+    context 'with existing user migrating from legacy account' do
+      let!(:existing_user) { FactoryBot.create(:user, email: 'existing@example.gov', provider: nil, uid: nil) }
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12352',
+          info: {
+            email: 'existing@example.gov',
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/users'
+      end
+
+      it 'updates the existing user with provider and uid' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_attempt], 'Event::Generic', 1, anything)
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_successful], 'User', anything, anything, anything)
+        
+        get :login_dot_gov
+        
+        expect(response).to redirect_to('/admin/users')
+        existing_user.reload
+        expect(existing_user.provider).to eq('login_dot_gov')
+        expect(existing_user.uid).to eq('12352')
+        expect(controller.current_user).to eq(existing_user)
+      end
+    end
+
+    context 'with existing user already having login.gov credentials' do
+      let!(:existing_user) do
+        FactoryBot.create(:user, email: 'returning@example.gov', provider: 'login_dot_gov', uid: '12353')
+      end
+      let(:auth_hash) do
+        OmniAuth::AuthHash.new(
+          provider: 'login_dot_gov',
+          uid: '12353',
+          info: {
+            email: 'returning@example.gov',
+            email_verified: true
+          }
+        )
+      end
+
+      before do
+        request.env['omniauth.auth'] = auth_hash
+        session['user_return_to'] = '/admin/cx_collections'
+      end
+
+      it 'successfully logs in the existing user without creating a new one' do
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_attempt], 'Event::Generic', 1, anything)
+        expect(Event).to receive(:log_event).with(Event.names[:user_authentication_successful], 'User', existing_user.id, anything, existing_user.id)
+        
+        expect do
+          get :login_dot_gov
+        end.not_to change(User, :count)
+        
+        expect(response).to redirect_to('/admin/cx_collections')
+        expect(controller.current_user).to eq(existing_user)
+      end
+    end
+  end
+end