Remove unnecessary bypass_sign_in calls

[mitchellhenke]

🛠 Summary of changes

The calls to bypass_sign_in removed in this PR seem to be unnecessary and are at best confusing since we should be able to rely on Devise to handle assignment of the user resource. The one exception is when updating the password while already authenticated because a change in the password hash terminates existing sessions.

[aduth]

Trying to understand the background here: The documentation also mentions this being "useful in cases where the user is already signed in, but we want to refresh the credentials in session.". Historically it looks like this was implemented following manipulation of the Warden session, which still technically happens, just indirectly through user_session. The internal implementation doesn't do much else than clear out and recreate the session. Wondering if there was something about setting to the user session which wouldn't persist on its own (or at least was perceived to), or something about the clearing to ensure a "clean slate"?

Overall, seems like a reasonable change, and the tests passing is at least some comfort in it not introducing any issues.

[mitchellhenke]

Trying to understand the background here: The documentation also mentions this being "useful in cases where the user is already signed in, but we want to refresh the credentials in session.". Historically it looks like this was implemented following manipulation of the Warden session, which still technically happens, just indirectly through user_session. The internal implementation doesn't do much else than clear out and recreate the session. Wondering if there was something about setting to the user session which wouldn't persist on its own (or at least was perceived to), or something about the clearing to ensure a "clean slate"?

Overall, seems like a reasonable change, and the tests passing is at least some comfort in it not introducing any issues.

Yeah, my research into it led me to a similar place. We don't appear to be setting any keys that match the devise regex that would be deleted by the method, and the method only writes warden.user.user.key with the user passed in without changing warder.user.user.session. In this case, "refresh the credentials" appears to only be about changing the User ID and password hash stored and doesn't give us a clean slate anyway. We're always passing current_user, which will generally not cause any changes to the session with the current functionality of the method (with the one exception).

Example session:

 {"warden.user.user.key"=>[[8582], "b02db48eb5cd218bcc6afe453512ad5bd5e37575e89519a7f262bb4bea583982"],
 "warden.user.user.session"=>
  {"need_two_factor_authentication"=>false,
   "unique_session_id"=>"K-SMJnzVtaZn7UqHkDRH",
   "last_request_at"=>1683128971,
   "auth_method"=>"remember_device",
   "authn_at"=>Wed, 03 May 2023 12:49:31.000000000 -03 -03:00}}