Skip to content

Remove no-op code re: clearing session

Jessie Young requested to merge jy-reg-tweaks into master

What does this MR do and why?

This code was added here: gitlab-foss!17634 (merged) Reasoning was: gitlab-foss!17634 (comment 70474261)

we need to invalidate the stored_location_for in order to avoid a 404 after the user signs in.

This is because, at the time, def authenticate_user! (what is referenced as app/controllers/invites_controller.rb#L57) looked like this:

def authenticate_user!
  return if current_user

  notice = "To accept this invitation, sign in"
  notice << " or create an account" if Gitlab::CurrentSettings.allow_signup?
  notice << "."

  store_location_for :user, request.fullpath
  redirect_to new_user_session_path, notice: notice
end

Today, authenticate_user! looks like this:

  def authenticate_user!
    return if current_user

    store_location_for(:user, invite_landing_url) if member

    if user_sign_up?
      set_session_invite_params

      redirect_to new_user_registration_path(invite_email: member.invite_email), notice: _("To accept this invitation, create an account or sign in.")
    else
      redirect_to new_user_session_path(sign_in_redirect_params), notice: sign_in_notice
    end
  end

See this MR for more info on the redirect behavior when accepting an invitation.

When the session clearing code aws added, store_location_for :user, request.fullpath would result in a 404 after the user signed in because request.fullpath was an tokenized invitation URL that was invalidated during the invitations acceptance flow.

Now, the user is taken to invite_landing_url via the saved session store_location_for, which is the activity path for the Project or Group that the user was invited to. This will not 404 because invitations are automatically accepted for users during the sign up flow.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Jessie Young

Merge request reports

Loading