Skip to content

Consolidate / standardize URL handling for after_sign_in_path_for#9159

Merged
aduth merged 6 commits intomainfrom
aduth-clean-after-sign-in-path
Sep 8, 2023
Merged

Consolidate / standardize URL handling for after_sign_in_path_for#9159
aduth merged 6 commits intomainfrom
aduth-clean-after-sign-in-path

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Sep 6, 2023

🛠 Summary of changes

Updates handling of signed-in URL handling:

  • Removes proxy methods in favor of inline condition early returns
    • The hope here is (a) make the logic clearer by avoiding the indirection and (b) avoid naming gymnastics (and resulting confusion) in generating a URL under only certain conditions (e.g. accept_rules_of_use_url returns rules_of_use_path only if the user must accept rules of user, otherwise returns nil).
  • Similar to Move user suspended check for sign-in #9147, moves more interception screens into the expected SP-initiated sign-in flow
    • Backup code reminder
    • Account reactivation
  • Removes a seemingly-redundant check for user_fully_authenticated? in signed_in_url

Ideally we would be able to eliminate one or the other of after_sign_in_path_for and signed_in_url, but we currently rely on signed_in_url for handling various pending profile states. However, this is something which we ought to reevaluate, since it blocks users from managing their account settings in some cases (see LG-10119).

📜 Testing Plan

Verify that there are no regressions in the following prompts:

  • Rules of use
  • PIV/CAC setup from sign-in attempt
  • Service-provider required MFA
  • Broken personal key
  • Suspended account dead-end
  • Identity-verified profile reactivation after password reset
  • GPO pending IdV
  • IPP pending IdV
  • Backup code reminder

changelog: Internal, Code Quality, Consolidate route handling for after-sign-in behavior
@aduth aduth marked this pull request as draft September 7, 2023 14:03
@aduth aduth marked this pull request as ready for review September 7, 2023 14:35
@aduth aduth merged commit e7cb9c3 into main Sep 8, 2023
@aduth aduth deleted the aduth-clean-after-sign-in-path branch September 8, 2023 14:03
@aduth aduth mentioned this pull request Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants