Get authorization using email selected for sharing if available (#11667)

changelog: Upcoming Features, Partner account, Select email to share with partner

- Refactor: add method ServiceProviderIdentity#email_address_for_sharing that does the right thing
- Add tests to verify correct behavior in SAML and OIDC
- Eliminate EmailContext class that seemed to be of limited value
- Create method EmailAddress#last_sign_in
- Remove code duplication from SAML and OIDC
- Get rid of EmailContext
- Get rid of `reek` remnants
- Fix minor typos

app/controllers/accounts/connected_accounts/selected_email_controller.rb

@@ -53,7 +53,7 @@ def identity
       end
 
       def last_email
-        EmailContext.new(current_user).last_sign_in_email_address.id
+        current_user.last_sign_in_email_address.id
       end
     end
   end

app/controllers/concerns/authorization_count_concern.rb

@@ -3,13 +3,11 @@
 module AuthorizationCountConcern
   extend ActiveSupport::Concern
 
-  # :reek:DuplicateMethodCall
   def auth_count
     session[:sp_auth_count] ||= {}
     session[:sp_auth_count][sp_session[:request_id]]
   end
 
-  # :reek:DuplicateMethodCall
   def auth_count=(value)
     session[:sp_auth_count] ||= {}
     session[:sp_auth_count][sp_session[:request_id]] = value

app/controllers/concerns/mfa_setup_concern.rb

@@ -96,7 +96,7 @@ def threatmetrix_attrs
       user_id: current_user.id,
       request_ip: request&.remote_ip,
       threatmetrix_session_id: user_session[:sign_up_threatmetrix_session_id],
-      email: EmailContext.new(current_user).last_sign_in_email_address.email,
+      email: current_user.last_sign_in_email_address.email,
       uuid_prefix: current_sp&.app_id,
     }
   end

app/controllers/openid_connect/authorization_controller.rb

@@ -93,7 +93,7 @@ def email_address_id
       if user_session[:selected_email_id_for_linked_identity].present?
         return user_session[:selected_email_id_for_linked_identity]
       end
-      identity = current_user.identities.find_by(service_provider: sp_session['issuer'])
+      identity = current_user.identities.find_by(service_provider: sp_session[:issuer])
       identity&.email_address_id
     end
 

app/controllers/sign_up/completions_controller.rb

@@ -22,7 +22,7 @@ def update
       update_verified_attributes
       send_in_person_completion_survey
       if user_session[:selected_email_id_for_linked_identity].nil?
-        user_session[:selected_email_id_for_linked_identity] = EmailContext.new(current_user)
+        user_session[:selected_email_id_for_linked_identity] = current_user
           .last_sign_in_email_address.id
       end
       if decider.go_back_to_mobile_app?

app/controllers/sign_up/select_email_controller.rb

@@ -51,7 +51,7 @@ def last_email
       if user_session[:selected_email_id_for_linked_identity]
         user_emails.find(user_session[:selected_email_id_for_linked_identity]).email
       else
-        EmailContext.new(current_user).last_sign_in_email_address.email
+        current_user.last_sign_in_email_address.email
       end
     end
 

app/controllers/users/authorization_confirmation_controller.rb

@@ -13,7 +13,8 @@ class AuthorizationConfirmationController < ApplicationController
     def new
       analytics.authentication_confirmation
       @sp = ServiceProvider.find_by(issuer: sp_session[:issuer])
-      @email = EmailContext.new(current_user).last_sign_in_email_address.email
+      @email = current_user.active_identity_for(@sp)&.email_address_for_sharing&.email ||
+               current_user.last_sign_in_email_address.email
     end
 
     def create

app/models/email_address.rb

@@ -49,6 +49,10 @@ def mil_email?
     email.end_with?('.mil')
   end
 
+  def self.last_sign_in
+    order('last_sign_in_at DESC NULLS LAST').first
+  end
+
   class << self
     def find_with_email(email)
       return nil if !email.is_a?(String) || email.empty?

app/models/service_provider_identity.rb

@@ -64,4 +64,11 @@ def service_provider_id
   def happened_at
     last_authenticated_at.in_time_zone('UTC')
   end
+
+  def email_address_for_sharing
+    if IdentityConfig.store.feature_select_email_to_share_enabled && email_address
+      return email_address
+    end
+    user.last_sign_in_email_address
+  end
 end

app/models/user.rb

@@ -65,15 +65,15 @@ class User < ApplicationRecord
   attr_accessor :asserted_attributes, :email
 
   def confirmed_email_addresses
