Avoid duplicated ServiceProvider queries in IdentityLinker

app/controllers/concerns/saml_idp_auth_concern.rb

@@ -24,15 +24,15 @@ def sign_out_if_forceauthn_is_true_and_user_is_signed_in
   end
 
   def check_sp_active
-    return if current_service_provider&.active?
+    return if saml_request_service_provider&.active?
     redirect_to sp_inactive_error_url
   end
 
   def validate_service_provider_and_authn_context
     @saml_request_validator = SamlRequestValidator.new
 
     @result = @saml_request_validator.call(
-      service_provider: current_service_provider,
+      service_provider: saml_request_service_provider,
       authn_context: requested_authn_contexts,
       authn_context_comparison: saml_request.requested_authn_context_comparison,
       nameid_format: name_id_format,
@@ -49,7 +49,7 @@ def name_id_format
   end
 
   def specified_name_id_format
-    if recognized_name_id_format? || current_service_provider&.use_legacy_name_id_behavior
+    if recognized_name_id_format? || saml_request_service_provider&.use_legacy_name_id_behavior
       saml_request.name_id_format
     end
   end
@@ -59,7 +59,7 @@ def recognized_name_id_format?
   end
 
   def default_name_id_format
-    if current_service_provider&.email_nameid_format_allowed
+    if saml_request_service_provider&.email_nameid_format_allowed
       return Saml::Idp::Constants::NAME_ID_FORMAT_EMAIL
     end
     Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT
@@ -80,16 +80,16 @@ def requested_authn_contexts
   end
 
   def default_aal_context
-    if current_service_provider&.default_aal
-      Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[current_service_provider.default_aal]
+    if saml_request_service_provider&.default_aal
+      Saml::Idp::Constants::AUTHN_CONTEXT_AAL_TO_CLASSREF[saml_request_service_provider.default_aal]
     else
       Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF
     end
   end
 
   def default_ial_context
-    if current_service_provider&.ial
-      Saml::Idp::Constants::AUTHN_CONTEXT_IAL_TO_CLASSREF[current_service_provider.ial]
+    if saml_request_service_provider&.ial
+      Saml::Idp::Constants::AUTHN_CONTEXT_IAL_TO_CLASSREF[saml_request_service_provider.ial]
     else
       Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF
     end
@@ -105,7 +105,7 @@ def requested_ial_authn_context
 
   def link_identity_from_session_data
     IdentityLinker.
-      new(current_user, current_issuer).
+      new(current_user, saml_request_service_provider).
       link_identity(
         ial: ial_context.ial_for_identity_record,
         rails_session_id: session.id,
@@ -121,7 +121,7 @@ def identity_needs_verification?
   def ial_context
     @ial_context ||= IalContext.new(
       ial: requested_ial_authn_context,
-      service_provider: current_service_provider,
+      service_provider: saml_request_service_provider,
     )
   end
 
@@ -137,7 +137,7 @@ def encode_authn_response(principal, opts)
   def attribute_asserter(principal)
     AttributeAsserter.new(
       user: principal,
-      service_provider: current_service_provider,
+      service_provider: saml_request_service_provider,
       name_id_format: name_id_format,
       authn_request: saml_request,
       decrypted_pii: decrypted_pii,
@@ -163,20 +163,21 @@ def saml_response
       reference_id: active_identity.session_uuid,
       encryption: encryption_opts,
       signature: saml_response_signature_options,
-      signed_response_message: current_service_provider&.signed_response_message_requested,
+      signed_response_message: saml_request_service_provider&.signed_response_message_requested,
     )
   end
 
   def encryption_opts
     query_params = UriService.params(request.original_url)
-    if query_params[:skip_encryption].present? && current_service_provider&.skip_encryption_allowed
+    if query_params[:skip_encryption].present? &&
+       saml_request_service_provider&.skip_encryption_allowed
       nil
-    elsif current_service_provider&.encrypt_responses?
+    elsif saml_request_service_provider&.encrypt_responses?
       cert = saml_request.service_provider.matching_cert ||
-             current_service_provider&.ssl_certs&.first
+             saml_request_service_provider&.ssl_certs&.first
       {
         cert: cert,
-        block_encryption: current_service_provider&.block_encryption,
+        block_encryption: saml_request_service_provider&.block_encryption,
         key_transport: 'rsa-oaep-mgf1p',
       }
     end
@@ -190,9 +191,9 @@ def saml_response_signature_options
     }
   end
 
-  def current_service_provider
-    return @current_service_provider if defined?(@current_service_provider)
-    @current_service_provider = ServiceProvider.find_by(issuer: current_issuer)
+  def saml_request_service_provider
+    return @saml_request_service_provider if defined?(@saml_request_service_provider)
+    @saml_request_service_provider = ServiceProvider.find_by(issuer: current_issuer)
   end
 
   def current_issuer

app/controllers/concerns/verify_sp_attributes_concern.rb

@@ -16,7 +16,7 @@ def needs_completion_screen_reason
   def update_verified_attributes
     IdentityLinker.new(
       current_user,
-      sp_session[:issuer],
+      current_sp,
     ).link_identity(
       ial: sp_session_ial,
       verified_attributes: sp_session[:requested_attributes],

app/forms/openid_connect_authorize_form.rb

@@ -70,7 +70,7 @@ def service_provider
   end
 
   def link_identity_to_service_provider(current_user, rails_session_id)
-    identity_linker = IdentityLinker.new(current_user, client_id)
+    identity_linker = IdentityLinker.new(current_user, service_provider)
     @identity = identity_linker.link_identity(
       nonce: nonce,
       rails_session_id: rails_session_id,

app/services/identity_linker.rb

@@ -1,14 +1,14 @@
 class IdentityLinker
-  attr_reader :user, :issuer
+  attr_reader :user, :service_provider
 
-  def initialize(user, issuer)
+  def initialize(user, service_provider)
     @user = user
-    @issuer = issuer
+    @service_provider = service_provider
     @ial = nil
   end
 
   def link_identity(**extra_attrs)
-    return unless user && issuer.present?
+    return unless user && service_provider.present?
     process_ial(extra_attrs)
     attributes = merged_attributes(extra_attrs)
     identity.update!(attributes)
@@ -46,11 +46,11 @@ def find_or_create_identity_with_costing
     identity_record = identity_relation.first
     return identity_record if identity_record
     Db::SpCost::AddSpCost.call(service_provider, @ial, :user_added)
-    user.identities.create(service_provider: issuer)
+    user.identities.create(service_provider: service_provider.issuer)
   end
 
   def identity_relation
-    user.identities.where(service_provider: issuer)
+    user.identities.where(service_provider: service_provider.issuer)
   end
 
   def merged_attributes(extra_attrs)
@@ -92,9 +92,4 @@ def merge_attributes(verified_attributes)
     verified_attributes = verified_attributes.to_a.map(&:to_s)
     (identity.verified_attributes.to_a + verified_attributes).uniq.sort
   end
-
-  def service_provider
-    return if issuer.blank?
-    @service_provider ||= ServiceProvider.find_by(issuer: issuer)
-  end
 end

spec/controllers/openid_connect/authorization_controller_spec.rb

@@ -10,6 +10,7 @@
   end
 
   let(:client_id) { 'urn:gov:gsa:openidconnect:test' }
+  let(:service_provider) { build(:service_provider, issuer: client_id) }
   let(:params) do
     {
       acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF,
@@ -34,7 +35,7 @@
 
       context 'with valid params' do
         it 'redirects back to the client app with a code' do
-          IdentityLinker.new(user, client_id).link_identity(ial: 1)
+          IdentityLinker.new(user, service_provider).link_identity(ial: 1)
           user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate])
           action
 
@@ -61,7 +62,7 @@
             with(Analytics::SP_REDIRECT_INITIATED,
                  ial: 1)
 
-          IdentityLinker.new(user, client_id).link_identity(ial: 1)
+          IdentityLinker.new(user, service_provider).link_identity(ial: 1)
           user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate])
 
           action
