Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api-key-management): Add permissions to the ApiKey #2853

Merged
merged 4 commits into from
Nov 26, 2024
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
44 changes: 44 additions & 0 deletions app/models/api_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,63 @@
class ApiKey < ApplicationRecord
include PaperTrailTraceable

RESOURCES = %w[
add_on analytic billable_metric coupon applied_coupon credit_note customer_usage
customer fee invoice organization payment_request plan subscription lifetime_usage
tax wallet wallet_transaction webhook_endpoint
].freeze

MODES = %w[read write].freeze

attribute :permissions, default: -> { default_permissions }

belongs_to :organization

before_create :set_value

validates :value, uniqueness: true
validates :value, presence: true, on: :update
validates :permissions, presence: true
validate :permissions_keys_compliance
validate :permissions_values_allowed

default_scope { active }

scope :active, -> { where('expires_at IS NULL OR expires_at > ?', Time.current) }
scope :non_expiring, -> { where(expires_at: nil) }

def permit?(resource, mode)
return true unless organization.premium_integrations.include?('api_permissions')

Array(permissions[resource]).include?(mode)
end

def self.default_permissions
RESOURCES.index_with { MODES.dup }
end

private

def permissions_keys_compliance
nudded marked this conversation as resolved.
Show resolved Hide resolved
return unless permissions

forbidden_permissions = permissions.keys - RESOURCES

if forbidden_permissions.any?
errors.add(:permissions, :forbidden_keys, keys: forbidden_permissions)
end
end

def permissions_values_allowed
return unless permissions

forbidden_values = permissions.values.flatten - MODES

if forbidden_values.any?
errors.add(:permissions, :forbidden_values, values: forbidden_values)
end
end

def set_value
loop do
self.value = SecureRandom.uuid
Expand All @@ -33,6 +76,7 @@ def set_value
# expires_at :datetime
# last_used_at :datetime
# name :string
# permissions :jsonb not null
# value :string not null
# created_at :datetime not null
# updated_at :datetime not null
Expand Down
2 changes: 1 addition & 1 deletion app/models/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class Organization < ApplicationRecord
].freeze

INTEGRATIONS = %w[
netsuite okta anrok xero progressive_billing hubspot auto_dunning revenue_analytics salesforce
netsuite okta anrok xero progressive_billing hubspot auto_dunning revenue_analytics salesforce api_permissions
].freeze
PREMIUM_INTEGRATIONS = INTEGRATIONS - %w[anrok]

Expand Down
6 changes: 6 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ en:
less_than_or_equal_to: value_is_out_of_range
missing_graduated_ranges: missing_graduated_ranges
missing_volume_ranges: missing_volume_ranges
models:
api_key:
attributes:
permissions:
forbidden_key: 'contains forbidden keys: %{keys}'
missing_key: 'missing required keys: %{keys}'
not_compatible_with_aggregation_type: not_compatible_with_aggregation_type
not_compatible_with_pay_in_advance: not_compatible_with_pay_in_advance
only_compatible_with_pay_in_advance_and_non_invoiceable: only_compatible_with_pay_in_advance_and_non_invoiceable
Expand Down
15 changes: 15 additions & 0 deletions db/migrate/20241120094557_add_permissions_to_api_keys.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

class AddPermissionsToApiKeys < ActiveRecord::Migration[7.1]
def up
add_column :api_keys, :permissions, :jsonb, null: false, default: {}

ApiKey.update_all(permissions: ApiKey.default_permissions) # rubocop:disable Rails/SkipsModelValidations

change_column_default :api_keys, :permissions, nil
end

def down
remove_column :api_keys, :permissions
end
end
1 change: 1 addition & 0 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions schema.graphql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

127 changes: 127 additions & 0 deletions spec/models/api_key_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
RSpec.describe ApiKey, type: :model do
it { is_expected.to belong_to(:organization) }

it { is_expected.to validate_presence_of(:permissions) }

describe 'validations' do
describe 'of value uniqueness' do
before { create(:api_key) }
Expand All @@ -27,6 +29,57 @@
it { is_expected.to validate_presence_of(:value) }
end
end

describe 'of permissions structure' do
subject { api_key.valid? }

let(:api_key) { build_stubbed(:api_key) }
let(:error) { api_key.errors.where(:permissions, :forbidden_keys) }

context 'when permissions has forbidden keys' do
before do
api_key.permissions = api_key.permissions.merge(forbidden: [])
subject
end

it 'adds forbidden keys error' do
expect(error).to be_present
end
end

context 'when permissions has no forbidden keys' do
before { subject }

it 'does not add forbidden keys error' do
expect(error).not_to be_present
end
end
end

describe 'of permissions values' do
subject { api_key.valid? }

let(:api_key) { build_stubbed(:api_key, permissions:) }
let(:error) { api_key.errors.where(:permissions, :forbidden_values) }

before { subject }

context 'when permission contains forbidden values' do
let(:permissions) { {add_on: ['forbidden', 'read']} }

it 'adds an error' do
expect(error).to be_present
end
end

context 'when permission contains only allowed values' do
let(:permissions) { {add_on: ['read', 'write']} }

it 'does not add an error' do
expect(error).not_to be_present
end
end
end
end

describe '#save' do
Expand Down Expand Up @@ -83,4 +136,78 @@
expect(subject).to contain_exactly scoped
end
end

describe "#permit?" do
subject { api_key.permit?(resource, mode) }

let(:api_key) { create(:api_key, permissions:) }
let(:resource) { described_class::RESOURCES.sample }
let(:mode) { described_class::MODES.sample }

before { api_key.organization.update!(premium_integrations:) }

context "when organization has 'api_permissions' add-on enabled" do
let(:premium_integrations) { ["api_permissions"] }

context "when corresponding resource is specified in permissions" do
let(:permissions) { {resource => allowed_modes} }

context "when corresponding resource allows provided mode" do
let(:allowed_modes) { [mode] }

it "returns true" do
expect(subject).to be true
end
end

context "when corresponding resource does not allow provided mode" do
let(:allowed_modes) { described_class::MODES.excluding(mode) }

it "returns false" do
expect(subject).to be false
end
end
end

context "when corresponding resource does not specified in permissions" do
let(:permissions) { described_class.default_permissions.without(resource) }

it "returns false" do
expect(subject).to be false
end
end
end

context "when organization has 'api_permissions' add-on disabled" do
let(:premium_integrations) { [] }

context "when corresponding resource is specified in permissions" do
let(:permissions) { {resource => allowed_modes} }

context "when corresponding resource allows provided mode" do
let(:allowed_modes) { [mode] }

it "returns true" do
expect(subject).to be true
end
end

context "when corresponding resource does not allow provided mode" do
let(:allowed_modes) { described_class::MODES.excluding(mode) }

it "returns true" do
expect(subject).to be true
end
end
end

context "when corresponding resource does not specified in permissions" do
let(:permissions) { described_class.default_permissions.without(resource) }

it "returns true" do
expect(subject).to be true
end
end
end
end
end