Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
ca1ebe3
add requested attribute to accounts_show_presenter
jmdembe Jan 2, 2025
aac0d52
add logic for decorated_sp_session
jmdembe Jan 3, 2025
953a404
check for requested_attributes and ial2_requested
jmdembe Jan 3, 2025
1d5090d
add tests for requested attribes and ial2 requested
jmdembe Jan 3, 2025
b166182
changelog: User-Facing Improvements, account management, no change av…
jmdembe Jan 3, 2025
217f82f
add logic to show/hide change button
jmdembe Jan 3, 2025
be2e479
clean up controller and presenter
jmdembe Jan 3, 2025
17effbe
clean up requested_attributes in the rest of controllers and specs
jmdembe Jan 3, 2025
ce0f227
change logic for all emails requested as well as `nil` to `false` in …
jmdembe Jan 3, 2025
a4bba56
remove `ial2_requested`
jmdembe Jan 3, 2025
81952e1
lintfix
jmdembe Jan 6, 2025
cd5d56c
fix logic to show desired behavior when is false
jmdembe Jan 6, 2025
dbb3e5f
remove debugger
jmdembe Jan 6, 2025
23f7f95
fix logic
jmdembe Jan 6, 2025
36d07ac
add supporting test for view
jmdembe Jan 6, 2025
a167fcc
match `all_emails_requested` to be a boolean value
jmdembe Jan 6, 2025
7f33bd9
add logic for if the partner does not have `email` as a consented att…
jmdembe Jan 6, 2025
18be39c
logic change to `all_emails_requested?`
jmdembe Jan 6, 2025
c95159d
refine check for no email (WIP), refine method name
jmdembe Jan 7, 2025
3f19f6a
fix all broken tests
jmdembe Jan 8, 2025
3cf72fa
change service provider -> partner
jmdembe Jan 8, 2025
ad232fb
more logic correction
jmdembe Jan 8, 2025
90d8b65
rename and fix logic
jmdembe Jan 9, 2025
7b6b40b
Update app/controllers/accounts/connected_accounts_controller.rb
jmdembe Jan 10, 2025
999f5f2
change `all_emails requested` -> `change_email_available?`
jmdembe Jan 10, 2025
451a25d
remove `change_email_available`
jmdembe Jan 14, 2025
8c302be
move logic around
jmdembe Jan 15, 2025
7533114
logic in presenter and view
jmdembe Jan 15, 2025
659c264
change to `change_option_available`, fix test
jmdembe Jan 16, 2025
0456265
edit test block
jmdembe Jan 16, 2025
213685a
change name to be more intuitive
jmdembe Jan 16, 2025
4d7ef10
add tests to view
jmdembe Jan 16, 2025
eafb246
fix account show presenter tests
jmdembe Jan 16, 2025
95a970c
method name cleanup
jmdembe Jan 16, 2025
2f0550a
more test fixes
jmdembe Jan 17, 2025
b40b628
more changes
jmdembe Jan 17, 2025
af117e8
fix eager loading problem
jmdembe Jan 17, 2025
7862086
lintfix
jmdembe Jan 17, 2025
355e734
add test
jmdembe Jan 17, 2025
d76872d
test 1 for connected apps
jmdembe Jan 21, 2025
cb41916
check against `identities` and pass value
jmdembe Jan 24, 2025
8fa07e9
place logic in model, remove logic from presenter and controller, add…
jmdembe Jan 24, 2025
778c072
no longer doing logic in controller, so return connected apps to prev…
jmdembe Jan 24, 2025
d958d7d
remove `requested_attributes`
jmdembe Jan 24, 2025
3d06eac
fix logic in model and view
jmdembe Jan 24, 2025
a78d13e
method name change
jmdembe Jan 24, 2025
2221714
remove unneeded keyword
jmdembe Jan 24, 2025
f8724f3
fix tests
jmdembe Jan 24, 2025
311d3e5
Repurpose ServiceProviderIdentity#hide_change_email? as verified_sing…
aduth Jan 27, 2025
6f6a444
Revert changes to ApplicationController, AccountShowPresenter spec
aduth Jan 27, 2025
1774820
Handle nil verified_attributes
aduth Jan 27, 2025
3739d47
Update tests for connected accounts view
aduth Jan 27, 2025
eaccf2a
Exempt eager loading error validation for connected accounts
aduth Jan 27, 2025
979db37
Update stubbed identity in connected accounts feature spec to include…
aduth Jan 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/models/service_provider_identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 18 additions & 12 deletions app/views/accounts/_connected_app.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,24 @@
</h2>

<% 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)),
) %>
<br />
<strong>
<%= identity.email_address&.email || t('account.connected_apps.email_not_selected') %>
</strong>
<%= 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)),
) %>
<strong>
<%= identity.email_address&.email || t('account.connected_apps.email_not_selected') %>
</strong>
<%= 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',
Expand Down
10 changes: 10 additions & 0 deletions config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions spec/features/account_connected_apps_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
42 changes: 42 additions & 0 deletions spec/models/service_provider_identity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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(
Expand Down
37 changes: 35 additions & 2 deletions spec/views/accounts/connected_accounts/show.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down