Skip to content

Commit

Permalink
LG-13001 remove nullify from EmailAddress model :identities (#11172)
Browse files Browse the repository at this point in the history
* changelog: Upcoming Features, Partner account, Select email to share with partner

* add select email form, build out show and create actions in controller

* add feature flag

* add additional email if have only 1

* add styles to make display match design better

* update translations

* fix typo in zh

* updated completion show spec

* add spec for select_email/show view

* add spec for select email controller

* fix duplicate key entry

* fix missing key

* adds email_address_id column to Identities table to track selected email address for sp

* add SAML, rename sp_email session key, improve syntax

* add checking for selected sp email to authentication on existing account

* fix lint errors and bug when adding sp for the first time

* add in missing email_address_id

* include expectation of email_address_id value

* fix app_name error in yml

* mising translated app_name

* add missing token to view

* fix test assertion

* clean up text syntax

* revise test assertion

* be more explicit about a fallback email to get the login id

* add sign_in test where an alternate email is selected

* add SAML example for switching email

* authorization_controller update session syntax

* add feature flags

* remove stray comment

* rename form to build_

* added association to identity model and updated methods that use email_address_id

* refactor email_address_id method

* make sure listed emails are confirmed

* adds validation if email belongs to user

* utilize StatusPageComponent

* add spec to check email ownership

* fix broken merge item and lint error

* remove unneeded block markup

* bind the multiple emails conditional in an elsif

* leverage id instead of email string, clarify how form is validating success

* add test at form class to confirm validation, fix id-based test changes and bug introduced at mutliple_emails?

* reduce conditional return and fix html formatting

* make email_id naming clearer

* created separate translation for Email not found error text

* remove lint faux pas

* remove updating last-sign in upon form submit. rename some variables for consistency.

* improve identity querying

* check for email id value before trying to find an email address

* Add specs for expected behavior after deleting email

* markup changes to form page

* verify completion at select email cont. account for deleting email that relates to identity, remove no longer needed method in session controller

* add needs completion state to controller

* change content class to grid-col-fill

* add test to attribute assertion, show selected email at completions view

* add a logout and explicit login instructions

* test change masks problem, reverted

* account for deleted or missing email by id

* add test for deleted email

* update tests to include selected_email_id

* add test to account for deleted email

* clean up queries add signup before_action test, fix typo, add :nullify

* fix variable availability

* optimize queries to reduce db load, fixes due to uncovered test failures from last commit, add test coverage for nil email id

* take out rescue condition

* re-add rescue cases for last_email methods

* revise conditional to detect multiple confirmed email addresses

* remove unneeded function

* add markup to fix button width

* remove unnecessary db call at id link

* fix some queries, change session to user_session

* reset test and select if multiple emails

* revise completion show tests

* add test cases where select email feature is disabled

* correct lint errors

* remove unused method and correct for consistent nil-safe

* fix email-identity association

* email_address_id set at consent but if nil does not change for already consented user

* remove commented

* change test

* remove required email_attribute_id from expectations

* ensure selected email is passed to SP

* rebase and normalize yaml

* remove old migration

* replace direct DB call with relationship

* changelog: Bug Fixes, Partner accounts, Fixes deletion timeouts caused by original LG-13001 PR

* place dependent: nil to satisfy rubocop

* delete from user identities email_address_id when email is destoryed

* make rubocop happy

* move email_address_id nil function to destroy callback

* move private block after self

* rename callback, remove nil dependent

* move destroy email tests to model spec

---------

Co-authored-by: Andrew Duthie <[email protected]>
  • Loading branch information
kevinsmaster5 and aduth authored Sep 4, 2024
1 parent 6e8b29e commit 9383fd8
Show file tree
Hide file tree
Showing 34 changed files with 739 additions and 24 deletions.
2 changes: 1 addition & 1 deletion app/components/icon_list_item_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
<%= content_tag(:div, class: icon_css_class) do %>
<%= render IconComponent.new(icon: icon) %>
<% end %>
<div class="usa-icon-list__content"><%= content %></div>
<div class="usa-icon-list__content grid-col-fill"><%= content %></div>
<% end %>
9 changes: 9 additions & 0 deletions app/controllers/concerns/saml_idp_auth_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,18 @@ def link_identity_from_session_data
link_identity(
ial: resolved_authn_context_int_ial,
rails_session_id: session.id,
email_address_id: email_address_id,
)
end

def email_address_id
return nil unless IdentityConfig.store.feature_select_email_to_share_enabled
return user_session[:selected_email_id] if user_session[:selected_email_id].present?
identity = current_user.identities.find_by(service_provider: sp_session['issuer'])
email_id = identity&.email_address_id
return email_id if email_id.is_a? Integer
end

def identity_needs_verification?
resolved_authn_context_result.identity_proofing? && current_user.identity_not_verified?
end
Expand Down
8 changes: 8 additions & 0 deletions app/controllers/openid_connect/authorization_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,17 @@ def link_identity_to_service_provider
current_user: current_user,
ial: resolved_authn_context_int_ial,
rails_session_id: session.id,
email_address_id: email_address_id,
)
end

