Skip to content

Commit

Permalink
FEATURE: rename whitelist to allow_authorized
Browse files Browse the repository at this point in the history
Whitelist is both a loaded term and confusing in rack-mini-profiler configuration.

We deprecated usage of  `authorization_mode = :whitelist` instead this should be used `authorization_mode = :allow_authorized` 

In 6 months or so we will consider removing the deprecation and forcing usage of the renamed option.
  • Loading branch information
rthbound authored Apr 12, 2021
1 parent 94670b7 commit e99b44d
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 34 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,21 +235,21 @@ rack-mini-profiler is designed with production profiling in mind. To enable that
Note:

Out-of-the-box we will initialize the `authorization_mode` to `:whitelist` in production. However, in some cases we may not be able to do it:
Out-of-the-box we will initialize the `authorization_mode` to `:allow_authorized` in production. However, in some cases we may not be able to do it:

- If you are running in development or test we will not enable whitelist mode
- If you are running in development or test we will not enable the explicit authorization mode
- If you use `require: false` on rack_mini_profiler we are unlikely to be able to run the railtie
- If you are running outside of rails we will not run the railtie

In those cases use:

```ruby
Rack::MiniProfiler.config.authorization_mode = :whitelist
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
```

When deciding to fully profile a page mini profiler consults with the `authorization_mode`

By default in production we attempt to set the authorization mode to `:whitelist` meaning that end user will only be able to see requests where somewhere `Rack::MiniProfiler.authorize_request` is invoked.
By default in production we attempt to set the authorization mode to `:allow_authorized` meaning that end user will only be able to see requests where somewhere `Rack::MiniProfiler.authorize_request` is invoked.

In development we run in the `:allow_all` authorization mode meaning every request is profiled and displayed to the end user.

Expand Down
6 changes: 3 additions & 3 deletions lib/mini_profiler/client_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def initialize(env, store, start)
def handle_cookie(result)
status, headers, _body = result

if (MiniProfiler.config.authorization_mode == :whitelist && !MiniProfiler.request_authorized?)
if (MiniProfiler.config.authorization_mode == :allow_authorized && !MiniProfiler.request_authorized?)
# this is non-obvious, don't kill the profiling cookie on errors or short requests
# this ensures that stuff that never reaches the rails stack does not kill profiling
if status.to_i >= 200 && status.to_i < 300 && ((Process.clock_gettime(Process::CLOCK_MONOTONIC) - @start) > 0.1)
Expand All @@ -59,7 +59,7 @@ def write!(headers)

tokens_changed = false

if MiniProfiler.request_authorized? && MiniProfiler.config.authorization_mode == :whitelist
if MiniProfiler.request_authorized? && MiniProfiler.config.authorization_mode == :allow_authorized
@allowed_tokens ||= @store.allowed_tokens
tokens_changed = !@orig_auth_tokens || ((@allowed_tokens - @orig_auth_tokens).length > 0)
end
Expand Down Expand Up @@ -90,7 +90,7 @@ def discard_cookie!(headers)
def has_valid_cookie?
valid_cookie = !@cookie.nil?

if (MiniProfiler.config.authorization_mode == :whitelist) && valid_cookie
if (MiniProfiler.config.authorization_mode == :allow_authorized) && valid_cookie
begin
@allowed_tokens ||= @store.allowed_tokens
rescue => e
Expand Down
14 changes: 14 additions & 0 deletions lib/mini_profiler/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,20 @@ def self.default

attr_reader :assets_url

def authorization_mode=(mode)
if mode == :whitelist
warn "[DEPRECATION] `:whitelist` authorization mode is deprecated. Please use `:allow_authorized` instead."

mode = :allow_authorized
end

warn <<~DEP unless mode == :allow_authorized || mode == :allow_all
[DEPRECATION] unknown authorization mode #{mode}. Expected `:allow_all` or `:allow_authorized`.
DEP

@authorization_mode = mode
end