@@ -77,7 +78,7 @@
             let(:user) { create(:profile, :active, :verified).user }
 
             it 'redirects to the redirect_uri immediately when pii is unlocked' do
-              IdentityLinker.new(user, client_id).link_identity(ial: 3)
+              IdentityLinker.new(user, service_provider).link_identity(ial: 3)
               user.identities.last.update!(
                 verified_attributes: %w[given_name family_name birthdate verified_at],
               )
@@ -88,7 +89,7 @@
             end
 
             it 'redirects to the password capture url when pii is locked' do
-              IdentityLinker.new(user, client_id).link_identity(ial: 3)
+              IdentityLinker.new(user, service_provider).link_identity(ial: 3)
               user.identities.last.update!(
                 verified_attributes: %w[given_name family_name birthdate verified_at],
               )
@@ -113,7 +114,7 @@
                 with(Analytics::SP_REDIRECT_INITIATED,
                      ial: 2)
 
-              IdentityLinker.new(user, client_id).link_identity(ial: 2)
+              IdentityLinker.new(user, service_provider).link_identity(ial: 2)
               user.identities.last.update!(
                 verified_attributes: %w[given_name family_name birthdate verified_at],
               )
@@ -131,7 +132,7 @@
               end
 
               it 'creates an IAL2 SpReturnLog record' do