def email_address_id
return nil unless IdentityConfig.store.feature_select_email_to_share_enabled
return user_session[:selected_email_id] if user_session[:selected_email_id].present?
identity = current_user.identities.find_by(service_provider: sp_session['issuer'])
identity&.email_address_id
end

def ial_context
IalContext.new(
ial: resolved_authn_context_int_ial,
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/sign_up/completions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ def update
track_completion_event('agency-page')
update_verified_attributes
send_in_person_completion_survey
if user_session[:selected_email_id].nil?
user_session[:selected_email_id] = EmailContext.new(current_user).
last_sign_in_email_address.id
end
if decider.go_back_to_mobile_app?
sign_user_out_and_instruct_to_go_back_to_mobile_app
else
Expand Down Expand Up @@ -49,6 +53,7 @@ def completions_presenter
requested_attributes: decorated_sp_session.requested_attributes.map(&:to_sym),
ial2_requested: ial2_requested?,
completion_context: needs_completion_screen_reason,
selected_email_id: user_session[:selected_email_id],
)
end

Expand Down
54 changes: 54 additions & 0 deletions app/controllers/sign_up/select_email_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# frozen_string_literal: true

module SignUp
class SelectEmailController < ApplicationController
before_action :confirm_two_factor_authenticated
before_action :verify_needs_completions_screen

def show
@sp_name = current_sp.friendly_name || sp.agency&.name
@user_emails = user_emails
@last_sign_in_email_address = last_email
@select_email_form = build_select_email_form
end

def create
@select_email_form = build_select_email_form

result = @select_email_form.submit(form_params)
if result.success?
user_session[:selected_email_id] = form_params[:selected_email_id]
redirect_to sign_up_completed_path
else
flash[:error] = result.first_error_message
redirect_to sign_up_select_email_path
end
end

def user_emails
@user_emails = current_user.confirmed_email_addresses
end

private

def build_select_email_form
SelectEmailForm.new(current_user)
end

def form_params
params.fetch(:select_email_form, {}).permit(:selected_email_id)
end

def last_email
if user_session[:selected_email_id]
user_emails.find(user_session[:selected_email_id]).email
else
EmailContext.new(current_user).last_sign_in_email_address.email
end
end

