Redirect to sign in from authentication controllers

[aduth]

🛠 Summary of changes

Updates the logic for redirecting during a partner-initiated authentication in order to remove the request_id query parameter, since the request ID will have already been saved to the session at this point.

This is a follow-up to #8137 (comment) :

I started exploring removing the request_id from the redirects initiated by SamlIdpController and OpenidConnect::AuthorizationController, since the request_id will have already been stored at those points, via store_saml_request and store_request respectively. I think there's definitely some simplification that can be done here, particularly with removing the argument to confirm_two_factor_authenticated and maybe even the first line of that method. But it's a bit more involved since the Warden authenticate_user! will cause an alert banner to be shown on the Sign In screen without that first line of the method.

📜 Testing Plan

• Confirm that you can sign in from a service provider, and that the Sign In screen maintains the branded experience and does not include any unexpected alert banners (e.g. "Your session has expired")

[mitchellhenke]

This rules 👏🏼 👏🏼 👏🏼 👏🏼 👏🏼

[aduth]

Couple thoughts:

• Maybe the device expired should take priority here (at least if checking second-factor), if it was running as the last thing in the method previously?
• Is user_fully_authenticated? still accurate with the new redirect and removal of the MFA policy method? Or should the user only be redirected to sign in if there's no sign-in at all (i.e. haven't entered username/password)

Suggested change

	if !user_fully_authenticated?
	redirect_to new_user_session_url
	elsif remember_device_expired_for_sp?
	redirect_to user_two_factor_authentication_url
	if !user_signed_in?
	redirect_to new_user_session_url
	elsif remember_device_expired_for_sp?
	redirect_to user_two_factor_authentication_url

[mitchellhenke]

The case where user_fully_authenticated? is false and user_signed_in? would happen if there was an email/password auth, but not 2FA? I think it would be fine to be stricter and switch to user_signed_in?

I think the elsif would have to change though:

- elsif remember_device_expired_for_sp?
+ elsif user_fully_authenticated? && remember_device_expired_for_sp?

[aduth]

Hmm, yeah, you might be right about that. I pushed the first half in f86ff64, but curious to see if any existing tests fail, and will dig into overlap between remember device + fully authenticated.

[aduth]

I added back the user_fully_authenticated? in 0e95583, and later refactored in d72c270 to try to get even closer to the initial logic.