-    email_addresses.where.not(confirmed_at: nil).order('last_sign_in_at DESC NULLS LAST')
+    email_addresses.confirmed.order('last_sign_in_at DESC NULLS LAST')
   end
 
   def fully_registered?
     !!registration_log&.registered_at
   end
 
   def confirmed?
-    email_addresses.where.not(confirmed_at: nil).any?
+    confirmed_email_addresses.any?
   end
 
   def has_fed_or_mil_email?
@@ -392,7 +392,7 @@ def qrcode(otp_secret_key)
       interval: IdentityConfig.store.totp_code_interval,
     }
     url = ROTP::TOTP.new(otp_secret_key, options).provisioning_uri(
-      EmailContext.new(self).last_sign_in_email_address.email,
+      last_sign_in_email_address.email,
     )
     qrcode = RQRCode::QRCode.new(url)
     qrcode.as_png(size: 240).to_data_url
@@ -521,6 +521,10 @@ def reload(...)
     super(...)
   end
 
+  def last_sign_in_email_address
+    email_addresses.confirmed.last_sign_in
+  end
+
   private
 
   def lockout_period

app/policies/email_policy.rb

@@ -2,7 +2,7 @@
 
 class EmailPolicy
   def initialize(user)
-    @user = EmailContext.new(user)
+    @user = user
   end
 
   def can_delete_email?(email)
@@ -12,17 +12,17 @@ def can_delete_email?(email)
   end
 
   def can_add_email?
-    user.email_address_count < IdentityConfig.store.max_emails_per_account
+    user.email_addresses.count < IdentityConfig.store.max_emails_per_account
   end
 
   private
 
   def last_confirmed_email_address?
-    user.confirmed_email_address_count <= 1
+    user.confirmed_email_addresses.count <= 1
   end
 
   def last_email_address?
-    user.email_address_count <= 1
+    user.email_addresses.count <= 1
   end
 
   attr_reader :user

app/presenters/account_show_presenter.rb

@@ -120,7 +120,7 @@ def personal_key_generated_at
   def header_personalization
     return decrypted_pii.first_name if decrypted_pii.present?
 
-    EmailContext.new(user).last_sign_in_email_address.email
+    user.last_sign_in_email_address.email
   end
 
   def totp_content

app/presenters/openid_connect_user_info_presenter.rb

@@ -15,7 +15,7 @@ def user_info
     info = {
       sub: uuid_from_sp_identity(identity),
       iss: root_url,
-      email: email_from_sp_identity,
+      email: identity.email_address_for_sharing.email,
       email_verified: true,
     }
 
@@ -53,18 +53,10 @@ def uuid_from_sp_identity(identity)
     AgencyIdentityLinker.new(identity).link_identity.uuid
   end
 
-  def email_from_sp_identity
-    identity.email_address&.email || email_context.last_sign_in_email_address.email
-  end
-
   def all_emails_from_sp_identity(identity)
     identity.user.confirmed_email_addresses.map(&:email)
   end
 
