Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ def after_sign_in_path_for(_user)

def signed_in_url
return user_two_factor_authentication_url unless user_fully_authenticated?
return account_or_verify_profile_url if profile_needs_verification?
return reactivate_account_url if user_needs_to_reactivate_account?
return url_for_pending_profile_reason if user_has_pending_profile?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth trying to move away from the generic "has pending profile"? If we wanted to, we could take a small shortcut and replace the conditionals used with:

Suggested change
return url_for_pending_profile_reason if user_has_pending_profile?
return url_for_pending_profile_reason if url_for_pending_profile_reason

It looks like the method would still be used by some analytics, but it might be worth splitting that up to be more specific too.

return backup_code_reminder_url if user_needs_backup_code_reminder?
account_url
end
Expand Down
14 changes: 7 additions & 7 deletions app/controllers/concerns/verify_profile_concern.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
module VerifyProfileConcern
private

def account_or_verify_profile_url
return reactivate_account_url if user_needs_to_reactivate_account?
return idv_gpo_verify_url if profile_needs_verification?
account_url
def url_for_pending_profile_reason
return idv_gpo_verify_url if current_user.gpo_verification_pending_profile?
return idv_in_person_ready_to_verify_url if current_user.in_person_pending_profile?
return idv_please_call_url if current_user.fraud_review_pending?
idv_not_verified_url if current_user.fraud_rejection?
end

def profile_needs_verification?
def user_has_pending_profile?
return false if current_user.blank?
current_user.gpo_verification_pending_profile? ||
user_needs_to_reactivate_account?
current_user.pending_profile?
end
end
31 changes: 6 additions & 25 deletions app/controllers/openid_connect/authorization_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ class AuthorizationController < ApplicationController
include SecureHeadersConcern
include AuthorizationCountConcern
include BillableEventTrackable
include FraudReviewConcern

before_action :build_authorize_form_from_params, only: [:index]
before_action :pre_validate_authorize_form, only: [:index]
Expand All @@ -22,10 +21,12 @@ class AuthorizationController < ApplicationController
before_action :prompt_for_password_if_ial2_request_and_pii_locked, only: [:index]

def index
return redirect_to_fraud_review if fraud_review_pending_for_ial2_request?
return redirect_to_fraud_rejection if fraud_rejection_for_ial2_request?
return redirect_to_account_or_verify_profile_url if profile_or_identity_needs_verification?
return redirect_to(sign_up_completed_url) if needs_completion_screen_reason
if @authorize_form.ial2_or_greater?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much easier to read 😀

return redirect_to reactivate_account_url if user_needs_to_reactivate_account?
return redirect_to url_for_pending_profile_reason if user_has_pending_profile?
return redirect_to idv_url if identity_needs_verification?
end
return redirect_to sign_up_completed_url if needs_completion_screen_reason
link_identity_to_service_provider

result = @authorize_form.submit
Expand Down Expand Up @@ -76,26 +77,6 @@ def handle_successful_handoff
delete_branded_experience
end

def redirect_to_account_or_verify_profile_url
return redirect_to(account_or_verify_profile_url) if profile_needs_verification?
redirect_to(idv_url) if identity_needs_verification?
end

def fraud_review_pending_for_ial2_request?
return false unless @authorize_form.ial2_or_greater?
fraud_review_pending?
end

def fraud_rejection_for_ial2_request?
return false unless @authorize_form.ial2_or_greater?
fraud_rejection?
end

def profile_or_identity_needs_verification?
return false unless @authorize_form.ial2_or_greater?
profile_needs_verification? || identity_needs_verification?
end

def track_authorize_analytics(result)
analytics_attributes = result.to_h.except(:redirect_uri).
merge(user_fully_authenticated: user_fully_authenticated?)
Expand Down
38 changes: 16 additions & 22 deletions app/controllers/saml_idp_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class SamlIdpController < ApplicationController
include AuthorizationCountConcern
include BillableEventTrackable
include SecureHeadersConcern
include FraudReviewConcern

