Merge branch 'sh-limit-unauthenticated-session-times' into 'master'

Limit the TTL for anonymous sessions to 1 hour

Closes #48101

See merge request gitlab-org/gitlab-ce!20700

app/controllers/application_controller.rb

@@ -11,6 +11,7 @@ class ApplicationController < ActionController::Base
   include EnforcesTwoFactorAuthentication
   include WithPerformanceBar
 
+  before_action :limit_unauthenticated_session_times
   before_action :authenticate_sessionless_user!
   before_action :authenticate_user!
   before_action :enforce_terms!, if: :should_enforce_terms?
@@ -85,6 +86,24 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  # By default, all sessions are given the same expiration time configured in
+  # the session store (e.g. 1 week). However, unauthenticated users can
+  # generate a lot of sessions, primarily for CSRF verification. It makes
+  # sense to reduce the TTL for unauthenticated to something much lower than
+  # the default (e.g. 1 hour) to limit Redis memory. In addition, Rails
+  # creates a new session after login, so the short TTL doesn't even need to
+  # be extended.
+  def limit_unauthenticated_session_times
+    return if current_user
+
+    # Rack sets this header, but not all tests may have it: https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L251-L259
+    return unless request.env['rack.session.options']
+
+    # This works because Rack uses these options every time a request is handled:
+    # https://github.com/rack/rack/blob/fdcd03a3c5a1c51d1f96fc97f9dfa1a9deac0c77/lib/rack/session/abstract/id.rb#L342
+    request.env['rack.session.options'][:expire_after] = Settings.gitlab['unauthenticated_session_expire_delay']
+  end
+
   protected
 
   def append_info_to_payload(payload)

changelogs/unreleased/sh-limit-unauthenticated-session-times.yml

@@ -0,0 +1,5 @@
+---
+title: Limit the TTL for anonymous sessions to 1 hour
+merge_request: 20700
+author:
+type: performance

config/initializers/1_settings.rb

@@ -140,6 +140,7 @@ Settings.gitlab['default_projects_features'] ||= {}
 Settings.gitlab['webhook_timeout'] ||= 10
 Settings.gitlab['max_attachment_size'] ||= 10
 Settings.gitlab['session_expire_delay'] ||= 10080
+Settings.gitlab['unauthenticated_session_expire_delay'] ||= 1.hour.to_i
 Settings.gitlab.default_projects_features['issues']             = true if Settings.gitlab.default_projects_features['issues'].nil?
 Settings.gitlab.default_projects_features['merge_requests']     = true if Settings.gitlab.default_projects_features['merge_requests'].nil?
 Settings.gitlab.default_projects_features['wiki']               = true if Settings.gitlab.default_projects_features['wiki'].nil?

spec/controllers/application_controller_spec.rb

@@ -89,6 +89,32 @@ describe ApplicationController do
     end
   end
 
+  describe 'session expiration' do
+    controller(described_class) do
+      def index
+        render text: 'authenticated'
+      end
+    end
+
+    context 'authenticated user' do
+      it 'does not set the expire_after option' do
+        sign_in(create(:user))
+
+        get :index
+
+        expect(request.env['rack.session.options'][:expire_after]).to be_nil
+      end
+    end
+
+    context 'unauthenticated user' do
+      it 'sets the expire_after option' do
+        get :index
+
+        expect(request.env['rack.session.options'][:expire_after]).to eq(Settings.gitlab['unauthenticated_session_expire_delay'])
+      end
+    end
+  end
+
   describe 'rescue from Gitlab::Git::Storage::Inaccessible' do
     controller(described_class) do
       def index