diff --git a/app/controllers/settings/api_keys_controller.rb b/app/controllers/settings/api_keys_controller.rb index cb6d889e3..c509df41a 100644 --- a/app/controllers/settings/api_keys_controller.rb +++ b/app/controllers/settings/api_keys_controller.rb @@ -16,7 +16,7 @@ class Settings::ApiKeysController < ApplicationController 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.exists? && !params[:regenerate] + redirect_to settings_api_key_path if Current.user.api_keys.active.visible.exists? && !params[:regenerate] @api_key = ApiKey.new end @@ -25,8 +25,9 @@ class Settings::ApiKeysController < ApplicationController @api_key = Current.user.api_keys.build(api_key_params) @api_key.key = @plain_key - # Temporarily revoke existing keys for validation to pass - existing_keys = Current.user.api_keys.active + # 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 @@ -40,7 +41,11 @@ class Settings::ApiKeysController < ApplicationController end def destroy - if @api_key&.revoke! + if @api_key.nil? + flash[:alert] = "API key not found" + elsif @api_key.demo_monitoring_key? + flash[:alert] = "This API key cannot be revoked" + elsif @api_key.revoke! flash[:notice] = "API key has been revoked successfully" else flash[:alert] = "Failed to revoke API key" @@ -51,7 +56,7 @@ class Settings::ApiKeysController < ApplicationController private def set_api_key - @api_key = Current.user.api_keys.active.first + @api_key = Current.user.api_keys.active.visible.first end def api_key_params diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 2e190500e..ae24ccec3 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -9,7 +9,8 @@ class ApiKey < ApplicationRecord end # Constants - SOURCES = [ "web", "mobile" ].freeze + SOURCES = [ "web", "mobile", "monitoring" ].freeze + DEMO_MONITORING_KEY = "demo_monitoring_key_a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" # Validations validates :display_key, presence: true, uniqueness: true @@ -21,9 +22,11 @@ class ApiKey < ApplicationRecord # Callbacks before_validation :set_display_key + before_destroy :prevent_demo_monitoring_key_destroy! # Scopes scope :active, -> { where(revoked_at: nil).where("expires_at IS NULL OR expires_at > ?", Time.current) } + scope :visible, -> { where.not(display_key: DEMO_MONITORING_KEY) } # Class methods def self.find_by_value(plain_key) @@ -57,9 +60,19 @@ class ApiKey < ApplicationRecord end def revoke! + raise ActiveRecord::RecordNotDestroyed, "Cannot revoke demo monitoring API key" if demo_monitoring_key? update!(revoked_at: Time.current) end + def delete + raise ActiveRecord::RecordNotDestroyed, "Cannot destroy demo monitoring API key" if demo_monitoring_key? + super + end + + def demo_monitoring_key? + display_key == DEMO_MONITORING_KEY + end + def update_last_used! update_column(:last_used_at, Time.current) end @@ -95,4 +108,11 @@ class ApiKey < ApplicationRecord errors.add(:user, "can only have one active API key per source (#{source})") end end + + def prevent_demo_monitoring_key_destroy! + return unless demo_monitoring_key? + + errors.add(:base, "Cannot destroy demo monitoring API key") + throw(:abort) + end end diff --git a/app/models/demo/generator.rb b/app/models/demo/generator.rb index 77ce710ee..3ff1051a0 100644 --- a/app/models/demo/generator.rb +++ b/app/models/demo/generator.rb @@ -1,10 +1,6 @@ require "securerandom" class Demo::Generator - # Deterministic API key for uptime monitoring - # This key is always the same so it can be hardcoded in monitoring tools - MONITORING_API_KEY = "demo_monitoring_key_a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" - # @param seed [Integer, String, nil] Seed value used to initialise the internal PRNG. If nil, the ENV variable DEMO_DATA_SEED will # be honoured and default to a random seed when not present. # @@ -193,24 +189,25 @@ class Demo::Generator return unless admin_user # Find existing key scoped to this admin user by the deterministic display_key value - existing_key = admin_user.api_keys.find_by(display_key: MONITORING_API_KEY) + existing_key = admin_user.api_keys.find_by(display_key: ApiKey::DEMO_MONITORING_KEY) if existing_key puts " → Use existing monitoring API key" return existing_key end - # Revoke any existing web API keys for this user to avoid one-per-source validation error - admin_user.api_keys.active.where(source: "web").find_each(&:revoke!) + # Revoke any existing user-created web API keys to keep demo access predictable. + # (the monitoring key uses the dedicated "monitoring" source and cannot be revoked) + admin_user.api_keys.active.visible.where(source: "web").find_each(&:revoke!) api_key = admin_user.api_keys.create!( name: "monitoring", - key: MONITORING_API_KEY, + key: ApiKey::DEMO_MONITORING_KEY, scopes: [ "read" ], - source: "web" + source: "monitoring" ) - puts " → Created monitoring API key: #{MONITORING_API_KEY}" + puts " → Created monitoring API key: #{ApiKey::DEMO_MONITORING_KEY}" api_key end diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index c7b3ac0c2..6a3eb3baf 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -2,7 +2,7 @@ class Rack::Attack # Enable Rack::Attack - enabled = Rails.env.production? || Rails.env.staging? + self.enabled = Rails.env.production? || Rails.env.staging? # Throttle requests to the OAuth token endpoint throttle("oauth/token", limit: 10, period: 1.minute) do |request| diff --git a/test/models/api_key_test.rb b/test/models/api_key_test.rb index d437e2c8b..edd60389a 100644 --- a/test/models/api_key_test.rb +++ b/test/models/api_key_test.rb @@ -163,6 +163,20 @@ class ApiKeyTest < ActiveSupport::TestCase assert second_key.valid? end + test "should allow active monitoring key alongside active web key" do + @api_key.save! + + monitoring_key = ApiKey.new( + user: @user, + name: "Monitoring API Key", + key: "monitoring_key_123", + scopes: [ "read" ], + source: "monitoring" + ) + + assert monitoring_key.valid? + end + test "should include active api keys in active scope" do @api_key.save! active_keys = ApiKey.active @@ -204,4 +218,54 @@ class ApiKeyTest < ActiveSupport::TestCase assert_not @api_key.valid? assert_includes @api_key.errors[:scopes], "must be either 'read' or 'read_write'" end + + test "should prevent destroying demo monitoring api key" do + demo_key = ApiKey.create!( + user: @user, + name: "Demo Monitoring Key", + display_key: ApiKey::DEMO_MONITORING_KEY, + scopes: [ "read" ] + ) + + assert_raises(ActiveRecord::RecordNotDestroyed) { demo_key.destroy! } + assert ApiKey.exists?(demo_key.id) + end + + test "should prevent revoking demo monitoring api key" do + demo_key = ApiKey.create!( + user: @user, + name: "Demo Monitoring Key", + display_key: ApiKey::DEMO_MONITORING_KEY, + scopes: [ "read" ] + ) + + assert_raises(ActiveRecord::RecordNotDestroyed) { demo_key.revoke! } + demo_key.reload + assert_nil demo_key.revoked_at + end + + test "should prevent deleting demo monitoring api key" do + demo_key = ApiKey.create!( + user: @user, + name: "Demo Monitoring Key", + display_key: ApiKey::DEMO_MONITORING_KEY, + scopes: [ "read" ] + ) + + assert_raises(ActiveRecord::RecordNotDestroyed) { demo_key.delete } + assert ApiKey.exists?(demo_key.id) + end + + test "should allow destroying non-demo api key" do + api_key = ApiKey.create!( + user: @user, + name: "Disposable API Key", + display_key: "disposable_key_123", + scopes: [ "read" ] + ) + + assert_difference("ApiKey.count", -1) do + api_key.destroy! + end + end end