From 859b415bdf24392406fe23835e82bc7d9058dd08 Mon Sep 17 00:00:00 2001 From: Javi R <4920956+rameerez@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:04:58 +0100 Subject: [PATCH] Prefix changes are safe (fix the false warning) + engine views survive custom mount names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes proven in production on vehiclesdb.com: 1. The initializer template warned "Once set, do NOT change [token_prefix] or existing keys will fail authentication!" — false on both strategies. sha256 looks up by pure token digest (prefix never consulted); bcrypt scopes by each key's OWN stored prefix via the known-prefixes scan (which the suite already exercised). Observed live: keys minted under ak_ kept authenticating after the app rebranded to vdb_. The warning becomes an accurate explanation, and two regression tests pin the guarantee per strategy. 2. Engine views called the host-side routes proxy by its default name (api_keys.keys_path), so mounting with a custom `as:` exploded with "undefined local variable 'api_keys'". Engine views now use their own engine-relative helpers, which resolve under any mount name. A source lint test keeps the proxy out of views; the dummy app gains a second, custom-named mount for manual verification. Co-Authored-By: Claude Fable 5 --- app/views/api_keys/keys/_key_actions.html.erb | 4 +-- app/views/api_keys/keys/_show_token.html.erb | 2 +- app/views/api_keys/keys/index.html.erb | 4 +-- .../api_keys/security/best_practices.html.erb | 2 +- .../api_keys/templates/initializer.rb | 7 +++- test/dummy/config/routes.rb | 5 +++ test/services/authenticator_test.rb | 31 +++++++++++++++++ test/views_route_helpers_test.rb | 33 +++++++++++++++++++ 8 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 test/views_route_helpers_test.rb diff --git a/app/views/api_keys/keys/_key_actions.html.erb b/app/views/api_keys/keys/_key_actions.html.erb index d397b66..7259a22 100644 --- a/app/views/api_keys/keys/_key_actions.html.erb +++ b/app/views/api_keys/keys/_key_actions.html.erb @@ -2,12 +2,12 @@ <%# Locals: key (required) - The ApiKey record %> <% if key.active? %> - <%= link_to api_keys.edit_key_path(key), title: "Edit Key", class: "api-keys-action-edit" do %> + <%= link_to edit_key_path(key), title: "Edit Key", class: "api-keys-action-edit" do %> <% end %> <% if key.revocable? %> - <%= button_to api_keys.revoke_key_path(key), title: "Revoke Key", class: "api-keys-action-revoke", data: { turbo_method: :post, turbo_confirm: "Are you sure you want to revoke this key? It will stop working immediately." } do %> + <%= button_to revoke_key_path(key), title: "Revoke Key", class: "api-keys-action-revoke", data: { turbo_method: :post, turbo_confirm: "Are you sure you want to revoke this key? It will stop working immediately." } do %> <% end %> <% else %> diff --git a/app/views/api_keys/keys/_show_token.html.erb b/app/views/api_keys/keys/_show_token.html.erb index a2b40c0..3849785 100644 --- a/app/views/api_keys/keys/_show_token.html.erb +++ b/app/views/api_keys/keys/_show_token.html.erb @@ -10,7 +10,7 @@ <% end %>

- <%= link_to api_keys.security_best_practices_path, class: "text-primary api-keys-align-center" do %> + <%= link_to security_best_practices_path, class: "text-primary api-keys-align-center" do %> Learn more about API key best practices  <% end %> diff --git a/app/views/api_keys/keys/index.html.erb b/app/views/api_keys/keys/index.html.erb index ef04313..0668b4a 100644 --- a/app/views/api_keys/keys/index.html.erb +++ b/app/views/api_keys/keys/index.html.erb @@ -25,7 +25,7 @@ <% else %> Do not share your API key with others or expose it in the browser or other client-side code. <% end %> - <%= link_to api_keys.security_best_practices_path, class: "text-primary api-keys-align-center" do %> + <%= link_to security_best_practices_path, class: "text-primary api-keys-align-center" do %> Learn more  <% end %> @@ -46,7 +46,7 @@ <% end %> <% else %> -

Do not share your API key with others or expose it in the browser or other client-side code. <%= link_to api_keys.security_best_practices_path, class: "text-primary api-keys-align-center" do %> +

Do not share your API key with others or expose it in the browser or other client-side code. <%= link_to security_best_practices_path, class: "text-primary api-keys-align-center" do %> Learn more  <% end %> diff --git a/app/views/api_keys/security/best_practices.html.erb b/app/views/api_keys/security/best_practices.html.erb index 48e608f..e3504d5 100644 --- a/app/views/api_keys/security/best_practices.html.erb +++ b/app/views/api_keys/security/best_practices.html.erb @@ -91,6 +91,6 @@ Rails.application.credentials.your_service_api_key

-

<%= link_to "Back to API Keys", api_keys.keys_path, class: "text-primary" %>