def assets_url=(lmbda)
if defined?(Rack::MiniProfilerRails)
Rack::MiniProfilerRails.create_engine
Expand Down
6 changes: 3 additions & 3 deletions lib/mini_profiler/profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def tool_disabled_message(client_settings)
def call(env)
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
client_settings = ClientSettings.new(env, @storage, start)
MiniProfiler.deauthorize_request if @config.authorization_mode == :whitelist
MiniProfiler.deauthorize_request if @config.authorization_mode == :allow_authorized

status = headers = body = nil
query_string = env['QUERY_STRING']
Expand All @@ -239,7 +239,7 @@ def call(env)
skip_it = (@config.pre_authorize_cb && !@config.pre_authorize_cb.call(env))

if skip_it || (
@config.authorization_mode == :whitelist &&
@config.authorization_mode == :allow_authorized &&
!client_settings.has_valid_cookie?
)
if take_snapshot?(path)
Expand Down Expand Up @@ -388,7 +388,7 @@ def call(env)

skip_it = current.discard

if (config.authorization_mode == :whitelist && !MiniProfiler.request_authorized?)
if (config.authorization_mode == :allow_authorized && !MiniProfiler.request_authorized?)
skip_it = true
end

Expand Down
2 changes: 1 addition & 1 deletion lib/mini_profiler/storage/abstract_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def diagnostics(user)
""
end

# a list of tokens that are permitted to access profiler in whitelist mode
# a list of tokens that are permitted to access profiler in explicit mode
def allowed_tokens
raise NotImplementedError.new("allowed_tokens is not implemented")
end
Expand Down
2 changes: 1 addition & 1 deletion lib/mini_profiler_rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def self.initialize!(app)
end

unless Rails.env.development? || Rails.env.test?
c.authorization_mode = :whitelist
c.authorization_mode = :allow_authorized
end