def verify_needs_completions_screen
redirect_to account_url unless needs_completion_screen_reason
end
end
end
4 changes: 3 additions & 1 deletion app/forms/openid_connect_authorize_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ def service_provider
def link_identity_to_service_provider(
current_user:,
ial:,
rails_session_id:
rails_session_id:,
email_address_id:
)
identity_linker = IdentityLinker.new(current_user, service_provider)
@identity = identity_linker.link_identity(
Expand All @@ -106,6 +107,7 @@ def link_identity_to_service_provider(
requested_aal_value: requested_aal_value,
scope: scope.join(' '),
code_challenge: code_challenge,
email_address_id: email_address_id,
)
end

Expand Down
31 changes: 31 additions & 0 deletions app/forms/select_email_form.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# frozen_string_literal: true

class SelectEmailForm
include ActiveModel::Model
include ActionView::Helpers::TranslationHelper

attr_reader :user, :selected_email_id

validate :validate_owns_selected_email

def initialize(user)
@user = user
end

def submit(params)
@selected_email_id = params[:selected_email_id]

success = valid?
FormResponse.new(success:, errors:)
end

private

def validate_owns_selected_email
return if user.confirmed_email_addresses.exists?(id: selected_email_id)

errors.add :email, I18n.t(
'email_address.not_found',
), type: :selected_email_id
end
end
17 changes: 17 additions & 0 deletions app/models/email_address.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@
class EmailAddress < ApplicationRecord
include EncryptableAttribute

before_destroy :reset_linked_identities

encrypted_attribute_without_setter(name: :email)

belongs_to :user, inverse_of: :email_addresses
validates :encrypted_email, presence: true
validates :email_fingerprint, presence: true
# rubocop:disable Rails/HasManyOrHasOneDependent
has_one :suspended_email

has_many :identities, class_name: 'ServiceProviderIdentity'
# rubocop:enable Rails/HasManyOrHasOneDependent

scope :confirmed, -> { where('confirmed_at IS NOT NULL') }
Expand Down Expand Up @@ -90,4 +94,17 @@ def create_fingerprints(email)
[Pii::Fingerprinter.fingerprint(email), *Pii::Fingerprinter.previous_fingerprints(email)]
end
end

private

# Remove email id from all user identities
# when the email is destroyed.
def reset_linked_identities
# rubocop:disable Rails/SkipsModelValidations
ServiceProviderIdentity.where(
user_id: user_id,
email_address_id: id,
).update_all(email_address_id: nil)
# rubocop:enable Rails/SkipsModelValidations
end
end
2 changes: 2 additions & 0 deletions app/models/service_provider_identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class ServiceProviderIdentity < ApplicationRecord
# rubocop:enable Rails/InverseOf
has_one :agency, through: :service_provider_record

belongs_to :email_address

scope :not_deleted, -> { where(deleted_at: nil) }

CONSENT_EXPIRATION = 1.year.freeze
Expand Down
12 changes: 10 additions & 2 deletions app/presenters/completions_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ class CompletionsPresenter
include ActionView::Helpers::TranslationHelper
include ActionView::Helpers::TagHelper

attr_reader :current_user, :current_sp, :decrypted_pii, :requested_attributes, :completion_context
attr_reader :current_user, :current_sp, :decrypted_pii, :requested_attributes,
:completion_context, :selected_email_id

SORTED_IAL2_ATTRIBUTE_MAPPING = [
[[:email], :email],
Expand Down Expand Up @@ -33,14 +34,16 @@ def initialize(
decrypted_pii:,
requested_attributes:,
ial2_requested:,
completion_context:
completion_context:,
selected_email_id:
)
@current_user = current_user
@current_sp = current_sp
@decrypted_pii = decrypted_pii
@requested_attributes = requested_attributes
@ial2_requested = ial2_requested
@completion_context = completion_context
@selected_email_id = selected_email_id
end

def ial2_requested?
Expand Down Expand Up @@ -102,6 +105,10 @@ def pii
end
end

def multiple_emails?
current_user.confirmed_email_addresses.many?
end

private

def first_time_signing_in?
Expand All @@ -112,6 +119,7 @@ def displayable_pii
@displayable_pii ||= DisplayablePiiFormatter.new(
current_user: current_user,
pii: decrypted_pii,
selected_email_id: @selected_email_id,
).format
end

Expand Down
2 changes: 1 addition & 1 deletion app/presenters/openid_connect_user_info_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def uuid_from_sp_identity(identity)
end

def email_from_sp_identity
email_context.last_sign_in_email_address.email
identity.email_address&.email || email_context.last_sign_in_email_address.email
end

def all_emails_from_sp_identity(identity)
Expand Down
12 changes: 11 additions & 1 deletion app/services/attribute_asserter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,10 @@ def attribute_getter_function_ascii(attr)

def add_email(attrs)
attrs[:email] = {
getter: ->(principal) { EmailContext.new(principal).last_sign_in_email_address.email },
getter: ->(principal) {
last_email_from_sp(principal) ||
EmailContext.new(principal).last_sign_in_email_address.email
},
name_format: 'urn:oasis:names:tc:SAML:2.0:attrname-format:basic',
name_id_format: Saml::XML::Namespaces::Formats::NameId::EMAIL_ADDRESS,
}
Expand All @@ -214,6 +217,13 @@ 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] || []
Expand Down
10 changes: 8 additions & 2 deletions app/services/displayable_pii_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ class DisplayablePiiFormatter

attr_reader :current_user
attr_reader :pii
attr_reader :selected_email_id

def initialize(current_user:, pii:)
def initialize(current_user:, pii:, selected_email_id:)
@current_user = current_user
@pii = pii
@selected_email_id = selected_email_id
end

# @return [FormattedPii]
Expand All @@ -36,7 +38,11 @@ def format
private

def email
EmailContext.new(current_user).last_sign_in_email_address.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
end
end

def all_emails
Expand Down
4 changes: 3 additions & 1 deletion app/services/identity_linker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def link_identity(
scope: nil,
verified_attributes: nil,
last_consented_at: nil,
clear_deleted_at: nil
clear_deleted_at: nil,
email_address_id: nil
)
return unless user && service_provider.present?

Expand All @@ -43,6 +44,7 @@ def link_identity(
rails_session_id: rails_session_id,
scope: scope,
verified_attributes: combined_verified_attributes(verified_attributes),
email_address_id: email_address_id,
).tap do |hash|
hash[:last_consented_at] = last_consented_at if last_consented_at
hash[:deleted_at] = nil if clear_deleted_at
Expand Down
11 changes: 11 additions & 0 deletions app/views/sign_up/completions/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@
last_number: attribute_value[-1],
),
) %>
<% elsif attribute_key == :email && IdentityConfig.store.feature_select_email_to_share_enabled %>
<div class="display-flex flex-justify">
<%= attribute_value.to_s %>
<p class='font-body-2xs text-right'>
<% if @presenter.multiple_emails? %>
<%= link_to t('help_text.requested_attributes.change_email_link'), sign_up_select_email_path %>
<% else %>
<%= link_to t('account.index.email_add'), add_email_path %>
<% end %>
</p>
</div>
<% else %>
<%= attribute_value.to_s %>
<% end %>
Expand Down
Loading

0 comments on commit 9383fd8

Please sign in to comment.