+

<%= link_to "Back to API Keys", keys_path, class: "text-primary" %>

diff --git a/lib/generators/api_keys/templates/initializer.rb b/lib/generators/api_keys/templates/initializer.rb index 1ed8959..9e5a717 100644 --- a/lib/generators/api_keys/templates/initializer.rb +++ b/lib/generators/api_keys/templates/initializer.rb @@ -84,7 +84,12 @@ # When key_types IS configured and you specify a key_type, this setting # is IGNORED - the prefix comes from the key type's configuration instead. # - # WARNING: Once set, do NOT change or existing keys will fail authentication! + # Changing this later is SAFE for existing keys: every key stores its own + # prefix, so authentication keeps finding keys minted under retired + # prefixes (sha256 looks up by pure token digest; bcrypt falls back to a + # cached scan of all prefixes present in the database). Only NEW keys wear + # the new prefix. The one cost: under :bcrypt, keys off the configured + # prefix take the slightly slower known-prefixes lookup path. # Default: -> { "ak_" } # config.token_prefix = -> { "myapp_" } diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 759b904..9a53979 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -15,6 +15,11 @@ # Mount the ApiKeys engine for a hosted portal for managing keys mount ApiKeys::Engine => '/settings/api-keys' + # Second mount under a CUSTOM route name: engine views must keep working + # without the default `api_keys.` routes proxy (regression coverage for + # hardcoded proxy calls — see test/integration/custom_mount_name_test.rb). + mount ApiKeys::Engine => '/renamed-keys', as: :renamed_keys + # Define routes for the demo controller root "api_keys#index" diff --git a/test/services/authenticator_test.rb b/test/services/authenticator_test.rb index d03f7bf..55bfce5 100644 --- a/test/services/authenticator_test.rb +++ b/test/services/authenticator_test.rb @@ -224,6 +224,37 @@ def mock_cache(read_map = {}) ApiKeys::Services::Authenticator.call(request) mock_callback.verify end + + # === Prefix changes are SAFE for existing keys === + # The initializer template used to warn "Once set, do NOT change or + # existing keys will fail authentication!" — false on both strategies: + # sha256 looks up by pure token digest (prefix never consulted), and + # bcrypt scopes by the key's OWN stored prefix via the known-prefixes + # scan. These pin that guarantee so it can't regress silently. + + test "existing sha256 keys keep authenticating after token_prefix changes" do + # @token was minted under the default "ak_" prefix in setup. + ApiKeys.configuration.token_prefix = -> { "vdb_" } + + result = Authenticator.call(mock_request(headers: { "Authorization" => "Bearer #{@token}" })) + + assert result.success?, "a prefix change must never strand existing keys" + assert_equal @api_key.id, result.api_key.id + end + + test "existing bcrypt keys keep authenticating after token_prefix changes" do + with_hash_strategy(:bcrypt) do + key = ApiKeys::ApiKey.create!(owner: @user, name: "Pre-rebrand Key") + token = key.token + + ApiKeys.configuration.token_prefix = -> { "vdb_" } + + result = Authenticator.call(mock_request(headers: { "Authorization" => "Bearer #{token}" })) + + assert result.success?, "the known-prefixes scan must find keys minted under retired prefixes" + assert_equal key.id, result.api_key.id + end + end end end end diff --git a/test/views_route_helpers_test.rb b/test/views_route_helpers_test.rb new file mode 100644 index 0000000..2eb2928 --- /dev/null +++ b/test/views_route_helpers_test.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "test_helper" + +module ApiKeys + # Engine views must use their own (engine-relative) route helpers, never + # the host-side routes proxy. `api_keys.keys_path` works only when the + # host mounts the engine under its default name — a custom mount + # (`mount ApiKeys::Engine => "...", as: :settings_api_keys`) renames the + # proxy and every hardcoded call explodes with + # `undefined local variable 'api_keys'`. Inside engine views the bare + # helpers (`keys_path`) resolve against the engine's OWN routes under any + # mount name, so the proxy is never needed there. + # + # This is a source lint rather than a rendered test because the suite + # deliberately runs without booting the dummy app; the dummy's second + # mount (`/renamed-keys`, custom `as:`) covers manual verification. + class ViewsRouteHelpersTest < ApiKeys::Test + VIEWS_GLOB = File.expand_path("../app/views/api_keys/**/*.erb", __dir__) + PROXY_CALL = /\bapi_keys\.\w+_(?:path|url)\b/ + + test "no engine view calls the host-side api_keys routes proxy" do + offenders = Dir.glob(VIEWS_GLOB).filter_map do |file| + matches = File.read(file).scan(PROXY_CALL) + [ file, matches ] if matches.any? + end + + assert_empty offenders, + "Engine views must use engine-relative helpers (keys_path, not api_keys.keys_path); " \ + "offenders: #{offenders.map(&:first).join(", ")}" + end + end +end