Skip to content

Commit

Permalink
Improve Web UI session experience (#4804)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
mperham committed Feb 12, 2021
1 parent 2f049ea commit 968bc81
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 147 deletions.
30 changes: 29 additions & 1 deletion Changes.md
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/sidekiq/version.rb
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module Sidekiq
VERSION = "6.1.3"
VERSION = "6.2.0"
end
92 changes: 23 additions & 69 deletions lib/sidekiq/web.rb
Expand Up @@ -13,10 +13,8 @@
require "sidekiq/web/csrf_protection"

require "rack/content_length"

require "rack/builder"
require "rack/file"
require "rack/session/cookie"

module Sidekiq
class Web
Expand All @@ -40,14 +38,6 @@ def settings
self
end

def middlewares
@middlewares ||= []
end

def use(*middleware_args, &block)
middlewares << [middleware_args, block]
end

def default_tabs
DEFAULT_TABS
end
Expand All @@ -73,32 +63,37 @@ def disable(*opts)
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)
middlewares << [middleware_args, block]
def middlewares
@middlewares ||= []
end

def middlewares
@middlewares ||= Web.middlewares.dup
def use(*args, &block)
middlewares << [args, block]
end

def call(env)
Expand Down Expand Up @@ -126,67 +121,26 @@ def set(attribute, value)
send(:"#{attribute}=", value)
end

# Default values
set :sessions, true

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
def sessions=(val)
puts "Sidekiq::Web#sessions= is no longer relevant and will be removed in Sidekiq 7.0. #{caller[2..2].first}"
end

def self.register(extension)
extension.registered(WebApplication)
end

private

def using?(middleware)
middlewares.any? do |(m, _)|
m.is_a?(Array) && (m[0] == middleware || m[0].is_a?(middleware))
end
def default_middlewares
@default ||= [
[[Sidekiq::Web::CsrfProtection]],
[[Rack::ContentLength]]
]
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
private

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|
Expand All @@ -195,7 +149,7 @@ def build
end
end

middlewares.each { |middleware, block| use(*middleware, &block) }
m.each { |middleware, block| use(*middleware, &block) }

run WebApplication.new(klass)
end
Expand Down
8 changes: 7 additions & 1 deletion lib/sidekiq/web/application.rb
Expand Up @@ -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:",
Expand Down Expand Up @@ -42,6 +41,13 @@ def self.set(key, val)
# 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)
Expand Down
23 changes: 22 additions & 1 deletion lib/sidekiq/web/csrf_protection.rb
Expand Up @@ -66,7 +66,28 @@ def deny(env)
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)
Expand Down
5 changes: 4 additions & 1 deletion lib/sidekiq/web/router.rb
Expand Up @@ -15,6 +15,10 @@ module WebRouter
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
Expand All @@ -39,7 +43,6 @@ def route(method, path, &block)
@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)
Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Down
9 changes: 7 additions & 2 deletions 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|
Expand All @@ -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

0 comments on commit 968bc81

Please sign in to comment.