mirror of
https://github.com/we-promise/sure.git
synced 2026-07-11 20:35:17 +00:00
* feat(api): allow multiple active API keys per user Previously a user could hold only one active API key; creating a new one silently revoked the existing key, breaking any app using it. Allow multiple named active keys instead. - Drop the one-active-key-per-source validation; add name uniqueness among the user's active visible keys (revoked names are reusable, same name allowed across users). - Rewrite Settings::ApiKeysController as a RESTful collection (index/show/new/create/destroy); create no longer revokes existing keys, and key lookup is scoped to the current user's active keys. - resource :api_key -> resources :api_keys. - Add an API keys list view with per-key revoke and an empty state; rewrite the show page for a single key. - Update i18n (remove single-key copy, pluralise nav label) and tests. * refactor(api): address review on multiple API keys - Remove unreachable destroy branches (cannot_revoke is guarded by the .visible 404; revoke! raises rather than returning false) and document that .visible is the demo-key revocation guard. - Delete the orphaned created.html.erb / created.turbo_stream.erb templates (no action renders them) and their unused locale keys. - Extract shared partials (_scope_badges, _status_indicator, _key_meta, _key_reveal, _usage) to de-duplicate the index and show views; unify the active-status indicator on the standard dot. - Carry forward the @container / @lg:flex-row / min-w-0 responsive fixes from #2079 into the shared key-reveal partial. * test(api): cover newly-created API key confirmation render * refactor(api): harden demo-key guard and address review nits - Document the demo-key revocation guard on ApiKey's `visible` scope (the authoritative spot) and add a model test locking the invariant that `.visible` excludes the demo monitoring key. - _scope_badges: use an i18n lookup with a humanize fallback instead of bypassing translation for unknown scopes. - _key_reveal: drop the hard-coded `id` from the shared partial; the system test now locates the key via its data-clipboard-target. * refactor(api-keys): migrate hand-rolled badges to DS::Pill Replace raw span elements in scope badges and status indicator with DS::Pill to align with the design system migration convention. * fix(loans): opening anchor now uses current balance, not original principal When creating a loan manually, the opening anchor valuation was being set to `initial_balance` (the original loan principal) instead of `account.balance` (the current outstanding balance). After the sync job ran, `account.balance` was overwritten to match the anchor, making every manually-created loan show its original principal as the current balance. Fix by always using `account.balance` for the opening anchor in `create_and_sync`, and reading `Loan#original_balance` from the `loans.initial_balance` column directly (with a fallback to `first_valuation_amount` for provider-synced loans that may not have the column populated). * fix(api-keys): strip accidental loan changes; rescue revoke! failures The fix(loans) commit was accidentally committed into this branch. Remove the loan-related changes from account.rb, loan.rb, and both test files, restoring them to their pre-loan-commit state. Also fix the destroy action: revoke! uses update! internally which raises ActiveRecord::RecordInvalid on failure rather than returning false. Add rescue for RecordInvalid and RecordNotDestroyed so failures produce a flash alert instead of a 500. Re-adds the revoke_failed locale key that was dropped from settings.api_keys.destroy. --------- Signed-off-by: Juan José Mata <juanjo.mata@gmail.com> Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
334 lines
8.1 KiB
Ruby
334 lines
8.1 KiB
Ruby
require "test_helper"
|
|
|
|
class ApiKeyTest < ActiveSupport::TestCase
|
|
def setup
|
|
@user = users(:family_admin)
|
|
# Clean up any existing API keys for this user to ensure tests start fresh
|
|
@user.api_keys.destroy_all
|
|
@api_key = ApiKey.new(
|
|
user: @user,
|
|
name: "Test API Key",
|
|
key: "test_plain_key_123",
|
|
scopes: [ "read_write" ]
|
|
)
|
|
end
|
|
|
|
test "should be valid with valid attributes" do
|
|
assert @api_key.valid?
|
|
end
|
|
|
|
test "should require display_key presence after save" do
|
|
@api_key.key = nil
|
|
assert_not @api_key.valid?
|
|
end
|
|
|
|
test "should require name presence" do
|
|
@api_key.name = nil
|
|
assert_not @api_key.valid?
|
|
assert_includes @api_key.errors[:name], "can't be blank"
|
|
end
|
|
|
|
test "should require scopes presence" do
|
|
@api_key.scopes = nil
|
|
assert_not @api_key.valid?
|
|
assert_includes @api_key.errors[:scopes], "can't be blank"
|
|
end
|
|
|
|
test "should require user association" do
|
|
@api_key.user = nil
|
|
assert_not @api_key.valid?
|
|
assert_includes @api_key.errors[:user], "must exist"
|
|
end
|
|
|
|
test "should set display_key from key before saving" do
|
|
original_key = @api_key.key
|
|
@api_key.save!
|
|
|
|
# display_key should be encrypted but plain_key should return the original
|
|
assert_equal original_key, @api_key.plain_key
|
|
end
|
|
|
|
test "should find api key by plain value" do
|
|
plain_key = @api_key.key
|
|
@api_key.save!
|
|
|
|
found_key = ApiKey.find_by_value(plain_key)
|
|
assert_equal @api_key, found_key
|
|
end
|
|
|
|
test "should return nil when finding by invalid value" do
|
|
@api_key.save!
|
|
|
|
found_key = ApiKey.find_by_value("invalid_key")
|
|
assert_nil found_key
|
|
end
|
|
|
|
test "should return nil when finding by nil value" do
|
|
@api_key.save!
|
|
|
|
found_key = ApiKey.find_by_value(nil)
|
|
assert_nil found_key
|
|
end
|
|
|
|
test "key_matches? should work with plain key" do
|
|
plain_key = @api_key.key
|
|
@api_key.save!
|
|
|
|
assert @api_key.key_matches?(plain_key)
|
|
assert_not @api_key.key_matches?("wrong_key")
|
|
end
|
|
|
|
test "should be active when not revoked and not expired" do
|
|
@api_key.save!
|
|
|
|
assert @api_key.active?
|
|
end
|
|
|
|
test "should not be active when revoked" do
|
|
@api_key.save!
|
|
@api_key.revoke!
|
|
|
|
assert_not @api_key.active?
|
|
assert @api_key.revoked?
|
|
end
|
|
|
|
test "should not be active when expired" do
|
|
@api_key.expires_at = 1.day.ago
|
|
@api_key.save!
|
|
|
|
assert_not @api_key.active?
|
|
assert @api_key.expired?
|
|
end
|
|
|
|
test "should be active when expires_at is in the future" do
|
|
@api_key.expires_at = 1.day.from_now
|
|
@api_key.save!
|
|
|
|
assert @api_key.active?
|
|
assert_not @api_key.expired?
|
|
end
|
|
|
|
test "should be active when expires_at is nil" do
|
|
@api_key.expires_at = nil
|
|
@api_key.save!
|
|
|
|
assert @api_key.active?
|
|
assert_not @api_key.expired?
|
|
end
|
|
|
|
test "should generate secure key" do
|
|
key = ApiKey.generate_secure_key
|
|
|
|
assert_kind_of String, key
|
|
assert_equal 64, key.length # hex(32) = 64 characters
|
|
assert key.match?(/\A[0-9a-f]+\z/) # only hex characters
|
|
end
|
|
|
|
test "should update last_used_at when update_last_used! is called" do
|
|
@api_key.save!
|
|
original_time = @api_key.last_used_at
|
|
|
|
sleep(0.01) # Ensure time difference
|
|
@api_key.update_last_used!
|
|
|
|
assert_not_equal original_time, @api_key.last_used_at
|
|
assert @api_key.last_used_at > (original_time || Time.at(0))
|
|
end
|
|
|
|
test "allows multiple active api keys for the same user" do
|
|
@api_key.save!
|
|
|
|
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
|
|
@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 "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
|
|
|
|
assert_includes active_keys, @api_key
|
|
end
|
|
|
|
test "should exclude revoked api keys from active scope" do
|
|
@api_key.save!
|
|
@api_key.revoke!
|
|
active_keys = ApiKey.active
|
|
|
|
assert_not_includes active_keys, @api_key
|
|
end
|
|
|
|
test "should exclude expired api keys from active scope" do
|
|
@api_key.expires_at = 1.day.ago
|
|
@api_key.save!
|
|
active_keys = ApiKey.active
|
|
|
|
assert_not_includes active_keys, @api_key
|
|
end
|
|
|
|
test "should return plain_key for display" do
|
|
original_key = @api_key.key
|
|
@api_key.save!
|
|
|
|
assert_equal original_key, @api_key.plain_key
|
|
end
|
|
|
|
test "should not allow multiple scopes" do
|
|
@api_key.scopes = [ "read", "read_write" ]
|
|
assert_not @api_key.valid?
|
|
assert_includes @api_key.errors[:scopes], "can only have one permission level"
|
|
end
|
|
|
|
test "should validate scope values" do
|
|
@api_key.scopes = [ "invalid_scope" ]
|
|
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
|