Improve Web UI session experience (#4804)

* Simplify Web UI sessions

Remove all of the hacks and support infrastructure around Rack sessions. Rails provides this by default so we don't need it for 95% of users. The other 5% need to provide a Rack session.

This is a big change and has the potential to break installs so it deserves at least a minor version bump.

See also #4671, #4728 and many others.

Changes.md

@@ -2,7 +2,35 @@
 
 [Sidekiq Changes](https://github.com/mperham/sidekiq/blob/master/Changes.md) | [Sidekiq Pro Changes](https://github.com/mperham/sidekiq/blob/master/Pro-Changes.md) | [Sidekiq Enterprise Changes](https://github.com/mperham/sidekiq/blob/master/Ent-Changes.md)
 
-6.1.3
+HEAD
+---------
+
+- Refactor Web UI session usage. [#4804]
+  Numerous people have hit "Forbidden" errors and struggled with Sidekiq's
+  Web UI session requirement. If you have code in your initializer for
+  Web sessions, it's quite possible it will need to be removed. Here's
+  an overview:
+```
+Sidekiq::Web needs a valid Rack session for CSRF protection. If this is a Rails app,
+make sure you mount Sidekiq::Web *inside* your routes in `config/routes.rb` so
+Sidekiq can reuse the Rails session:
+
+  Rails.application.routes.draw do
+    mount Sidekiq::Web => "/sidekiq"
+    ....
+  end
+
+If this is a bare Rack app, use a session middleware before Sidekiq::Web:
+
+  # first, use IRB to create a shared secret key for sessions and commit it
+  require 'securerandom'; File.open(".session.key", "w") {|f| f.write(SecureRandom.hex(32)) }
+
+  # now, update your Rack app to include the secret with a session cookie middleware
+  use Rack::Session::Cookie, secret: File.read(".session.key"), same_site: true, max_age: 86400
+  run Sidekiq::Web
+```
+
+  6.1.3
 ---------
 
 - Warn if Redis is configured to evict data under memory pressure [#4752]

Gemfile.lock

@@ -9,7 +9,7 @@ GIT
 PATH
   remote: .
   specs:
-    sidekiq (6.1.3)
+    sidekiq (6.2.0)
       connection_pool (>= 2.2.2)
       rack (~> 2.0)
       redis (>= 4.2.0)

lib/sidekiq/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Sidekiq
-  VERSION = "6.1.3"
+  VERSION = "6.2.0"
 end

lib/sidekiq/web.rb

@@ -13,10 +13,8 @@ require "sidekiq/web/application"
 require "sidekiq/web/csrf_protection"
 
 require "rack/content_length"
-
 require "rack/builder"
 require "rack/file"
-require "rack/session/cookie"
 
 module Sidekiq
   class Web
@@ -40,14 +38,6 @@ module Sidekiq
         self
       end
 
-      def middlewares
-        @middlewares ||= []
-      end
-
-      def use(*middleware_args, &block)
-        middlewares << [middleware_args, block]
-      end
-
       def default_tabs
         DEFAULT_TABS
       end
@@ -73,32 +63,37 @@ module Sidekiq
         opts.each { |key| set(key, false) }
       end
 
-      # Helper for the Sinatra syntax: Sidekiq::Web.set(:session_secret, Rails.application.secrets...)
       def set(attribute, value)
         send(:"#{attribute}=", value)
       end
 
-      attr_accessor :app_url, :session_secret, :redis_pool, :sessions
+      def sessions=(val)
+        puts "WARNING: Sidekiq::Web.sessions= is no longer relevant and will be removed in Sidekiq 7.0. #{caller(1..1).first}"
+      end
+
+      def session_secret=(val)
+        puts "WARNING: Sidekiq::Web.session_secret= is no longer relevant and will be removed in Sidekiq 7.0. #{caller(1..1).first}"
+      end
+
+      attr_accessor :app_url, :redis_pool
       attr_writer :locales, :views
     end
 
     def self.inherited(child)
       child.app_url = app_url
-      child.session_secret = session_secret
       child.redis_pool = redis_pool
-      child.sessions = sessions
     end
 
     def settings
       self.class.settings
     end
 
-    def use(*middleware_args, &block)
+    def middlewares
-      middlewares << [middleware_args, block]
+      @middlewares ||= []
     end
 
-    def middlewares
+    def use(*args, &block)
-      @middlewares ||= Web.middlewares.dup
+      middlewares << [args, block]
     end
 
     def call(env)
@@ -126,67 +121,26 @@ module Sidekiq
       send(:"#{attribute}=", value)
     end
 
-    # Default values
+    def sessions=(val)
-    set :sessions, true
+      puts "Sidekiq::Web#sessions= is no longer relevant and will be removed in Sidekiq 7.0. #{caller[2..2].first}"
-
-    attr_writer :sessions
-
-    def sessions
-      unless instance_variable_defined?("@sessions")
-        @sessions = self.class.sessions
-        @sessions = @sessions.to_hash.dup if @sessions.respond_to?(:to_hash)
-      end
-
-      @sessions
     end
 
     def self.register(extension)
       extension.registered(WebApplication)
     end
 
+    def default_middlewares
+      @default ||= [
+        [[Sidekiq::Web::CsrfProtection]],
+        [[Rack::ContentLength]]
+      ]
+    end
+
     private
 
-    def using?(middleware)
-      middlewares.any? do |(m, _)|
-        m.is_a?(Array) && (m[0] == middleware || m[0].is_a?(middleware))
-      end
-    end
-
-    def build_sessions
-      middlewares = self.middlewares
-
-      s = sessions
-
-      # turn on CSRF protection if sessions are enabled and this is not the test env
-      if s && !using?(CsrfProtection) && ENV["RACK_ENV"] != "test"
-        middlewares.unshift [[CsrfProtection], nil]
-      end
-
-      if s && !using?(::Rack::Session::Cookie)
-        unless (secret = Web.session_secret)
-          require "securerandom"
-          secret = SecureRandom.hex(64)
-        end
-
-        options = {secret: secret}
-        options = options.merge(s.to_hash) if s.respond_to? :to_hash
-
-        middlewares.unshift [[::Rack::Session::Cookie, options], nil]
-      end
-
-      # Since Sidekiq::WebApplication no longer calculates its own
-      # Content-Length response header, we must ensure that the Rack middleware
-      # that does this is loaded
-      unless using? ::Rack::ContentLength
-        middlewares.unshift [[::Rack::ContentLength], nil]
-      end
-    end
-
     def build
-      build_sessions
-
-      middlewares = self.middlewares
       klass = self.class
+      m = middlewares + default_middlewares
 
       ::Rack::Builder.new do
         %w[stylesheets javascripts images].each do |asset_dir|
@@ -195,7 +149,7 @@ module Sidekiq
           end
         end
 
-        middlewares.each { |middleware, block| use(*middleware, &block) }
+        m.each { |middleware, block| use(*middleware, &block) }
 
         run WebApplication.new(klass)
       end

lib/sidekiq/web/application.rb

@@ -4,7 +4,6 @@ module Sidekiq
   class WebApplication
     extend WebRouter
 
-    CONTENT_LENGTH = "Content-Length"
     REDIS_KEYS = %w[redis_version uptime_in_days connected_clients used_memory_human used_memory_peak_human]
     CSP_HEADER = [
       "default-src 'self' https: http:",
@@ -42,6 +41,13 @@ module Sidekiq
       # nothing, backwards compatibility
     end
 
+    head "/" do
+      # HEAD / is the cheapest heartbeat possible,
+      # it hits Redis to ensure connectivity
+      Sidekiq.redis { |c| c.llen("queue:default") }
+      ""
+    end
+
     get "/" do
       @redis_info = redis_info.select { |k, v| REDIS_KEYS.include? k }
       stats_history = Sidekiq::Stats::History.new((params["days"] || 30).to_i)

lib/sidekiq/web/csrf_protection.rb

@@ -66,7 +66,28 @@ module Sidekiq
       end
 
       def session(env)
-        env["rack.session"] || fail("you need to set up a session middleware *before* #{self.class}")
+        env["rack.session"] || fail(<<~EOM)
+          Sidekiq::Web needs a valid Rack session for CSRF protection. If this is a Rails app,
+          make sure you mount Sidekiq::Web *inside* your application routes:
+
+
+          Rails.application.routes.draw do
+            mount Sidekiq::Web => "/sidekiq"
+            ....
+          end
+
+
+          If this is a bare Rack app, use a session middleware before Sidekiq::Web:
+
+
+          # first, use IRB to create a shared secret key for sessions and commit it
+          require 'securerandom'; File.open(".session.key", "w") {|f| f.write(SecureRandom.hex(32)) }
+
+
+          # now use the secret with a session cookie middleware
+          use Rack::Session::Cookie, secret: File.read(".session.key"), same_site: true, max_age: 86400
+          run Sidekiq::Web
+        EOM
       end
 
       def accept?(env)

lib/sidekiq/web/router.rb

@@ -15,6 +15,10 @@ module Sidekiq
     REQUEST_METHOD = "REQUEST_METHOD"
     PATH_INFO = "PATH_INFO"
 
+    def head(path, &block)
+      route(HEAD, path, &block)
+    end
+
     def get(path, &block)
       route(GET, path, &block)
     end
@@ -39,7 +43,6 @@ module Sidekiq
       @routes ||= {GET => [], POST => [], PUT => [], PATCH => [], DELETE => [], HEAD => []}
 
       @routes[method] << WebRoute.new(method, path, block)
-      @routes[HEAD] << WebRoute.new(method, path, block) if method == GET
     end
 
     def match(env)

myapp/db/migrate/20120123214055_create_posts.rb

@@ -1,4 +1,4 @@
-class CreatePosts < ActiveRecord::Migration
+class CreatePosts < ActiveRecord::Migration[6.0]
   def change
     create_table :posts do |t|
       t.string :title

myapp/simple.ru

@@ -1,7 +1,7 @@
 # Easiest way to run Sidekiq::Web.
 # Run with "bundle exec rackup simple.ru"
 
-require 'sidekiq'
+require 'sidekiq/web'
 
 # A Web process always runs as client, no need to configure server
 Sidekiq.configure_client do |config|
@@ -10,5 +10,10 @@ end
 
 Sidekiq::Client.push('class' => "HardWorker", 'args' => [])
 
-require 'sidekiq/web'
+# In a multi-process deployment, all Web UI instances should share
+# this secret key so they can all decode the encrypted browser cookies
+# and provide a working session.
+# Rails does this in /config/initializers/secret_token.rb
+secret_key = SecureRandom.hex(32)
+use Rack::Session::Cookie, secret: secret_key, same_site: true, max_age: 86400
 run Sidekiq::Web

test/test_web.rb

@@ -9,7 +9,7 @@ describe Sidekiq::Web do
   include Rack::Test::Methods
 
   def app
-    Sidekiq::Web
+    @app ||= Sidekiq::Web.new
   end
 
   def job_params(job, score)
@@ -17,9 +17,8 @@ describe Sidekiq::Web do
   end
 
   before do
-    ENV["RACK_ENV"] = "test"
     Sidekiq.redis {|c| c.flushdb }
-    Sidekiq::Web.middlewares.clear
+    app.default_middlewares.clear
   end
 
   class WebWorker
@@ -30,11 +29,6 @@ describe Sidekiq::Web do
     end
   end
 
-  it 'can configure via set() syntax' do
-    app.set(:session_secret, "foo")
-    assert_equal "foo", app.session_secret
-  end
-
   it 'can show text with any locales' do
     rackenv = {'HTTP_ACCEPT_LANGUAGE' => 'ru,en'}
     get '/', {}, rackenv
@@ -743,6 +737,7 @@ describe Sidekiq::Web do
     def app
       app = Sidekiq::Web.new
       app.use(Rack::Auth::Basic) { |user, pass| user == "a" && pass == "b" }
+      app.use(Rack::Session::Cookie, secret: SecureRandom.hex(32))
 
       app
     end
@@ -780,66 +775,6 @@ describe Sidekiq::Web do
       assert_equal 'v3rys3cr31', session_options[:secret]
       assert_equal 'nicehost.org', session_options[:host]
     end
-
-    describe 'sessions options' do
-      include Rack::Test::Methods
-
-      describe 'using #disable' do
-        def app
-          app = Sidekiq::Web.new
-          app.disable(:sessions)
-          app
-        end
-
-        it "doesn't create sessions" do
-          get '/'
-          assert_nil last_request.env['rack.session']
-        end
-      end
-
-      describe 'using #set with false argument' do
-        def app
-          app = Sidekiq::Web.new
-          app.set(:sessions, false)
-          app
-        end
-
-        it "doesn't create sessions" do
-          get '/'
-          assert_nil last_request.env['rack.session']
-        end
-      end
-
-      describe 'using #set with an hash' do
-        def app
-          app = Sidekiq::Web.new
-          app.set(:sessions, { domain: :all })
-          app
-        end
-
-        it "creates sessions" do
-          get '/'
-          refute_nil   last_request.env['rack.session']
-          refute_empty last_request.env['rack.session'].options
-          assert_equal :all, last_request.env['rack.session'].options[:domain]
-        end
-      end
-
-      describe 'using #enable' do
-        def app
-          app = Sidekiq::Web.new
-          app.enable(:sessions)
-          app
-        end
-
-        it "creates sessions" do
-          get '/'
-          refute_nil   last_request.env['rack.session']
-          refute_empty last_request.env['rack.session'].options
-          refute_nil   last_request.env['rack.session'].options[:secret]
-        end
-      end
-    end
   end
 
   describe "redirecting in before" do
@@ -857,7 +792,9 @@ describe Sidekiq::Web do
     end
 
     def app
-      Sidekiq::Web.new
+      app = Sidekiq::Web.new
+      app.use Rack::Session::Cookie, secret: 'v3rys3cr31', host: 'nicehost.org'
+      app
     end
 
     it "allows afters to run" do