Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Changelog

## Unreleased

- Fix: the OIDC `filter_parameters` entries now match exact keys instead of substrings. `code_verifier`, `id_token`, `access_token` and `refresh_token` are filtered at any depth; `code`, `state`, `session_state` and `nonce` only at the top level. Host params such as `code_id`, `state_eq` or `order[state]` are no longer filtered from logs. If you relied on the old substring match to hide params like `invite_code` or `reset_code`, add them to your own `filter_parameters`.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ The gem's Rails engine handles several things so host apps don't have to:
* **Login view override** — the engine prepends an SSO-only login page (no email/password fields) to the sessions controller's view path. If your host app ships its own `app/views/active_admin/devise/sessions/new.html.erb`, the gem detects it and backs off — your view wins.
* **Session routes** — the engine mounts `GET /admin/login` (renders the SSO landing page) and `DELETE /admin/logout` under `devise_scope`, with the scope name derived from `config.admin_user_class`. Devise normally generates session routes as a side effect of `:database_authenticatable`; without that module the route helpers would not exist and ActiveAdmin's login redirect would 404.
* **Path prefix** — the engine registers the strategy with `path_prefix: '/admin/auth'` so the middleware intercepts requests under ActiveAdmin's mount point, and sets `Devise.omniauth_path_prefix` to the prefix Devise declares its routes with. Compatible with Rails 7.2+ and Rails 8's lazy route loading.
* **Parameter filtering** — `code`, `id_token`, `access_token`, `refresh_token`, `state`, and `nonce` are added to `Rails.application.config.filter_parameters`.
* **Parameter filtering** — the OIDC keys are added to `Rails.application.config.filter_parameters` as exact-key matches. `code_verifier`, `id_token`, `access_token` and `refresh_token` are filtered at any depth. The generic `code`, `state`, `session_state` and `nonce` are filtered only as top-level params, where the callback sends them, so host params like `code_id`, `state_eq` or `order[state]` stay visible.

## Configuration

Expand Down Expand Up @@ -410,7 +410,7 @@ The gem also adds a unique `(provider, uid)` partial index in its own install mi

### What's filtered from logs

The engine merges `code`, `id_token`, `access_token`, `refresh_token`, `state`, and `nonce` into `Rails.application.config.filter_parameters` so a mid-callback crash can't dump them into production logs. Your own `filter_parameters` entries are preserved.
The engine adds the OIDC keys to `Rails.application.config.filter_parameters`, so a mid-callback crash can't dump them into production logs. Keys are matched exactly. `code_verifier`, `id_token`, `access_token` and `refresh_token` are filtered at any depth; `code`, `state`, `session_state` and `nonce` only as top-level params. So `code_id`, `state_eq` and nested params like `order[state]` are not filtered. Your own `filter_parameters` entries are preserved.

## Logger

Expand Down
9 changes: 8 additions & 1 deletion lib/activeadmin/oidc/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,14 @@ def controllers
end

initializer 'activeadmin_oidc.filter_parameters' do |app|
app.config.filter_parameters |= %i[code id_token access_token refresh_token state nonce]
# ActiveSupport::ParameterFilter matches a regexp containing "\." against the dotted
# full key (data.attributes.state), so the generic callback names stay visible when nested.
callback_params = %w[code state session_state nonce]
token_params = %w[code_verifier id_token access_token refresh_token]
app.config.filter_parameters |= [
/\A(?!.*\.)(?:#{Regexp.union(callback_params).source})\z/i,
/\A(?:#{Regexp.union(token_params).source})\z/i
]
end

# The gem is OIDC-first: mount our SSO landing page at /admin/login
Expand Down
27 changes: 16 additions & 11 deletions spec/security_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,23 @@
# out of the Rails log is a liability — a crash mid-callback will
# dump the whole params hash (including `code`, `id_token`,
# `access_token`, `refresh_token`) straight into production logs.
#
# Note: once Rails has served a request, filter_parameters is
# compiled into a single combined regex (e.g.
# "(?-mix:(?i:code)|(?i:id_token)|(?i:access_token)|...)"
# ), so we stringify the collection and look for each key in it
# rather than asserting on discrete symbol entries.
it "includes the OIDC token/code/state/nonce keys so they never end up in Rails logs" do
filters_str = Rails.application.config.filter_parameters.map(&:to_s).join(" ")
let(:oidc_keys) { %w[code code_verifier state session_state nonce id_token access_token refresh_token] }
let(:host_params) do
{ "code_id" => "1", "state_eq" => "2", "data" => { "attributes" => { "state" => "shipped", "code" => "X" } } }
end
let(:nested_tokens) { %w[code_verifier id_token access_token refresh_token] }

[
["raw", ->(filters) { filters }],
["precompiled", ->(filters) { ActiveSupport::ParameterFilter.precompile_filters(filters) }]
].each do |form, compile|
it "filters OIDC keys and nested tokens, and leaves host params visible (#{form} filters)" do
filter = ActiveSupport::ParameterFilter.new(compile.call(Rails.application.config.filter_parameters))
params = oidc_keys.index_with("secret").merge(host_params, "auth" => nested_tokens.index_with("secret"))
result = filter.filter(params)

%w[code id_token access_token refresh_token state nonce].each do |key|
expect(filters_str).to include(key),
"expected filter_parameters to filter #{key.inspect}, got: #{filters_str}"
expect(result).to eq(oidc_keys.index_with("[FILTERED]")
.merge(host_params, "auth" => nested_tokens.index_with("[FILTERED]")))
end
end
end
Expand Down
Loading