Follow-up from "Add `Remember me` to the SAML workflow"
The following discussions from !103987 (merged) should be addressed:
-
@tachyons-gitlab started a discussion: (+1 comment) I am getting
undefined method
username' for nil:NilClass` when the user is not logged in -
@dblessing started a discussion: (+4 comments) set_remember_me
is already called insign_in_user_flow
. I think we already support remember me for self-managed OmniAuth sign in. Do you know for sure?If that's the case, then maybe it's best to move this logic to
Groups::OmniauthCallbacksController
. That, or move the existing one to a place like this that works for all cases. -
@dblessing started a discussion: (+4 comments) I finally looked into this placement more and understand the difference between placing it here, and placing it where it currently is, within
sign_in_user_flow
.The current location sets the remember me cookie only for users that are completely signed out, and are signing in via OmniAuth. This makes sense because for Google, Twitter, etc. we really don't have this concept of already being signed in locally and then signing in again with those. However, we do have this re-auth/multi-auth concept with Group SAML.
So the question is, do we only want to set the remember me cookie if the user is signed out completely and then signs in with SAML, or for re-auth, too? I suppose it makes it easier if we don't need logic on the view to determine whether to show the checkbox or not.