-                IdentityLinker.new(user, client_id).link_identity(ial: 22)
+                IdentityLinker.new(user, service_provider).link_identity(ial: 22)
                 user.identities.last.update!(
                   verified_attributes: %w[given_name family_name birthdate verified_at],
                 )
@@ -176,7 +177,7 @@
 
         context 'user has already approved this application' do
           before do
-            IdentityLinker.new(user, client_id).link_identity
+            IdentityLinker.new(user, service_provider).link_identity
             user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate])
           end
 

spec/controllers/openid_connect/token_controller_spec.rb

@@ -18,6 +18,7 @@
     let(:grant_type) { 'authorization_code' }
     let(:code) { identity.session_uuid }
     let(:client_id) { 'urn:gov:gsa:openidconnect:test' }
+    let(:service_provider) { build(:service_provider, issuer: client_id) }
     let(:client_assertion) do
       jwt_payload = {
         iss: client_id,
@@ -33,7 +34,10 @@
     end
 
     let!(:identity) do
-      IdentityLinker.new(user, client_id).link_identity(rails_session_id: SecureRandom.hex, ial: 1)
+      IdentityLinker.new(user, service_provider).link_identity(
+        rails_session_id: SecureRandom.hex,
+        ial: 1,
+      )
     end
 
     context 'with valid params' do

spec/controllers/risc/security_events_controller_spec.rb

@@ -4,7 +4,7 @@
   include Rails.application.routes.url_helpers
 
   let(:user) { create(:user) }
-  let(:identity) { IdentityLinker.new(user, service_provider.issuer).link_identity }
+  let(:identity) { IdentityLinker.new(user, service_provider).link_identity }
   let(:service_provider) { create(:service_provider) }
 
   let(:rp_private_key) do

spec/controllers/saml_idp_controller_spec.rb

@@ -476,7 +476,7 @@ def name_id_version(format_urn)
 
       before do
         stub_sign_in(user)
-        IdentityLinker.new(user, sp1_issuer).link_identity(ial: 2)
+        IdentityLinker.new(user, sp1).link_identity(ial: 2)
         user.identities.last.update!(
           verified_attributes: %w[given_name family_name social_security_number address],
         )
@@ -892,7 +892,8 @@ def name_id_version(format_urn)
         end
 
         it 'does not redirect after verifying attributes' do
-          IdentityLinker.new(@user, saml_settings.issuer).link_identity(
+          service_provider = build(:service_provider, issuer: saml_settings.issuer)
+          IdentityLinker.new(@user, service_provider).link_identity(
             verified_attributes: ['email'],
           )
           saml_get_auth(saml_settings)
@@ -968,7 +969,8 @@ def name_id_version(format_urn)
             Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF,
           ] },
         )
-        IdentityLinker.new(user, auth_settings.issuer).link_identity
+        service_provider = build(:service_provider, issuer: auth_settings.issuer)
+        IdentityLinker.new(user, service_provider).link_identity
         user.identities.last.update!(verified_attributes: ['email'])
         generate_saml_response(user, auth_settings)
 