if Rails.logger
Expand Down
38 changes: 19 additions & 19 deletions spec/integration/mini_profiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def app
map '/html' do
run lambda { |env| [200, { 'Content-Type' => 'text/html' }, +"<html><BODY><h1>Hi</h1></BODY>\n \t</html>"] }
end
map '/whitelisted-html' do
map '/explicitly-allowed-html' do
run lambda { |env|
Rack::MiniProfiler.authorize_request
[200, { 'Content-Type' => 'text/html' }, +"<html><BODY><h1>Hi</h1></BODY>\n \t</html>"]
Expand All @@ -54,7 +54,7 @@ def app
[200, { 'Content-Type' => 'text/html' }, +'<h1>Hi</h1>']
}
end
map '/whitelisted' do
map '/explicitly-allowed' do
run lambda { |env|
Rack::MiniProfiler.authorize_request
[200, { 'Content-Type' => 'text/html' }, +'<h1>path1</h1>']
Expand Down Expand Up @@ -303,17 +303,17 @@ def load_prof(response)
expect(last_response.body).to include('/mini-profiler-resources/includes.js')
end

it "does not re-enable functionality if not whitelisted" do
Rack::MiniProfiler.config.authorization_mode = :whitelist
it "does not re-enable functionality if not explicitly allowed" do
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
get '/html?pp=enable'
get '/html?pp=enable'
expect(last_response.body).not_to include('/mini-profiler-resources/includes.js')
end

it "re-enabled functionality if whitelisted" do
Rack::MiniProfiler.config.authorization_mode = :whitelist
get '/whitelisted-html?pp=enable'
get '/whitelisted-html?pp=enable'
it "re-enabled functionality if explicitly allowed" do
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
get '/explicitly-allowed-html?pp=enable'
get '/explicitly-allowed-html?pp=enable'
expect(last_response.body).to include('/mini-profiler-resources/includes.js')
end
end
Expand All @@ -339,21 +339,21 @@ def load_prof(response)
end
end

describe 'authorization mode whitelist' do
describe 'authorization mode :allow_authorized' do
before do
Rack::MiniProfiler.config.authorization_mode = :whitelist
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
end

it "should ban requests that are not whitelisted" do
it "should ban requests that are not explicitly allowed" do
get '/html'
expect(last_response.headers['X-MiniProfiler-Ids']).to be_nil
end

it "should allow requests that are whitelisted" do
get '/whitelisted'
it "should allow requests that are explicitly allowed" do
get '/explicitly-allowed'
# second time will ensure cookie is set
# first time around there is no cookie, so no profiling
get '/whitelisted'
get '/explicitly-allowed'
expect(last_response.headers['X-MiniProfiler-Ids']).not_to be_nil
end
end
Expand Down Expand Up @@ -392,7 +392,7 @@ def load_prof(response)
context 'snapshots sampling' do
before(:each) do
Rack::MiniProfiler.config.tap do |c|
c.authorization_mode = :whitelist
c.authorization_mode = :allow_authorized
c.snapshot_every_n_requests = 1
end
end
Expand Down Expand Up @@ -429,12 +429,12 @@ def load_prof(response)

it 'does not take snapshots of requests that have valid token in cookie' do
Rack::MiniProfiler.config.pre_authorize_cb = lambda { |env| true }
get '/whitelisted-html'
get '/explicitly-allowed-html'
cookies = last_response.set_cookie_header
get '/path2/a', nil, { cookie: cookies } # no snapshot here
data = Rack::MiniProfiler.config.storage_instance.snapshot_groups_overview
expect(data.size).to eq(1)
expect(data[0][:name]).to eq("GET /whitelisted-html")
expect(data[0][:name]).to eq("GET /explicitly-allowed-html")
end

it 'respects snapshot_every_n_requests config' do
Expand Down Expand Up @@ -488,12 +488,12 @@ def load_prof(response)
context 'snapshots page' do
it 'allows only authorized users to access it' do
base_url = Rack::MiniProfiler.config.base_url_path
Rack::MiniProfiler.config.authorization_mode = :whitelist
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
get "#{base_url}snapshots"
expect(last_response.status).to eq(404)
expect(last_response.body).to eq("Not Found: /mini-profiler-resources/snapshots")

get '/whitelisted-html'
get '/explicitly-allowed-html'
cookies = last_response.set_cookie_header
get "#{base_url}snapshots", nil, { cookie: cookies }
expect(last_response.status).to eq(200)
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/client_settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@
end

it 'writes auth token for authorized reqs' do
Rack::MiniProfiler.config.authorization_mode = :whitelist
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
Rack::MiniProfiler.authorize_request
hash = {}
@settings.write!(hash)
expect(hash["Set-Cookie"]).to include(@store.allowed_tokens.join("|"))
end

it 'does nothing on short unauthed requests' do
Rack::MiniProfiler.config.authorization_mode = :whitelist
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
Rack::MiniProfiler.deauthorize_request
hash = {}
@settings.handle_cookie([200, hash, []])
Expand All @@ -58,7 +58,7 @@
end

it 'discards on long unauthed requests' do
Rack::MiniProfiler.config.authorization_mode = :whitelist
Rack::MiniProfiler.config.authorization_mode = :allow_authorized
Rack::MiniProfiler.deauthorize_request
hash = {}
clock_travel(Process.clock_gettime(Process::CLOCK_MONOTONIC) + 1) do
Expand Down
23 changes: 23 additions & 0 deletions spec/lib/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,28 @@ module Rack
end
end

describe 'authorization_mode' do
before do
@config = MiniProfiler::Config.default
end

it 'is false by default' do
expect(@config.authorization_mode).to be(:allow_all)
end

it 'is set to :allow_authorized when given :whitelist' do
expect { @config.authorization_mode = :whitelist }.to output(<<~DEP).to_stderr
[DEPRECATION] `:whitelist` authorization mode is deprecated. Please use `:allow_authorized` instead.
DEP

expect(@config.authorization_mode).to eq :allow_authorized
end

it 'emits deprecation warning if set to an unrecognized mode' do
expect { @config.authorization_mode = :unknown_mode }.to output(<<~DEP).to_stderr
[DEPRECATION] unknown authorization mode unknown_mode. Expected `:allow_all` or `:allow_authorized`.
DEP
end
end
end
end

0 comments on commit e99b44d

Please sign in to comment.