diff --git a/app/controllers/settings/api_keys_controller.rb b/app/controllers/settings/api_keys_controller.rb index cb8b2c3db..ac5d53aa4 100644 --- a/app/controllers/settings/api_keys_controller.rb +++ b/app/controllers/settings/api_keys_controller.rb @@ -5,18 +5,24 @@ class Settings::ApiKeysController < ApplicationController before_action :set_api_key, only: [ :show, :destroy ] - def show + def index + @api_keys = Current.user.api_keys.active.visible.order(created_at: :desc) @breadcrumbs = [ [ t("breadcrumbs.home"), root_path ], - [ t("breadcrumbs.api_key"), nil ] + [ t("breadcrumbs.api_keys"), nil ] + ] + end + + def show + @newly_created = params[:newly_created].present? + @breadcrumbs = [ + [ t("breadcrumbs.home"), root_path ], + [ t("breadcrumbs.api_keys"), settings_api_keys_path ], + [ @api_key.name, nil ] ] - @current_api_key = @api_key end def new - # Allow regeneration by not redirecting if user explicitly wants to create a new key - # Only redirect if user stumbles onto new page without explicit intent - redirect_to settings_api_key_path if Current.user.api_keys.active.visible.exists? && !params[:regenerate] @api_key = ApiKey.new end @@ -25,42 +31,33 @@ class Settings::ApiKeysController < ApplicationController @api_key = Current.user.api_keys.build(api_key_params) @api_key.key = @plain_key - # Temporarily revoke existing visible keys for validation to pass - # (demo monitoring key is excluded and remains active) - existing_keys = Current.user.api_keys.active.visible - existing_keys.each { |key| key.update_column(:revoked_at, Time.current) } - if @api_key.save flash[:notice] = t(".success") - redirect_to settings_api_key_path + redirect_to settings_api_key_path(@api_key, newly_created: true) else - # Restore existing keys if new key creation failed - existing_keys.each { |key| key.update_column(:revoked_at, nil) } render :new, status: :unprocessable_entity end end def destroy - if @api_key.nil? - flash[:alert] = t(".not_found") - elsif @api_key.demo_monitoring_key? - flash[:alert] = t(".cannot_revoke") - elsif @api_key.revoke! - flash[:notice] = t(".revoked_successfully") - else - flash[:alert] = t(".revoke_failed") - end - redirect_to settings_api_key_path + @api_key.revoke! + flash[:notice] = t(".revoked_successfully") + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotDestroyed + flash[:alert] = t(".revoke_failed") + ensure + redirect_to settings_api_keys_path end private + # `.visible` excludes the demo monitoring key, so a demo key id 404s here + # before #destroy can revoke it — this is intentional (see the SECURITY note + # on ApiKey's `visible` scope). def set_api_key - @api_key = Current.user.api_keys.active.visible.first + @api_key = Current.user.api_keys.active.visible.find(params[:id]) end def api_key_params - # Convert single scope value to array for storage permitted_params = params.require(:api_key).permit(:name, :scopes) if permitted_params[:scopes].present? permitted_params[:scopes] = [ permitted_params[:scopes] ] diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index 6d821ecdb..658b9199d 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -18,7 +18,7 @@ module SettingsHelper # Advanced section { name: -> { t("settings.settings_nav.ai_prompts_label") }, path: :settings_ai_prompts_path, condition: :admin_user? }, { name: -> { t("settings.settings_nav.llm_usage_label") }, path: :settings_llm_usage_path, condition: :admin_user? }, - { name: -> { t("settings.settings_nav.api_key_label") }, path: :settings_api_key_path, condition: :admin_user? }, + { name: -> { t("settings.settings_nav.api_key_label") }, path: :settings_api_keys_path, condition: :admin_user? }, { name: -> { t("settings.settings_nav.self_hosting_label") }, path: :settings_hosting_path, condition: :self_hosted_and_admin? }, { name: -> { t("settings.settings_nav.imports_label") }, path: :imports_path, condition: :admin_user? }, { name: -> { t("settings.settings_nav.exports_label") }, path: :family_exports_path, condition: :admin_user? }, diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 4c12b4668..1b5543763 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -18,7 +18,7 @@ class ApiKey < ApplicationRecord validates :scopes, presence: true validates :source, presence: true, inclusion: { in: SOURCES } validate :scopes_not_empty - validate :one_active_key_per_user_per_source, on: :create + validate :name_unique_among_active_keys, on: :create # Callbacks before_validation :set_display_key @@ -26,6 +26,10 @@ class ApiKey < ApplicationRecord # Scopes scope :active, -> { where(revoked_at: nil).where("expires_at IS NULL OR expires_at > ?", Time.current) } + # SECURITY: excluding the demo monitoring key here is also the revocation guard + # in Settings::ApiKeysController#destroy — a demo key id 404s in `set_api_key` + # (it is not `.visible`) before it can be revoked. Do NOT broaden this scope to + # include monitoring keys without adding another explicit destroy guard. scope :visible, -> { where.not(display_key: DEMO_MONITORING_KEY) } # Class methods @@ -103,10 +107,11 @@ class ApiKey < ApplicationRecord end end - def one_active_key_per_user_per_source - if user&.api_keys&.active&.where(source: source)&.where&.not(id: id)&.exists? - errors.add(:user, "can only have one active API key per source (#{source})") - end + def name_unique_among_active_keys + return if name.blank? || user.blank? + scope = user.api_keys.active.visible.where(name: name) + scope = scope.where.not(id: id) if persisted? + errors.add(:name, :taken) if scope.exists? end def prevent_demo_monitoring_key_destroy! diff --git a/app/views/reports/google_sheets_instructions.html.erb b/app/views/reports/google_sheets_instructions.html.erb index ba8e79a9f..3eb8f4bb2 100644 --- a/app/views/reports/google_sheets_instructions.html.erb +++ b/app/views/reports/google_sheets_instructions.html.erb @@ -51,7 +51,7 @@ text: t("reports.google_sheets_instructions.go_to_api_keys"), variant: "primary", full_width: true, - href: settings_api_key_path, + href: settings_api_keys_path, frame: "_top" ) %> <% end %> diff --git a/app/views/settings/_settings_nav.html.erb b/app/views/settings/_settings_nav.html.erb index b65101ead..2af887c63 100644 --- a/app/views/settings/_settings_nav.html.erb +++ b/app/views/settings/_settings_nav.html.erb @@ -29,7 +29,7 @@ nav_sections = [ items: [ { label: t(".ai_prompts_label"), path: settings_ai_prompts_path, icon: "bot" }, { label: t(".llm_usage_label"), path: settings_llm_usage_path, icon: "activity" }, - { label: t(".api_keys_label"), path: settings_api_key_path, icon: "key" }, + { label: t(".api_keys_label"), path: settings_api_keys_path, icon: "key" }, { label: t(".mcp_label"), path: settings_mcp_path, icon: "plug" }, { label: t(".debug_label", default: "Debug"), path: settings_debug_path, icon: "bug", if: Current.user&.super_admin? }, { label: t(".self_hosting_label"), path: settings_hosting_path, icon: "database", if: self_hosted? }, diff --git a/app/views/settings/api_keys/_key_meta.html.erb b/app/views/settings/api_keys/_key_meta.html.erb new file mode 100644 index 000000000..df515e7dc --- /dev/null +++ b/app/views/settings/api_keys/_key_meta.html.erb @@ -0,0 +1,9 @@ +<%# locals: (api_key:) %> +

