Add middleware to prevent cross-cell session cookie overwrite

What does this MR do and why?

Every GitLab Cell shares the same _gitlab_session cookie name; only the cookie value is prefixed with the owning cell (e.g. cell-3-<sid>). When path-based routing sends a request to a non-owning cell and the visitor is not authenticated there, Rails mints a brand-new empty session and writes it back under the same cookie name. The browser overwrites the home cell's cookie with the new value, orphaning the original session and logging the user out of their home cell permanently.

This MR adds Gitlab::Middleware::PreventCrossCellSessionOverwrite, inserted right after the session store in the middleware stack. After the app call completes, it checks whether:

  1. Cells are enabled (Gitlab.config.cell.enabled).
  2. The incoming session cookie carries a cell-<id>- prefix belonging to a different cell than the one handling the request.
  3. The visitor is not authenticated on this cell (checked via Warden, same pattern as UnauthenticatedSessionExpiry).

When all three hold, it sets rack.session.options[:skip] = true. Rack then skips both the Set-Cookie response header and the Redis write, leaving the browser's original cookie intact.

Exception: the sign-in page. SessionsController#new allows the session overwrite. It calls Gitlab::Middleware::PreventCrossCellSessionOverwrite.allow_overwrite!(request) in a prepend_before_action for new only. When a controller allowed it, the middleware does not set rack.session.options[:skip].

Why the sign-in page needs this

A user can enter a username that lives on another cell. The router then sends GET /users/sign_in?login= to the home cell of that user. Without the opt-in, that cell does not replace the cookie of the previous cell. The router then sends the password POST to the previous cell, because of the old cookie. The POST fails with 422 Can't verify CSRF token authenticity.

The opt-in covers every route to sessions#new: /users/sign_in, /o/:organization_path/users/sign_in (organization routes clone all routes), and /users/auth/geo/sign_in. The opt-in lives in the controller. The middleware has no path list.

A user who is signed in on cell 1 and opens a sign-in link for an account on cell 2 loses the cell 1 session. This is an explicit account switch. It matches the behavior before this MR.

The behavior is gated behind the prevent_cross_cell_session_overwrite gitlab_com_derisk feature flag (default off).

Known trade-off: an anonymous visitor who submits another form on a page routed to another cell may hit a CSRF mismatch. Sign-in is no longer affected.

Why this trade-off is acceptable

The ephemeral session of the visitor is never persisted. This is a narrow edge case, because almost all write actions require login. It is substantially better than the current hard logout.

References

Screenshots or screen recordings

Not applicable - no UI changes.

How to set up and validate locally

  1. Enable cells in config/gitlab.yml with cell.enabled: true and cell.id: 1.
  2. Enable the feature flag in a Rails console: Feature.enable(:prevent_cross_cell_session_overwrite).
  3. Set a cookie in the browser with value cell-3-<anything> for the _gitlab_session key.
  4. Make a request to the local instance. Confirm no Set-Cookie header is returned for _gitlab_session.
  5. Without the flag, confirm the old Set-Cookie: _gitlab_session=cell-1-… response is present.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability

Edited by Marco Gregorius

Merge request reports

Loading
Loading