Skip to content

Fix long session expiry for some requests

What does this MR do and why?

We lower session expiry times for unauthenticated users in ApplicationController but this does not cover other cases like GraphQL explorer and Grape API routes.

This change moves the lowering of the expiry to a Rack middleware that runs just before we commit the session to Redis.

History

  1. This started as a before_action in ApplicationController. !6586 (merged)
  2. This did not work for OAuth logins because current_user was set at the end of the controller action. So this was moved to an after_action. gitlab-foss!21144 (merged)
  3. This did not work for routes that redirected early because the after_action would be skipped. So we moved it back to a before_action hook but added another after_action to handle the OAuth case and set it back to 7 days if logged in. !70444 (merged)
  4. There were still some cases missed because the before_action wasn't early enough. So it was moved to the top of ApplicationController. !88514 (merged)

This MR makes it similar to the after_action implementation in 2. But since it is implemented in Rack middleware, it will not get interrupted by redirects or other before_action hooks. Also, this would cover cases which don't go through ApplicationController.

MR acceptance checklist

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

How to set up and validate locally

  1. Run gdk redis-cli monitor | grep "session:gitlab" to see the session expiry times being set. Authenticated should be 604800 and unauthenticated ones should be 7200.
  2. Run through the different scenarios below to verify the correct expiry time.

Scenarios fixed by the current MR

  1. Visit /-/graphql-explorer

    This creates a new session because it needs to set the CSRF token. This is a mounted engine from a gem so this was not covered by ApplicationController before.

  2. Using a web browser, visit any URL to create a session (this will be 7200 initially), then visit /api/v4/version or any other API endpoint

    If you visit an API endpoint directly, we don't create sessions. But if you have an existing session, API endpoints read the session to check for the current user since we allow cookie auth.

    When Rack session data is loaded, they are persisted again at the end of the request: https://github.com/rack/rack-session/blob/d2f080c243cac167fc5176c5cf869e23fe7f6ec6/lib/rack/session/abstract/id.rb#L345-L355. And since these API endpoints don't inherit from ApplicationController, the expiry will be set to 604800.

Edited by Heinrich Lee Yu

Merge request reports

Loading