+ <%= t("settings.api_keys.shared.created_ago", time: time_ago_in_words(api_key.created_at)) %> + <% if api_key.last_used_at %> + • <%= t("settings.api_keys.shared.last_used_ago", time: time_ago_in_words(api_key.last_used_at)) %> + <% else %> + • <%= t("settings.api_keys.shared.never_used") %> + <% end %> +

diff --git a/app/views/settings/api_keys/_key_reveal.html.erb b/app/views/settings/api_keys/_key_reveal.html.erb new file mode 100644 index 000000000..8852b0021 --- /dev/null +++ b/app/views/settings/api_keys/_key_reveal.html.erb @@ -0,0 +1,17 @@ +<%# locals: (api_key:, heading:, description:, copy_label:) %> +
+

<%= heading %>

+

<%= description %>

+ +
+
+ <%= api_key.plain_key %> + <%= render DS::Button.new( + text: copy_label, + variant: "ghost", + icon: "copy", + data: { action: "clipboard#copy" } + ) %> +
+
+
diff --git a/app/views/settings/api_keys/_scope_badges.html.erb b/app/views/settings/api_keys/_scope_badges.html.erb new file mode 100644 index 000000000..bb7ef4ecd --- /dev/null +++ b/app/views/settings/api_keys/_scope_badges.html.erb @@ -0,0 +1,11 @@ +<%# locals: (scopes:) %> +
+ <% scopes.each do |scope| %> + <% label = case scope + when "read" then t("settings.api_keys.shared.scope_read_only") + when "read_write" then t("settings.api_keys.shared.scope_read_write") + else t("settings.api_keys.shared.scopes.#{scope}", default: scope.humanize) + end %> + <%= render DS::Pill.new(label: label, icon: "shield-check", tone: :indigo, style: :soft, marker: false) %> + <% end %> +
diff --git a/app/views/settings/api_keys/_status_indicator.html.erb b/app/views/settings/api_keys/_status_indicator.html.erb new file mode 100644 index 000000000..ec4bedc3e --- /dev/null +++ b/app/views/settings/api_keys/_status_indicator.html.erb @@ -0,0 +1 @@ +<%= render DS::Pill.new(label: t("settings.api_keys.shared.active"), tone: :success, show_dot: true, marker: false) %> diff --git a/app/views/settings/api_keys/_usage.html.erb b/app/views/settings/api_keys/_usage.html.erb new file mode 100644 index 000000000..2b1f41602 --- /dev/null +++ b/app/views/settings/api_keys/_usage.html.erb @@ -0,0 +1,8 @@ +<%# locals: (api_key:, title:, description:) %> +
+

<%= title %>

+

<%= description %>

+
+ curl -H "X-Api-Key: <%= api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts +
+
diff --git a/app/views/settings/api_keys/created.html.erb b/app/views/settings/api_keys/created.html.erb deleted file mode 100644 index d0c4e7bca..000000000 --- a/app/views/settings/api_keys/created.html.erb +++ /dev/null @@ -1,87 +0,0 @@ -<%= content_for :page_title, t(".page_title") %> - -<%= settings_section title: t(".success_title"), subtitle: t(".success_description") do %> -
-
-
- <%= render DS::FilledIcon.new( - icon: "check-circle", - rounded: true, - size: "lg", - variant: :success - ) %> -
-

<%= t(".success_title") %>

-

<%= t(".key_ready", name: @api_key.name) %>

-
-
-
- -
-

<%= t(".your_api_key") %>

-

<%= t(".copy_store_securely") %>

- -
-
- <%= @api_key.plain_key %> - <%= render DS::Button.new( - text: t(".copy_key"), - variant: "ghost", - icon: "copy", - data: { action: "clipboard#copy" } - ) %> -
-
-
- -
-

<%= t(".key_details_title") %>

-
-
- <%= t(".key_name_label") %> - <%= @api_key.name %> -
-
- <%= t(".permissions_label") %> - - <%= @api_key.scopes.map { |scope| - case scope - when "read_accounts" then t("settings.api_keys_controller.scope_descriptions.read_accounts") - when "read_transactions" then t("settings.api_keys_controller.scope_descriptions.read_transactions") - when "read_balances" then t("settings.api_keys_controller.scope_descriptions.read_balances") - when "write_transactions" then t("settings.api_keys_controller.scope_descriptions.write_transactions") - else scope.humanize - end - }.join(", ") %> - -
-
- <%= t(".created_label") %> - <%= @api_key.created_at.strftime("%B %d, %Y at %I:%M %p") %> -
-
-
- - <%= render DS::Alert.new(title: t(".security_note_title"), variant: :warning) do %> -

