diff --git a/app/models/service_provider_identity.rb b/app/models/service_provider_identity.rb index 5ad3eaf6ede..3da98d4eb9f 100644 --- a/app/models/service_provider_identity.rb +++ b/app/models/service_provider_identity.rb @@ -65,6 +65,12 @@ def happened_at last_authenticated_at.in_time_zone('UTC') end + def verified_single_email_attribute? + verified_attributes.present? && + verified_attributes.include?('email') && + !verified_attributes.include?('all_emails') + end + def email_address_for_sharing if IdentityConfig.store.feature_select_email_to_share_enabled && email_address return email_address diff --git a/app/views/accounts/_connected_app.html.erb b/app/views/accounts/_connected_app.html.erb index 63b229fea39..615fbae0409 100644 --- a/app/views/accounts/_connected_app.html.erb +++ b/app/views/accounts/_connected_app.html.erb @@ -12,18 +12,24 @@ <% if IdentityConfig.store.feature_select_email_to_share_enabled %> - <%= t( - 'account.connected_apps.associated_attributes_html', - timestamp_html: render(TimeComponent.new(time: identity.created_at)), - ) %> -
- - <%= identity.email_address&.email || t('account.connected_apps.email_not_selected') %> - - <%= link_to( - t('help_text.requested_attributes.change_email_link'), - edit_connected_account_selected_email_path(identity_id: identity.id), - ) %> + <% if identity.verified_single_email_attribute? %> + <%= t( + 'account.connected_apps.associated_attributes_html', + timestamp_html: render(TimeComponent.new(time: identity.created_at)), + ) %> + + <%= identity.email_address&.email || t('account.connected_apps.email_not_selected') %> + + <%= link_to( + t('help_text.requested_attributes.change_email_link'), + edit_connected_account_selected_email_path(identity_id: identity.id), + ) %> + <% else %> + <%= t( + 'account.connected_apps.associated_html', + timestamp_html: render(TimeComponent.new(time: identity.created_at)), + ) %> + <% end %> <% else %> <%= t( 'account.connected_apps.associated_html', diff --git a/config/environments/test.rb b/config/environments/test.rb index 2abaa4644e9..b85d3bc7c1d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -52,6 +52,16 @@ ].each do |association| Bullet.add_safelist(type: :n_plus_one_query, class_name: 'User', association: association) end + + # Eager loading of email addresses is used on the Connected Accounts page, since most accounts + # will share an email address that can be changed by the user. An unoptimized query error is + # raised by bullet if the email address is not used, but it can't be known at the time of the + # query whether the email addresses will be used for all connected accounts. + Bullet.add_safelist( + type: :unused_eager_loading, + class_name: 'ServiceProviderIdentity', + association: :email_address, + ) end config.active_support.test_order = :random diff --git a/spec/features/account_connected_apps_spec.rb b/spec/features/account_connected_apps_spec.rb index 9ed731123a6..c37ad9cd25f 100644 --- a/spec/features/account_connected_apps_spec.rb +++ b/spec/features/account_connected_apps_spec.rb @@ -18,6 +18,7 @@ user: user, created_at: Time.zone.now - 80.days, service_provider: 'http://localhost:3000', + verified_attributes: ['email'], ) end let(:identity_without_link) do @@ -27,6 +28,7 @@ user: user, created_at: Time.zone.now - 50.days, service_provider: 'https://rp2.serviceprovider.com/auth/saml/metadata', + verified_attributes: ['email'], ) end let(:identity_timestamp) do diff --git a/spec/models/service_provider_identity_spec.rb b/spec/models/service_provider_identity_spec.rb index 54015505d9d..f188c84368f 100644 --- a/spec/models/service_provider_identity_spec.rb +++ b/spec/models/service_provider_identity_spec.rb @@ -2,10 +2,12 @@ RSpec.describe ServiceProviderIdentity do let(:user) { create(:user, :fully_registered) } + let(:verified_attributes) { [] } let(:identity) do ServiceProviderIdentity.create( user_id: user.id, service_provider: 'externalapp', + verified_attributes:, ) end subject { identity } @@ -182,6 +184,46 @@ end end + describe '#verified_single_email_attribute?' do + subject(:verified_single_email_attribute?) { identity.verified_single_email_attribute? } + + context 'with attributes nil' do + let(:verified_attributes) { nil } + + it { is_expected.to be false } + end + + context 'with no attributes verified' do + let(:verified_attributes) { [] } + + it { is_expected.to be false } + end + + context 'with a non-email attribute verified' do + let(:verified_attributes) { ['openid'] } + + it { is_expected.to be false } + end + + context 'with all_emails attribute verified' do + let(:verified_attributes) { ['all_emails'] } + + it { is_expected.to be false } + end + + context 'with email attribute verified' do + let(:verified_attributes) { ['email'] } + + it { is_expected.to be true } + + context 'with all_emails attribute verified' do + let(:verified_attributes) { ['email', 'all_emails'] } + + it { is_expected.to be false } + end + end + end + describe '#email_address_for_sharing' do let!(:last_login_email_address) do create( diff --git a/spec/views/accounts/connected_accounts/show.html.erb_spec.rb b/spec/views/accounts/connected_accounts/show.html.erb_spec.rb index 9a18c8cbb00..eed3b16bb81 100644 --- a/spec/views/accounts/connected_accounts/show.html.erb_spec.rb +++ b/spec/views/accounts/connected_accounts/show.html.erb_spec.rb @@ -31,7 +31,7 @@ end context 'with a connected app' do - let!(:identity) { create(:service_provider_identity, user:) } + let!(:identity) { create(:service_provider_identity, user:, verified_attributes: ['email']) } it 'lists applications with link to revoke' do render @@ -68,10 +68,43 @@ end end + context 'when the partner requests all_emails' do + before { identity.update(verified_attributes: ['all_emails']) } + + it 'does not show the change link' do + render + + expect(rendered).not_to have_content(t('account.connected_apps.email_not_selected')) + expect(rendered).not_to have_link( + t('help_text.requested_attributes.change_email_link'), + href: edit_connected_account_selected_email_path(identity_id: identity.id), + ) + end + end + + context 'when the partner does not request email' do + before { identity.update(verified_attributes: ['ssn']) } + + it 'hides the change link' do + render + + expect(rendered).not_to have_content(t('account.connected_apps.email_not_selected')) + expect(rendered).to_not have_link( + t('help_text.requested_attributes.change_email_link'), + href: edit_connected_account_selected_email_path(identity_id: identity.id), + ) + end + end + context 'with connected app having linked email' do let(:email_address) { user.confirmed_email_addresses.take } let!(:identity) do - create(:service_provider_identity, user:, email_address_id: email_address.id) + create( + :service_provider_identity, + user:, + email_address_id: email_address.id, + verified_attributes: ['email'], + ) end it 'renders associated email with option to change' do