prepend_before_action :skip_session_load, only: [:metadata, :remotelogout]
prepend_before_action :skip_session_expiration, only: [:metadata, :remotelogout]
Expand All @@ -24,13 +23,16 @@ class SamlIdpController < ApplicationController
before_action :redirect_to_sign_in, only: :auth, unless: :user_signed_in?
before_action :confirm_two_factor_authenticated, only: :auth
before_action :redirect_to_reauthenticate, only: :auth, if: :remember_device_expired_for_sp?
before_action :prompt_for_password_if_ial2_request_and_pii_locked, only: :auth

def auth
capture_analytics
return redirect_to_fraud_review if fraud_review_pending? && ial2_requested?
return redirect_to_fraud_rejection if fraud_rejection? && ial2_requested?
return redirect_to_verification_url if profile_or_identity_needs_verification_or_decryption?
return redirect_to(sign_up_completed_url) if needs_completion_screen_reason
if ial_context.ial2_or_greater?
return redirect_to reactivate_account_url if user_needs_to_reactivate_account?
return redirect_to url_for_pending_profile_reason if user_has_pending_profile?
return redirect_to idv_url if identity_needs_verification?
end
return redirect_to sign_up_completed_url if needs_completion_screen_reason
if auth_count == 1 && first_visit_for_sp?
return redirect_to(user_authorization_confirmation_url)
end
Expand Down Expand Up @@ -99,31 +101,23 @@ def saml_metadata
SamlEndpoint.new(params[:path_year]).saml_metadata
end

def redirect_to_verification_url
return redirect_to(account_or_verify_profile_url) if profile_needs_verification?
redirect_to(idv_url) if identity_needs_verification?
redirect_to capture_password_url if identity_needs_decryption?
end

def profile_or_identity_needs_verification_or_decryption?
return false unless ial_context.ial2_or_greater? || ialmax_requested_with_ial2_user?
profile_needs_verification? || identity_needs_verification? || identity_needs_decryption?
def prompt_for_password_if_ial2_request_and_pii_locked
return unless pii_requested_but_locked?
redirect_to capture_password_url
end

def ialmax_requested_with_ial2_user?
ial_context.ialmax_requested? && identity_needs_decryption?
end

def identity_needs_decryption?
current_user.identity_verified? &&
!Pii::Cacher.new(current_user, user_session).exists_in_session?
def pii_requested_but_locked?
if (sp_session && sp_session_ial > 1) || ial_context.ialmax_requested?
current_user.identity_verified? &&
!Pii::Cacher.new(current_user, user_session).exists_in_session?
end
end

def capture_analytics
analytics_payload = @result.to_h.merge(
endpoint: api_saml_auth_path(path_year: params[:path_year]),
idv: identity_needs_verification?,
finish_profile: profile_needs_verification?,
finish_profile: user_has_pending_profile?,
requested_ial: requested_ial,
request_signed: saml_request.signed?,
matching_cert_serial: saml_request.service_provider.matching_cert&.serial&.to_s,
Expand Down
8 changes: 8 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ def fraud_rejection_profile
pending_profile if pending_profile&.fraud_rejection?
end

def in_person_pending_profile?
in_person_pending_profile.present?
end

def in_person_pending_profile
pending_profile if pending_profile&.in_person_verification_pending?
end

def personal_key_generated_at
encrypted_recovery_code_digest_generated_at ||
active_profile&.verified_at ||
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/saml_idp_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2073,7 +2073,7 @@ def stub_requested_attributes

stub_analytics
allow(controller).to receive(:identity_needs_verification?).and_return(false)
allow(controller).to receive(:profile_needs_verification?).and_return(true)
allow(controller).to receive(:user_has_pending_profile?).and_return(true)

analytics_hash = {
success: true,
Expand Down