- <%= t(".security_note_body") %> -

- <% end %> - -
-

<%= t(".usage_instructions_title") %>

-

<%= t("settings.api_keys.show.current_api_key.usage_instructions", product_name: product_name) %>

-
- curl -H "X-Api-Key: <%= @api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts -
-
- -
- <%= render DS::Link.new( - text: t(".continue"), - href: settings_api_key_path, - variant: "primary" - ) %> -
-
-<% end %> diff --git a/app/views/settings/api_keys/created.turbo_stream.erb b/app/views/settings/api_keys/created.turbo_stream.erb deleted file mode 100644 index f187a6d31..000000000 --- a/app/views/settings/api_keys/created.turbo_stream.erb +++ /dev/null @@ -1,95 +0,0 @@ -<%= turbo_stream.update "main" do %> -
-
-

- <%= t("settings.api_keys.created.page_title") %> -

- - <%= settings_section title: t("settings.api_keys.created.success_title"), subtitle: t("settings.api_keys.created.success_description") do %> -
-
-
- <%= render DS::FilledIcon.new( - icon: "check-circle", - rounded: true, - size: "lg", - variant: :success - ) %> -
-

<%= t("settings.api_keys.created.success_title") %>

-

<%= t("settings.api_keys.created.key_ready", name: @api_key.name) %>

-
-
-
- -
-

<%= t("settings.api_keys.created.your_api_key") %>

-

<%= t("settings.api_keys.created.copy_store_securely") %>

- -
-
- <%= @api_key.plain_key %> - <%= render DS::Button.new( - text: t("settings.api_keys.created.copy_key"), - variant: "ghost", - icon: "copy", - data: { action: "clipboard#copy" } - ) %> -
-
-
- -
-

<%= t("settings.api_keys.created.key_details_title") %>

-
-
- <%= t("settings.api_keys.created.key_name_label") %> - <%= @api_key.name %> -
-
- <%= t("settings.api_keys.created.permissions_label") %> - - <%= @api_key.scopes.map { |scope| - case scope - when "read_accounts" then t("settings.api_keys_controller.scope_descriptions.read_accounts") - when "read_transactions" then t("settings.api_keys_controller.scope_descriptions.read_transactions") - when "read_balances" then t("settings.api_keys_controller.scope_descriptions.read_balances") - when "write_transactions" then t("settings.api_keys_controller.scope_descriptions.write_transactions") - else scope.humanize - end - }.join(", ") %> - -
-
- <%= t("settings.api_keys.created.created_label") %> - <%= @api_key.created_at.strftime("%B %d, %Y at %I:%M %p") %> -
-
-
- - <%= render DS::Alert.new(title: t("settings.api_keys.created.security_note_title"), variant: :warning) do %> -

- <%= t("settings.api_keys.created.security_note_body") %> -

- <% end %> - -
-

<%= t("settings.api_keys.created.usage_instructions_title") %>

-

<%= t("settings.api_keys.show.current_api_key.usage_instructions", product_name: product_name) %>

-
- curl -H "X-Api-Key: <%= @api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts -
-
- -
- <%= render DS::Link.new( - text: t("settings.api_keys.created.continue"), - href: settings_api_key_path, - variant: "primary" - ) %> -
-
- <% end %> -
-
-<% end %> diff --git a/app/views/settings/api_keys/index.html.erb b/app/views/settings/api_keys/index.html.erb new file mode 100644 index 000000000..7a6cbd2a4 --- /dev/null +++ b/app/views/settings/api_keys/index.html.erb @@ -0,0 +1,72 @@ +<%= content_for :page_title, t(".title") %> +<% unless @api_keys.empty? %> + <%= content_for :page_actions do %> + <%= render DS::Link.new( + text: t(".new_key"), + href: new_settings_api_key_path, + variant: "primary", + icon: "plus" + ) %> + <% end %> +<% end %> + +<% if @api_keys.empty? %> +
+
+
+ <%= render DS::FilledIcon.new( + icon: "key", + rounded: true, + size: "lg" + ) %> +
+

<%= t(".empty_heading") %>

+

<%= t(".empty_description") %>

+
+ <%= render DS::Link.new( + text: t(".new_key"), + href: new_settings_api_key_path, + variant: "primary", + icon: "plus" + ) %> +
+
+
+<% else %> +
+ <% @api_keys.each do |api_key| %> +
+
+
+ <%= render DS::FilledIcon.new( + icon: "key", + rounded: true, + size: "lg" + ) %> +
+ <%= link_to api_key.name, settings_api_key_path(api_key), class: "text-primary font-medium hover:underline break-all" %> + <%= render "settings/api_keys/key_meta", api_key: api_key %> +
+ <%= render "settings/api_keys/scope_badges", scopes: api_key.scopes %> +
+
+
+ +
+ <%= render "settings/api_keys/status_indicator" %> + <%= render DS::Button.new( + text: t(".revoke_key"), + href: settings_api_key_path(api_key), + method: :delete, + variant: "destructive", + size: "sm", + data: { + turbo_confirm: t(".revoke_confirmation", name: api_key.name) + } + ) %> +
+
+
+ <% end %> +
+<% end %> diff --git a/app/views/settings/api_keys/new.html.erb b/app/views/settings/api_keys/new.html.erb index d37a24f70..656640436 100644 --- a/app/views/settings/api_keys/new.html.erb +++ b/app/views/settings/api_keys/new.html.erb @@ -1,7 +1,7 @@ <%= content_for :page_title, t(".create_new_api_key") %> <%= settings_section title: nil, subtitle: t(".subtitle") do %> - <%= styled_form_with model: @api_key, url: settings_api_key_path, class: "space-y-4" do |form| %> + <%= styled_form_with model: @api_key, url: settings_api_keys_path, class: "space-y-4" do |form| %> <%= form.text_field :name, placeholder: t(".name_placeholder"), label: t(".name_label"), @@ -39,7 +39,7 @@
<%= render DS::Link.new( text: t(".cancel"), - href: settings_api_key_path, + href: settings_api_keys_path, variant: "ghost" ) %> diff --git a/app/views/settings/api_keys/show.html.erb b/app/views/settings/api_keys/show.html.erb index c32e87349..f27dc4166 100644 --- a/app/views/settings/api_keys/show.html.erb +++ b/app/views/settings/api_keys/show.html.erb @@ -1,4 +1,13 @@ -<% if @newly_created && @plain_key %> +<%= content_for :page_actions do %> + <%= render DS::Link.new( + text: t(".current_api_key.back_to_keys"), + href: settings_api_keys_path, + variant: "secondary", + icon: "chevron-left" + ) %> +<% end %> + +<% if @newly_created %> <%= content_for :page_title, t(".newly_created.page_title") %>
@@ -13,54 +22,33 @@ ) %>

<%= t(".newly_created.heading") %>

-

<%= t(".newly_created.key_ready", name: @current_api_key.name) %>

+

<%= t(".newly_created.key_ready", name: @api_key.name) %>

-
-

<%= t(".newly_created.your_api_key") %>

-

<%= t(".newly_created.copy_store_securely") %>

+ <%= render "settings/api_keys/key_reveal", + api_key: @api_key, + heading: t(".newly_created.your_api_key"), + description: t(".newly_created.copy_store_securely"), + copy_label: t(".newly_created.copy_api_key") %> -
-
- <%= @current_api_key.plain_key %> - <%= render DS::Button.new( - text: t(".newly_created.copy_api_key"), - variant: "ghost", - icon: "copy", - data: { action: "clipboard#copy" } - ) %> -
-
-
- -
-

<%= t(".newly_created.how_to_use") %>

-

<%= t(".current_api_key.usage_instructions", product_name: product_name) %>

-
- curl -H "X-Api-Key: <%= @current_api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts -
-
+ <%= render "settings/api_keys/usage", + api_key: @api_key, + title: t(".newly_created.how_to_use"), + description: t(".current_api_key.usage_instructions", product_name: product_name) %>
<%= render DS::Link.new( text: t(".newly_created.continue"), - href: settings_api_key_path, + href: settings_api_keys_path, variant: "primary" ) %>
-<% elsif @current_api_key %> +<% else %> <%= content_for :page_title, t(".current_api_key.title") %> - <%= content_for :page_actions do %> - <%= render DS::Link.new( - text: t(".current_api_key.regenerate_key"), - href: new_settings_api_key_path(regenerate: true), - variant: "secondary" - ) %> - <% end %>
@@ -73,68 +61,34 @@ ) %>
-

<%= @current_api_key.name %>

-

- <%= t(".current_api_key.created_ago", time: time_ago_in_words(@current_api_key.created_at)) %> - <% if @current_api_key.last_used_at %> - • <%= t(".current_api_key.last_used_ago", time: time_ago_in_words(@current_api_key.last_used_at)) %> - <% else %> - • <%= t(".current_api_key.never_used") %> - <% end %> -

+

<%= @api_key.name %>

+ <%= render "settings/api_keys/key_meta", api_key: @api_key %>
-
-

<%= t(".current_api_key.active") %>

-
+ <%= render "settings/api_keys/status_indicator" %>

<%= t(".current_api_key.permissions") %>

-
- <% @current_api_key.scopes.each do |scope| %> - - <%= icon("shield-check", class: "w-3 h-3") %> - <%= case scope - when "read" then t(".current_api_key.scope_read_only") - when "read_write" then t(".current_api_key.scope_read_write") - else scope.humanize - end %> - - <% end %> -
+ <%= render "settings/api_keys/scope_badges", scopes: @api_key.scopes %>
-
-

<%= t(".current_api_key.title") %>

-

<%= t(".current_api_key.copy_store_securely") %>

+ <%= render "settings/api_keys/key_reveal", + api_key: @api_key, + heading: t(".current_api_key.title"), + description: t(".current_api_key.copy_store_securely"), + copy_label: t(".current_api_key.copy_api_key") %> -
-
- <%= @current_api_key.plain_key %> - <%= render DS::Button.new( - text: t(".current_api_key.copy_api_key"), - variant: "ghost", - icon: "copy", - data: { action: "clipboard#copy" } - ) %> -
-
-
- -
-

<%= t(".current_api_key.usage_instructions_title") %>

-

<%= t(".current_api_key.usage_instructions", product_name: product_name) %>