@@ -1002,7 +1004,8 @@ def name_id_version(format_urn)
 
       it 'defaults to persistent' do
         auth_settings = saml_settings(overrides: { name_identifier_format: nil })
-        IdentityLinker.new(user, auth_settings.issuer).link_identity
+        service_provider = build(:service_provider, issuer: auth_settings.issuer)
+        IdentityLinker.new(user, service_provider).link_identity
         user.identities.last.update!(verified_attributes: ['email'])
         generate_saml_response(user, auth_settings)
 
@@ -1035,7 +1038,7 @@ def name_id_version(format_urn)
         ServiceProvider.
           find_by(issuer: auth_settings.issuer).
           update!(email_nameid_format_allowed: true)
-        IdentityLinker.new(user, auth_settings.issuer).link_identity
+        IdentityLinker.new(user, sp1).link_identity
         user.identities.last.update!(verified_attributes: ['email'])
         generate_saml_response(user, auth_settings)
 
@@ -1103,7 +1106,8 @@ def name_id_version(format_urn)
         auth_settings = saml_settings(overrides: { name_identifier_format: nil })
         auth_settings.name_identifier_format =
           'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified'
-        IdentityLinker.new(user, auth_settings.issuer).link_identity
+        service_provider = build(:service_provider, issuer: auth_settings.issuer)
+        IdentityLinker.new(user, service_provider).link_identity
         user.identities.last.update!(verified_attributes: ['email'])
         generate_saml_response(user, auth_settings)
 
@@ -1126,14 +1130,14 @@ def name_id_version(format_urn)
         expect(@analytics).to have_received(:track_event).
           with(Analytics::SAML_AUTH, analytics_hash)
       end
+
       it 'sends the appropriate identifier for email NameID SPs' do
         auth_settings = saml_settings(overrides: { name_identifier_format: nil })
         auth_settings.name_identifier_format =
           'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified'
-        ServiceProvider.
-          find_by(issuer: auth_settings.issuer).
-          update!(email_nameid_format_allowed: true)
-        IdentityLinker.new(user, auth_settings.issuer).link_identity
+        service_provider = ServiceProvider.find_by(issuer: auth_settings.issuer)
+        service_provider.update!(email_nameid_format_allowed: true)
+        IdentityLinker.new(user, service_provider).link_identity
         user.identities.last.update!(verified_attributes: ['email'])
         generate_saml_response(user, auth_settings)
 
@@ -1156,14 +1160,14 @@ def name_id_version(format_urn)
         expect(@analytics).to have_received(:track_event).
           with(Analytics::SAML_AUTH, analytics_hash)
       end
+
       it 'sends the old user ID for legacy SPS' do
         auth_settings = saml_settings(overrides: { name_identifier_format: nil })
         auth_settings.name_identifier_format =
           'urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified'
-        ServiceProvider.
-          find_by(issuer: auth_settings.issuer).
-          update!(use_legacy_name_id_behavior: true)
-        IdentityLinker.new(user, auth_settings.issuer).link_identity
+        service_provider = ServiceProvider.find_by(issuer: auth_settings.issuer)
+        service_provider.update!(use_legacy_name_id_behavior: true)
+        IdentityLinker.new(user, service_provider).link_identity
         user.identities.last.update!(verified_attributes: ['email'])
         generate_saml_response(user, auth_settings)
 

spec/controllers/users/service_provider_revoke_controller_spec.rb

@@ -18,7 +18,7 @@
   before do
     stub_sign_in(user)
 
-    @identity = IdentityLinker.new(user, service_provider.issuer).link_identity
+    @identity = IdentityLinker.new(user, service_provider).link_identity
   end
 
   describe '#show' do

spec/features/account_reset/delete_account_spec.rb

@@ -62,7 +62,8 @@
     end
 
     it 'sends push notifications if push_notifications_enabled is true' do
-      identity = IdentityLinker.new(user, 'urn:gov:gsa:openidconnect:test').link_identity
+      service_provider = build(:service_provider, issuer: 'urn:gov:gsa:openidconnect:test')
+      identity = IdentityLinker.new(user, service_provider).link_identity
       agency_identity = AgencyIdentityLinker.new(identity).link_identity
 
       signin(user_email, user.password)