-  def email_context
-    @email_context ||= EmailContext.new(identity.user)
-  end
-
   def ial2_attributes
     {
       given_name: stringify_attr(ial2_data.first_name),

app/services/attribute_asserter.rb

@@ -201,8 +201,7 @@ def attribute_getter_function_ascii(attr)
   def add_email(attrs)
     attrs[:email] = {
       getter: ->(principal) {
-        last_email_from_sp(principal) ||
-          EmailContext.new(principal).last_sign_in_email_address.email
+        principal.active_identity_for(service_provider).email_address_for_sharing.email
       },
       name_format: 'urn:oasis:names:tc:SAML:2.0:attrname-format:basic',
       name_id_format: Saml::XML::Namespaces::Formats::NameId::EMAIL_ADDRESS,
@@ -217,13 +216,6 @@ def add_all_emails(attrs)
     }
   end
 
-  def last_email_from_sp(principal)
-    return nil unless IdentityConfig.store.feature_select_email_to_share_enabled
-    identity = principal.active_identity_for(service_provider)
-    email_id = identity&.email_address_id
-    principal.confirmed_email_addresses.find_by(id: email_id)&.email if email_id
-  end
-
   def bundle
     @bundle ||= (
       authn_request_bundle || service_provider.metadata[:attribute_bundle] || []

app/services/displayable_pii_formatter.rb

@@ -41,7 +41,7 @@ def email
     if @selected_email_id
       current_user.confirmed_email_addresses.find(@selected_email_id).email
     else
-      EmailContext.new(current_user).last_sign_in_email_address.email
+      current_user.last_sign_in_email_address.email
     end
   end
 

config/initializers/saml_idp.rb

@@ -27,7 +27,7 @@
   config.name_id.formats =
     {
       persistent: ->(principal) { principal.asserted_attributes[:uuid][:getter].call(principal) },
-      email_address: ->(principal) { EmailContext.new(principal).last_sign_in_email_address.email },
+      email_address: ->(principal) { principal.last_sign_in_email_address.email },
     }
 
   ## Technical contact ##

spec/controllers/accounts/connected_accounts/selected_email_controller_spec.rb

@@ -74,10 +74,10 @@
       end
     end
 
-    context 'when users has max number of emails' do
+    context 'when the user already has max number of emails' do
       before do
-        allow(user).to receive(:email_address_count).and_return(2)
-        allow(IdentityConfig.store).to receive(:max_emails_per_account).and_return(2)
+        allow(IdentityConfig.store).to receive(:max_emails_per_account)
+          .and_return(user.email_addresses.count)
       end
 
       it 'can add email variable set to false' do

spec/features/openid_connect/authorization_confirmation_spec.rb

@@ -31,36 +31,83 @@ def create_user_and_remember_device
       user1
     end
 
-    shared_examples 'signin email after signing in again' do
+    shared_examples 'signing in with a different email prompts with the shared email' do
       it 'confirms the user wants to continue to SP' do
+        shared_email = user1.identities.first.email_address.email
         second_email = create(:email_address, user: user1)
         sign_in_user(user1, second_email.email)
         visit_idp_from_ial1_oidc_sp
         expect(current_url).to match(user_authorization_confirmation_path)
-        expect(page).to have_content second_email.email
+        expect(page).to have_content shared_email
 
         continue_as(second_email.email)
         expect(oidc_redirect_url).to match('http://localhost:7654/auth/result')
       end
     end
 
-    it_behaves_like 'signin email after signing in again'
+    shared_examples 'signing in with a different email prompts with the signed in email' do
+      it 'confirms the user wants to continue to SP' do
+        second_email = create(:email_address, user: user1)
+        sign_in_user(user1, second_email.email)
+        visit_idp_from_ial1_oidc_sp
+        expect(current_url).to match(user_authorization_confirmation_path)
+        expect(page).to have_content second_email.email
+
+        continue_as(second_email.email)
+        expect(oidc_redirect_url).to match('http://localhost:7654/auth/result')
+      end
+    end
 
-    context 'with client-side redirect' do
+    context 'when email sharing feature is enabled' do
       before do
-        allow(IdentityConfig.store).to receive(:openid_connect_redirect).and_return('client_side')
+        allow(IdentityConfig.store)
+          .to receive(:feature_select_email_to_share_enabled).and_return(true)
       end
 
-      it_behaves_like 'signin email after signing in again'
+      it_behaves_like 'signing in with a different email prompts with the shared email'
+
+      context 'with client-side redirect' do
+        before do
+          allow(IdentityConfig.store).to receive(:openid_connect_redirect).and_return('client_side')
+        end
+
+        it_behaves_like 'signing in with a different email prompts with the shared email'
+      end
+
+      context 'with client-side javascript redirect' do
+        before do
+          allow(IdentityConfig.store).to receive(:openid_connect_redirect)
+            .and_return('client_side_js')
+        end
+
+        it_behaves_like 'signing in with a different email prompts with the shared email'
+      end
     end
 
-    context 'with client-side javascript redirect' do
+    context 'when email sharing feature is disabled' do
       before do
-        allow(IdentityConfig.store).to receive(:openid_connect_redirect)
-          .and_return('client_side_js')
+        allow(IdentityConfig.store)
+          .to receive(:feature_select_email_to_share_enabled).and_return(false)
       end
 
-      it_behaves_like 'signin email after signing in again'
+      it_behaves_like 'signing in with a different email prompts with the signed in email'
+
+      context 'with client-side redirect' do
+        before do
+          allow(IdentityConfig.store).to receive(:openid_connect_redirect).and_return('client_side')
+        end
+
+        it_behaves_like 'signing in with a different email prompts with the signed in email'
+      end
+
+      context 'with client-side javascript redirect' do
+        before do
+          allow(IdentityConfig.store).to receive(:openid_connect_redirect)
+            .and_return('client_side_js')
+        end
+
+        it_behaves_like 'signing in with a different email prompts with the signed in email'
+      end
     end
 
     it 'it allows the user to switch accounts prior to continuing to the SP' do