-
- curl -H "X-Api-Key: <%= @current_api_key.plain_key %>" <%= request.base_url %>/api/v1/accounts -
-
+ <%= render "settings/api_keys/usage", + api_key: @api_key, + title: t(".current_api_key.usage_instructions_title"), + description: t(".current_api_key.usage_instructions", product_name: product_name) %>
<%= render DS::Button.new( text: t(".current_api_key.revoke_key"), - href: settings_api_key_path, + href: settings_api_key_path(@api_key), method: :delete, variant: "destructive", data: { @@ -144,54 +98,4 @@
-<% else %> - <%= content_for :page_title, t(".no_api_key.title") %> - <%= content_for :page_actions do %> - <%= render DS::Link.new( - text: t(".no_api_key.create_api_key"), - href: new_settings_api_key_path, - variant: "primary" - ) %> - <% end %> - -
-
-
-
- <%= render DS::FilledIcon.new( - icon: "key", - rounded: true, - size: "lg" - ) %> -
-

<%= t(".no_api_key.heading", product_name: product_name) %>

-

<%= t(".no_api_key.description") %>

-
-
-
- -
-

<%= t(".no_api_key.what_you_can_do") %>

-
    -
  • - <%= icon("check", class: "w-4 h-4 text-primary mt-0.5") %> - <%= t(".no_api_key.feature_1") %> -
    • -
  • - <%= icon("check", class: "w-4 h-4 text-primary mt-0.5") %> - <%= t(".no_api_key.feature_2") %> -
    • -
  • - <%= icon("check", class: "w-4 h-4 text-primary mt-0.5") %> - <%= t(".no_api_key.feature_3") %> -
    • -
-
- -
-

<%= t(".no_api_key.security_note_title") %>

-

<%= t(".no_api_key.security_note") %>

-
-
-
<% end %> diff --git a/config/locales/views/settings/api_keys/en.yml b/config/locales/views/settings/api_keys/en.yml index ebc072b88..c9fd6eba5 100644 --- a/config/locales/views/settings/api_keys/en.yml +++ b/config/locales/views/settings/api_keys/en.yml @@ -5,32 +5,29 @@ en: success: "Your API key has been created successfully" revoked_successfully: "API key has been revoked successfully" revoke_failed: "Failed to revoke API key" - scope_descriptions: - read_accounts: "View Accounts" - read_transactions: "View Transactions" - read_balances: "View Balances" - write_transactions: "Create Transactions" api_keys: create: success: "Your API key has been created successfully" destroy: - not_found: "API key not found" - cannot_revoke: "This API key cannot be revoked" revoked_successfully: "API key has been revoked successfully" revoke_failed: "Failed to revoke API key" + shared: + active: "Active" + created_ago: "Created %{time} ago" + last_used_ago: "Last used %{time} ago" + never_used: "Never used" + scope_read_only: "Read Only" + scope_read_write: "Read/Write" + index: + title: "API Keys" + subtitle: "Manage API keys for programmatic access to your data." + new_key: "New API Key" + empty_heading: "No API keys yet" + empty_description: "Create an API key to access your data programmatically." + revoke_key: "Revoke" + revoke_confirmation: "Are you sure you want to revoke \"%{name}\"? This will immediately disable any application using this key." show: title: "API Key Management" - no_api_key: - title: "API Key" - heading: "Access your account data programmatically" - description: "Get programmatic access to your Sure data with a secure API key." - what_you_can_do: "What you can do with the API:" - feature_1: "Access your account data programmatically" - feature_2: "Build custom integrations and applications" - feature_3: "Automate data retrieval and analysis" - security_note_title: "Security First" - security_note: "Your API key will have restricted permissions based on the scopes you select. You can only have one active API key at a time." - create_api_key: "Create API Key" newly_created: page_title: "API Key Created Successfully" heading: "API Key Created Successfully!" @@ -42,25 +39,12 @@ en: continue: "Continue to API Key Settings" current_api_key: title: "Your API Key" - description: "Your active API key is ready to use. Keep it secure and never share it publicly." - active: "Active" - key_name: "Name" - created_at: "Created" - created_ago: "Created %{time} ago" - last_used: "Last Used" - last_used_ago: "Last used %{time} ago" - expires: "Expires" - ago: "ago" - never_used: "Never used" - never_expires: "Never expires" permissions: "Permissions" - scope_read_only: "Read Only" - scope_read_write: "Read/Write" copy_api_key: "Copy API Key" copy_store_securely: "Copy and store this key securely. You'll need it to authenticate your API requests." usage_instructions_title: "How to use your API key" usage_instructions: "Include your API key in the X-Api-Key header when making requests to the %{product_name} API:" - regenerate_key: "Create New Key" + back_to_keys: "Back to API Keys" revoke_key: "Revoke Key" revoke_confirmation: "Are you sure you want to revoke this API key? This action cannot be undone and will immediately disable all applications using this key." new: @@ -88,28 +72,4 @@ en: security_warning: "Your API key will be shown only once after creation. Store it securely and never share it publicly. If you lose it, you'll need to create a new one." create_key: "Create API Key" cancel: "Cancel" - save_api_key: "Save API Key" - created: - page_title: "API Key Created" - title: "API Key Created" - success_title: "API Key Created Successfully" - success_description: "Your new API key has been generated successfully." - key_ready: "Your new API key \"%{name}\" has been created and is ready to use." - your_api_key: "Your API Key" - copy_store_securely: "Copy and store this key securely. You'll need it to authenticate your API requests." - key_details_title: "Key Details" - key_name_label: "Name:" - permissions_label: "Permissions:" - created_label: "Created:" - security_note_title: "Important Security Note" - security_note_body: "This is the only time your API key will be displayed. Make sure to copy it now and store it securely. If you lose this key, you'll need to generate a new one." - usage_instructions_title: "How to use your API key" - key_name: "Name" - permissions: "Permissions" - critical_warning_title: "Critical: Save Your API Key Now" - critical_warning_1: "This is the only time you'll see your API key in plain text." - critical_warning_2: "Copy and store it securely in your password manager or application." - critical_warning_3: "If you lose this key, you'll need to create a new one." - usage_instructions: "Use your API key by including it in the X-Api-Key header:" - copy_key: "Copy API Key" - continue: "Continue to API Key Settings" \ No newline at end of file + save_api_key: "Save API Key" \ No newline at end of file diff --git a/config/locales/views/settings/en.yml b/config/locales/views/settings/en.yml index c9913ee8e..ee825a7d0 100644 --- a/config/locales/views/settings/en.yml +++ b/config/locales/views/settings/en.yml @@ -262,7 +262,7 @@ en: accounts_label: Accounts advanced_section_title: Advanced ai_prompts_label: AI Prompts - api_key_label: API Key + api_key_label: API Keys payment_label: Payment categories_label: Categories feedback_label: Feedback @@ -288,7 +288,7 @@ en: transactions_section_title: Transactions users_label: Users whats_new_label: What's new - api_keys_label: API Key + api_keys_label: API Keys appearance_label: Appearance bank_sync_label: Bank sync debug_label: Debug diff --git a/config/routes.rb b/config/routes.rb index 3570ce392..8dbef6d5a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -258,7 +258,7 @@ Rails.application.routes.draw do post :options, on: :collection end resources :sso_identities, only: :destroy - resource :api_key, only: [ :show, :new, :create, :destroy ] + resources :api_keys, only: [ :index, :show, :new, :create, :destroy ] resource :mcp, controller: "mcp", only: :show do delete "tokens/:token_id", to: "mcp#revoke", as: :revoke_token end diff --git a/test/controllers/settings/api_keys_controller_test.rb b/test/controllers/settings/api_keys_controller_test.rb index 1905a4cab..df20dd313 100644 --- a/test/controllers/settings/api_keys_controller_test.rb +++ b/test/controllers/settings/api_keys_controller_test.rb @@ -7,29 +7,20 @@ class Settings::ApiKeysControllerTest < ActionDispatch::IntegrationTest sign_in @user end - test "should show no API key page when user has no active keys" do - get settings_api_key_path - assert_response :success - end - - test "should show current API key when user has active key" do - @api_key = ApiKey.create!( + test "index shows api keys list" do + ApiKey.create!( user: @user, - name: "Test API Key", - display_key: "test_key_123", + name: "Listed Key", + display_key: "listed_key_123", scopes: [ "read" ] ) - get settings_api_key_path + get settings_api_keys_path assert_response :success + assert_includes response.body, "Listed Key" end - test "should show new API key form" do - get new_settings_api_key_path - assert_response :success - end - - test "should redirect to show when user already has active key and tries to visit new" do + test "new always renders form (no redirect when key exists)" do ApiKey.create!( user: @user, name: "Existing API Key", @@ -38,57 +29,38 @@ class Settings::ApiKeysControllerTest < ActionDispatch::IntegrationTest ) get new_settings_api_key_path - assert_redirected_to settings_api_key_path + assert_response :success end - test "should create new API key with valid parameters" do + test "create makes a new key without revoking existing keys" do + existing = ApiKey.create!( + user: @user, + name: "Existing API Key", + display_key: "existing_key_123", + scopes: [ "read" ] + ) + assert_difference "ApiKey.count", 1 do - post settings_api_key_path, params: { + post settings_api_keys_path, params: { api_key: { - name: "Test Integration Key", + name: "Brand New Key", scopes: "read_write" } } end - assert_redirected_to settings_api_key_path - follow_redirect! - assert_response :success + new_key = @user.api_keys.active.visible.find_by(name: "Brand New Key") + assert new_key.present? + assert_redirected_to settings_api_key_path(new_key, newly_created: true) - api_key = @user.api_keys.active.first - assert_equal "Test Integration Key", api_key.name - assert_includes api_key.scopes, "read_write" + existing.reload + refute existing.revoked? + assert_includes new_key.scopes, "read_write" end - test "should revoke existing key when creating new one" do - old_key = ApiKey.create!( - user: @user, - name: "Old API Key", - display_key: "old_key_123", - scopes: [ "read" ] - ) - - post settings_api_key_path, params: { - api_key: { - name: "New API Key", - scopes: "read_write" - } - } - - assert_redirected_to settings_api_key_path - follow_redirect! - assert_response :success - - old_key.reload - assert old_key.revoked? - - new_key = @user.api_keys.active.first - assert_equal "New API Key", new_key.name - end - - test "should not create API key without name" do + test "create rejects blank name" do assert_no_difference "ApiKey.count" do - post settings_api_key_path, params: { + post settings_api_keys_path, params: { api_key: { name: "", scopes: "read" @@ -99,93 +71,133 @@ class Settings::ApiKeysControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity end - test "should not create API key without scopes" do - # Ensure clean state for this specific test - @user.api_keys.destroy_all - initial_user_count = @user.api_keys.count - - assert_no_difference "@user.api_keys.count" do - post settings_api_key_path, params: { - api_key: { - name: "Test Key", - scopes: [] + test "create rejects blank scopes" do + assert_no_difference "ApiKey.count" do + post settings_api_keys_path, params: { + api_key: { + name: "No Scopes Key", + scopes: [] + } } - } + end + + assert_response :unprocessable_entity end - assert_response :unprocessable_entity - assert_equal initial_user_count, @user.api_keys.reload.count -end + test "create rejects duplicate active name" do + ApiKey.create!( + user: @user, + name: "Dup", + display_key: "dup_key_123", + scopes: [ "read" ] + ) - test "should revoke API key" do - @api_key = ApiKey.create!( + assert_no_difference "ApiKey.count" do + post settings_api_keys_path, params: { + api_key: { + name: "Dup", + scopes: "read_write" + } + } + end + + assert_response :unprocessable_entity + end + + test "show renders a key" do + created_key = ApiKey.create!( user: @user, name: "Test API Key", display_key: "test_key_123", scopes: [ "read" ] ) - delete settings_api_key_path - - assert_redirected_to settings_api_key_path - follow_redirect! + get settings_api_key_path(created_key) assert_response :success - - @api_key.reload - assert @api_key.revoked? + assert_includes response.body, "Test API Key" end - test "should handle revoke when no API key exists" do - delete settings_api_key_path + test "show renders the newly created confirmation" do + created_key = ApiKey.create!( + user: @user, + name: "Fresh Key", + display_key: "fresh_key_123", + scopes: [ "read" ] + ) - assert_redirected_to settings_api_key_path - # Should not error even when no API key exists + get settings_api_key_path(created_key, newly_created: true) + assert_response :success + assert_includes response.body, created_key.plain_key + assert_select "h3", text: I18n.t("settings.api_keys.show.newly_created.heading") end - test "should only allow one active API key per user" do - # Create first API key - post settings_api_key_path, params: { - api_key: { - name: "First Key", - scopes: "read" - } - } + test "show 404s on another user's key" do + other_user = users(:family_member) + other_user.api_keys.destroy_all + other_key = ApiKey.create!( + user: other_user, + name: "Other User Key", + display_key: "other_user_key_123", + scopes: [ "read" ] + ) - first_key = @user.api_keys.active.first - - # Create second API key - post settings_api_key_path, params: { - api_key: { - name: "Second Key", - scopes: "read_write" - } - } - - # First key should be revoked - first_key.reload - assert first_key.revoked? - - # Only one active key should exist - assert_equal 1, @user.api_keys.active.count - assert_equal "Second Key", @user.api_keys.active.first.name + get settings_api_key_path(other_key) + assert_response :not_found end - test "should generate secure random API key" do - post settings_api_key_path, params: { + test "destroy revokes the targeted key only" do + key1 = ApiKey.create!( + user: @user, + name: "Key One", + display_key: "key_one_123", + scopes: [ "read" ] + ) + key2 = ApiKey.create!( + user: @user, + name: "Key Two", + display_key: "key_two_123", + scopes: [ "read_write" ] + ) + + delete settings_api_key_path(key1) + assert_redirected_to settings_api_keys_path + + key1.reload + key2.reload + assert key1.revoked? + refute key2.revoked? + end + + test "destroy cannot revoke demo monitoring key" do + # set_api_key scopes to .visible which EXCLUDES the demo key, so the + # demo key id is not found by the controller and the request 404s + # before reaching the cannot_revoke branch. + demo_key = ApiKey.create!( + user: @user, + name: "Demo Monitoring Key", + display_key: ApiKey::DEMO_MONITORING_KEY, + scopes: [ "read" ] + ) + + delete settings_api_key_path(demo_key) + assert_response :not_found + + demo_key.reload + refute demo_key.revoked? + end + + test "create generates a secure random API key" do + post settings_api_keys_path, params: { api_key: { name: "Random Key Test", scopes: "read" } } - assert_redirected_to settings_api_key_path - follow_redirect! - assert_response :success - - # Verify the API key was created with expected properties - api_key = @user.api_keys.active.first - assert api_key.present? - assert_equal "Random Key Test", api_key.name - assert_includes api_key.scopes, "read" + created_key = @user.api_keys.active.visible.find_by(name: "Random Key Test") + assert created_key.present? + assert_redirected_to settings_api_key_path(created_key, newly_created: true) + assert_includes created_key.scopes, "read" + assert_equal 64, created_key.plain_key.length end end diff --git a/test/models/api_key_test.rb b/test/models/api_key_test.rb index edd60389a..46087f790 100644 --- a/test/models/api_key_test.rb +++ b/test/models/api_key_test.rb @@ -135,7 +135,7 @@ class ApiKeyTest < ActiveSupport::TestCase assert @api_key.last_used_at > (original_time || Time.at(0)) end - test "should prevent user from having multiple active api keys" do + test "allows multiple active api keys for the same user" do @api_key.save! second_key = ApiKey.new( @@ -145,22 +145,69 @@ class ApiKeyTest < ActiveSupport::TestCase scopes: [ "read" ] ) - assert_not second_key.valid? - assert_includes second_key.errors[:user], "can only have one active API key per source (web)" - end - - test "should allow user to have new active key after revoking old one" do - @api_key.save! - @api_key.revoke! - - second_key = ApiKey.new( - user: @user, - name: "Second API Key", - key: "another_key_123", - scopes: [ "read" ] - ) - assert second_key.valid? + assert second_key.save + assert second_key.active? + assert_equal 2, @user.api_keys.active.count + end + + test "rejects duplicate name among active keys for the same user" do + ApiKey.create!( + user: @user, + name: "Dup", + key: "dup_key_123", + scopes: [ "read" ] + ) + + duplicate = ApiKey.new( + user: @user, + name: "Dup", + key: "dup_key_456", + scopes: [ "read" ] + ) + + assert_not duplicate.valid? + assert_includes duplicate.errors.attribute_names, :name + end + + test "allows reusing the name of a revoked key" do + first = ApiKey.create!( + user: @user, + name: "Reuse", + key: "reuse_key_123", + scopes: [ "read" ] + ) + first.revoke! + + reused = ApiKey.new( + user: @user, + name: "Reuse", + key: "reuse_key_456", + scopes: [ "read" ] + ) + + assert reused.valid? + end + + test "allows the same name across different users" do + ApiKey.create!( + user: @user, + name: "Shared", + key: "shared_key_123", + scopes: [ "read" ] + ) + + other_user = users(:family_member) + other_user.api_keys.destroy_all + + other_key = ApiKey.new( + user: other_user, + name: "Shared", + key: "shared_key_456", + scopes: [ "read" ] + ) + + assert other_key.valid? end test "should allow active monitoring key alongside active web key" do @@ -177,6 +224,21 @@ class ApiKeyTest < ActiveSupport::TestCase assert monitoring_key.valid? end + test "visible scope excludes the demo monitoring key" do + # The demo key's exclusion from `.visible` is the revocation guard used by + # Settings::ApiKeysController#destroy. Lock the invariant so broadening the + # scope can't silently make the demo key revocable. + demo_key = ApiKey.create!( + user: @user, + name: "Demo Monitoring Key", + display_key: ApiKey::DEMO_MONITORING_KEY, + scopes: [ "read" ] + ) + + refute_includes ApiKey.visible, demo_key + refute_includes @user.api_keys.active.visible, demo_key + end + test "should include active api keys in active scope" do @api_key.save! active_keys = ApiKey.active diff --git a/test/system/settings/api_keys_test.rb b/test/system/settings/api_keys_test.rb index e558cb945..bf296309f 100644 --- a/test/system/settings/api_keys_test.rb +++ b/test/system/settings/api_keys_test.rb @@ -7,200 +7,75 @@ class Settings::ApiKeysTest < ApplicationSystemTestCase login_as @user end - test "should show no API key state when user has no active keys" do - visit settings_api_key_path + test "should show empty state when user has no API keys" do + visit settings_api_keys_path - assert_text "API Key" - assert_link "Create API Key", href: new_settings_api_key_path - assert_text "Access your account data programmatically" + assert_text "API Keys" + assert_text "No API keys yet" + assert_link "New API Key", href: new_settings_api_key_path end - test "should navigate to create new API key form" do - visit settings_api_key_path - click_link "Create API Key" + test "should create multiple keys, list them, and revoke one" do + # Create the first key + visit settings_api_keys_path + click_link "New API Key" assert_current_path new_settings_api_key_path assert_text "Create New API Key" - assert_field "API Key Name" - assert_text "Read Only" - assert_text "Read/Write" - end - test "should create a new API key with selected scopes" do - visit new_settings_api_key_path - - fill_in "API Key Name", with: "Test Integration Key" + fill_in "API Key Name", with: "First Key" choose "Read/Write" - click_button "Save API Key" - # Should redirect to show page with the API key details - assert_current_path settings_api_key_path - assert_text "Test Integration Key" - assert_text "Your API Key" - - # Should show the actual API key value - api_key_display = find("#api-key-display") + # Newly-created confirmation page shows the plain key + assert_text "API Key Created Successfully" + assert_text "First Key" + api_key_display = find("[data-clipboard-target='source']") assert api_key_display.text.length > 30 # Should be a long hex string - # Should show copy buttons - assert_button "Copy API Key" - assert_link "Create New Key" - end + # Back to the list, the first key is shown + click_link "Continue to API Key Settings" + assert_current_path settings_api_keys_path + assert_text "First Key" - test "should show current API key details after creation" do - # Create an API key first - api_key = ApiKey.create!( - user: @user, - name: "Production API Key", - display_key: "test_plain_key_123", - scopes: [ "read_write" ] - ) - - visit settings_api_key_path - - assert_text "Your API Key" - assert_text "Production API Key" - assert_text "Active" - assert_text "Read/Write" - assert_text "Never used" - assert_link "Create New Key" - assert_button "Revoke Key" - end - - test "should show usage instructions and example curl command" do - api_key = ApiKey.create!( - user: @user, - name: "Test API Key", - display_key: "test_key_123", - scopes: [ "read" ] - ) - - visit settings_api_key_path - - assert_text "How to use your API key" - assert_text "curl -H \"X-Api-Key: test_key_123\"" - assert_text "/api/v1/accounts" - end - - test "should allow regenerating API key" do - api_key = ApiKey.create!( - user: @user, - name: "Old API Key", - display_key: "old_key_123", - scopes: [ "read" ] - ) - - visit settings_api_key_path - click_link "Create New Key" - - # Should be on the new API key form - assert_text "Create New API Key" - - fill_in "API Key Name", with: "New API Key" + # Create a SECOND key with a different name + click_link "New API Key" + fill_in "API Key Name", with: "Second Key" choose "Read Only" click_button "Save API Key" - # Should redirect to show page with new key - assert_text "New API Key" - assert_text "Your API Key" + assert_text "API Key Created Successfully" + click_link "Continue to API Key Settings" - # Old key should be revoked - api_key.reload - assert api_key.revoked? - end + # Both keys appear in the list + assert_current_path settings_api_keys_path + assert_text "First Key" + assert_text "Second Key" - test "should allow revoking API key with confirmation" do - api_key = ApiKey.create!( - user: @user, - name: "Test API Key", - display_key: "test_key_123", - scopes: [ "read" ] - ) + # Revoke the first key via the custom confirmation dialog + within all(".bg-container", text: "First Key").first do + click_button "Revoke" + end - visit settings_api_key_path - - # Click the revoke button to open the modal - click_button "Revoke Key" - - # Wait for the dialog and then confirm assert_selector "#confirm-dialog", visible: true within "#confirm-dialog" do click_button "Confirm" end - - # Wait for redirect after revoke assert_no_selector "#confirm-dialog" - assert_text "API Key" - assert_text "Access your account data programmatically" + # First key gone, second remains + assert_no_text "First Key" + assert_text "Second Key" - # Key should be revoked in the database - api_key.reload - assert api_key.revoked? + assert @user.api_keys.where(name: "First Key").first.revoked? + refute @user.api_keys.active.where(name: "Second Key").empty? end - test "should redirect to show when user already has active key and tries to visit new" do - api_key = ApiKey.create!( - user: @user, - name: "Existing API Key", - display_key: "existing_key_123", - scopes: [ "read" ] - ) - - visit new_settings_api_key_path - - assert_current_path settings_api_key_path - end - - test "should show API key in navigation" do - visit settings_api_key_path + test "should show API keys in navigation" do + visit settings_api_keys_path within("nav") do - assert_text "API Key" + assert_text "API Keys" end end - - test "should validate API key name is required" do - visit new_settings_api_key_path - - # Try to submit without name - choose "Read Only" - click_button "Save API Key" - - # Should stay on form with validation error - assert_current_path new_settings_api_key_path - assert_field "API Key Name" # Form should still be visible - # The form might not show the validation error inline, but should remain on the form - end - - test "should show last used timestamp when API key has been used" do - api_key = ApiKey.create!( - user: @user, - name: "Used API Key", - display_key: "used_key_123", - scopes: [ "read" ], - last_used_at: 2.hours.ago - ) - - visit settings_api_key_path - - assert_text "2 hours ago" - assert_no_text "Never used" - end - - test "should show expiration date when API key has expiration" do - api_key = ApiKey.create!( - user: @user, - name: "Expiring API Key", - display_key: "expiring_key_123", - scopes: [ "read" ], - expires_at: 30.days.from_now - ) - - visit settings_api_key_path - - # Should show some indication of expiration (exact format may vary) - assert_no_text "Never expires" - end end diff --git a/test/system/settings_test.rb b/test/system/settings_test.rb index 96ec24dcc..954c15266 100644 --- a/test/system/settings_test.rb +++ b/test/system/settings_test.rb @@ -30,7 +30,7 @@ class SettingsTest < ApplicationSystemTestCase @settings_links.insert(merchants_index + 1, [ "Statement Vault", account_statements_path ]) @settings_links += [ [ "AI Prompts", settings_ai_prompts_path ], - [ "API Key", settings_api_key_path ] + [ "API Keys", settings_api_keys_path ] ] end end @@ -93,7 +93,7 @@ class SettingsTest < ApplicationSystemTestCase # Assert that admin-only settings are not present in the navigation assert_no_selector "li", text: "AI Prompts" - assert_no_selector "li", text: "API Key" + assert_no_selector "li", text: "API Keys" assert_no_selector "li", text: "Bank sync" assert_no_selector "li", text: "Statement Vault" end