diff --git a/.rubocop.yml b/.rubocop.yml index 23b0a10dc0ac..473f9bc11da0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,6 +5,8 @@ require: - rubocop-rails - rubocop-thread_safety - rubocop-graphql + - rubocop-factory_bot + - rubocop-rspec_rails - ./dev/cops/service_call_cop.rb inherit_gem: diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 1ed7979bb754..a447d00b0b24 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -27,8 +27,9 @@ def authenticate true end - def current_organization(api_key = nil) - @current_organization ||= Organization.find_by(api_key:) + def current_organization(value = nil) + @current_organization ||= + ApiKey.find_by(value:)&.organization || Organization.find_by(api_key: value) end def set_context_source diff --git a/app/graphql/types/organizations/current_organization_type.rb b/app/graphql/types/organizations/current_organization_type.rb index 495ea24a1a0d..b0dab79616fc 100644 --- a/app/graphql/types/organizations/current_organization_type.rb +++ b/app/graphql/types/organizations/current_organization_type.rb @@ -52,6 +52,10 @@ class CurrentOrganizationType < BaseOrganizationType def webhook_url object.webhook_endpoints.map(&:webhook_url).first end + + def api_key + object.api_keys.first.value + end end end end diff --git a/app/models/api_key.rb b/app/models/api_key.rb new file mode 100644 index 000000000000..10dbd4ccdbd2 --- /dev/null +++ b/app/models/api_key.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +class ApiKey < ApplicationRecord + include PaperTrailTraceable + + belongs_to :organization + + before_create :set_value + + def generate_value + value = SecureRandom.uuid + api_key = ApiKey.find_by(value:) + + return generate_value if api_key.present? + + value + end + + private + + def set_value + self.value = generate_value + end +end + +# == Schema Information +# +# Table name: api_keys +# +# id :uuid not null, primary key +# value :string not null +# created_at :datetime not null +# updated_at :datetime not null +# organization_id :uuid not null +# +# Indexes +# +# index_api_keys_on_organization_id (organization_id) +# +# Foreign Keys +# +# fk_rails_... (organization_id => organizations.id) +# diff --git a/app/models/organization.rb b/app/models/organization.rb index 7de77e34f12f..59f8d583e06e 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -10,6 +10,7 @@ class Organization < ApplicationRecord 'credit_note.created' ].freeze + has_many :api_keys has_many :memberships has_many :users, through: :memberships has_many :billable_metrics @@ -58,8 +59,6 @@ class Organization < ApplicationRecord enum document_numbering: DOCUMENT_NUMBERINGS - before_create :generate_api_key - validates :country, country_code: true, unless: -> { country.nil? } validates :default_currency, inclusion: {in: currency_list} validates :document_locale, language_code: true @@ -124,15 +123,6 @@ def auto_dunning_enabled? private - def generate_api_key - api_key = SecureRandom.uuid - orga = Organization.find_by(api_key:) - - return generate_api_key if orga.present? - - self.api_key = SecureRandom.uuid - end - # NOTE: After creating an organization, default document_number_prefix needs to be generated. # Example of expected format is ORG-4321 def generate_document_number_prefix diff --git a/app/models/webhook.rb b/app/models/webhook.rb index b3ab9ac128e6..55bb96bf2843 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -8,8 +8,7 @@ class Webhook < ApplicationRecord belongs_to :webhook_endpoint belongs_to :object, polymorphic: true, optional: true - # TODO: Use relation to be able to eager load - delegate :organization, to: :webhook_endpoint + has_one :organization, through: :webhook_endpoint enum status: STATUS @@ -57,7 +56,8 @@ def jwt_signature end def hmac_signature - hmac = OpenSSL::HMAC.digest('sha-256', organization.api_key, payload.to_json) + api_key_value = organization.api_keys.first.value + hmac = OpenSSL::HMAC.digest('sha-256', api_key_value, payload.to_json) Base64.strict_encode64(hmac) end diff --git a/app/services/organizations/create_service.rb b/app/services/organizations/create_service.rb new file mode 100644 index 000000000000..5140b01e26a3 --- /dev/null +++ b/app/services/organizations/create_service.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Organizations + class CreateService < BaseService + def initialize(params) + @params = params + super + end + + def call + organization = Organization.new(params) + + ActiveRecord::Base.transaction do + organization.save! + organization.api_keys.create! + end + + result.organization = organization + result + rescue ActiveRecord::RecordInvalid => e + result.record_validation_failure!(record: e.record) + end + + private + + attr_reader :params + end +end diff --git a/app/services/users_service.rb b/app/services/users_service.rb index ec3e289b84bd..f4c67306d0bd 100644 --- a/app/services/users_service.rb +++ b/app/services/users_service.rb @@ -29,7 +29,10 @@ def register(email, password, organization_name) ActiveRecord::Base.transaction do result.user = User.create!(email:, password:) - result.organization = Organization.create!(name: organization_name, document_numbering: 'per_organization') + + result.organization = Organizations::CreateService + .call(name: organization_name, document_numbering: 'per_organization') + .organization result.membership = Membership.create!( user: result.user, diff --git a/db/migrate/20241021140054_create_api_keys.rb b/db/migrate/20241021140054_create_api_keys.rb new file mode 100644 index 000000000000..7d287597e628 --- /dev/null +++ b/db/migrate/20241021140054_create_api_keys.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class CreateApiKeys < ActiveRecord::Migration[7.1] + def up + create_table :api_keys, id: :uuid do |t| + t.references :organization, type: :uuid, null: false, foreign_key: true + + t.string :value, null: false + + t.timestamps + end + + safety_assured do + execute <<-SQL + INSERT INTO api_keys (value, organization_id, created_at, updated_at) + SELECT organizations.api_key, organizations.id, organizations.created_at, organizations.created_at + FROM organizations + SQL + end + end + + def down + safety_assured do + execute <<-SQL + UPDATE organizations + SET api_key = first_api_key.value + FROM ( + SELECT DISTINCT ON (organization_id) + organization_id, + value + FROM api_keys + ORDER BY organization_id, id ASC + ) first_api_key + WHERE organizations.id = first_api_key.organization_id + AND organizations.api_key IS NULL + SQL + end + + drop_table :api_keys + end +end diff --git a/db/schema.rb b/db/schema.rb index 0f8f96059deb..a06571e40ed0 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_10_21_095706) do +ActiveRecord::Schema[7.1].define(version: 2024_10_21_140054) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -101,6 +101,14 @@ t.index ["subscription_id"], name: "index_adjusted_fees_on_subscription_id" end + create_table "api_keys", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "organization_id", null: false + t.string "value", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["organization_id"], name: "index_api_keys_on_organization_id" + end + create_table "applied_add_ons", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "add_on_id", null: false t.uuid "customer_id", null: false @@ -1254,6 +1262,7 @@ add_foreign_key "adjusted_fees", "groups" add_foreign_key "adjusted_fees", "invoices" add_foreign_key "adjusted_fees", "subscriptions" + add_foreign_key "api_keys", "organizations" add_foreign_key "applied_add_ons", "add_ons" add_foreign_key "applied_add_ons", "customers" add_foreign_key "applied_usage_thresholds", "invoices" diff --git a/db/seeds/base.rb b/db/seeds/base.rb index eb1097f4b521..ddf111e4bd58 100644 --- a/db/seeds/base.rb +++ b/db/seeds/base.rb @@ -10,6 +10,7 @@ organization = Organization.find_or_create_by!(name: 'Hooli') organization.premium_integrations = Organization::PREMIUM_INTEGRATIONS organization.save! +ApiKey.find_or_create_by!(organization:) Membership.find_or_create_by!(user:, organization:, role: :admin) # NOTE: define a billing model diff --git a/db/seeds/webhooks.rb b/db/seeds/webhooks.rb index 26cc3ec9eef9..ed322a322a1a 100644 --- a/db/seeds/webhooks.rb +++ b/db/seeds/webhooks.rb @@ -4,6 +4,7 @@ require 'factory_bot_rails' organization = Organization.find_or_create_by!(name: 'Hooli') +ApiKey.find_or_create_by!(organization:) webhook_endpoint = WebhookEndpoint.find_or_create_by!(organization:, webhook_url: 'http://test.lago.dev/webhook') diff --git a/lib/tasks/signup.rake b/lib/tasks/signup.rake index 68c106d6b28d..9bf0c4c3d524 100644 --- a/lib/tasks/signup.rake +++ b/lib/tasks/signup.rake @@ -13,9 +13,15 @@ namespace :signup do .find_or_create_by!(email: ENV['LAGO_ORG_USER_EMAIL']) organization = Organization.find_or_create_by!(name: ENV['LAGO_ORG_NAME']) raise "Couldn't find LAGO_ORG_API_KEY in environement variables" if ENV['LAGO_ORG_API_KEY'].blank? - - organization.api_key = ENV['LAGO_ORG_API_KEY'] organization.save! + + existing_api_key = ApiKey.find_by(organization:, value: ENV['LAGO_ORG_API_KEY']) + + unless existing_api_key + api_key = ApiKey.create!(organization:) + api_key.update!(value: ENV['LAGO_ORG_API_KEY']) + end + Membership.find_or_create_by!(user:, organization:, role: :admin) pp 'ending seeding environment' diff --git a/spec/factories/api_keys.rb b/spec/factories/api_keys.rb new file mode 100644 index 000000000000..804d9c51f05a --- /dev/null +++ b/spec/factories/api_keys.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :api_key do + organization { association(:organization) } + end +end diff --git a/spec/factories/organizations.rb b/spec/factories/organizations.rb index 4f01e0bfb00c..5d4bf73e4844 100644 --- a/spec/factories/organizations.rb +++ b/spec/factories/organizations.rb @@ -13,7 +13,11 @@ end after(:create) do |organization, evaluator| - organization.webhook_endpoints.create(webhook_url: evaluator.webhook_url) + if evaluator.webhook_url + organization.webhook_endpoints.create!(webhook_url: evaluator.webhook_url) + end + + organization.api_keys.create! end end end diff --git a/spec/graphql/resolvers/organization_resolver_spec.rb b/spec/graphql/resolvers/organization_resolver_spec.rb index dc409999f64c..8167bf3a64df 100644 --- a/spec/graphql/resolvers/organization_resolver_spec.rb +++ b/spec/graphql/resolvers/organization_resolver_spec.rb @@ -66,7 +66,7 @@ aggregate_failures do expect(data['taxIdentificationNumber']).to eq(organization.tax_identification_number) - expect(data['apiKey']).to eq(organization.api_key) + expect(data['apiKey']).to eq(organization.api_keys.first.value) expect(data['webhookUrl']).to eq(organization.webhook_endpoints.first.webhook_url) expect(data['billingConfiguration']['invoiceFooter']).to eq(organization.invoice_footer) expect(data['emailSettings']).to eq(organization.email_settings.map { _1.tr('.', '_') }) diff --git a/spec/models/api_key_spec.rb b/spec/models/api_key_spec.rb new file mode 100644 index 000000000000..2961119e3d37 --- /dev/null +++ b/spec/models/api_key_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ApiKey, type: :model do + it { is_expected.to belong_to(:organization) } + + describe '#save' do + subject { api_key.save! } + + before do + allow(api_key).to receive(:set_value).and_call_original + subject + end + + context 'with a new record' do + let(:api_key) { build(:api_key) } + + it 'calls #set_value' do + expect(api_key).to have_received(:set_value) + end + end + + context 'with a persisted record' do + let(:api_key) { create(:api_key) } + + it 'does not call #set_value' do + expect(api_key).not_to have_received(:set_value) + end + end + end + + describe '#set_value' do + subject { api_key.send(:set_value) } + + let(:api_key) { build(:api_key) } + let(:unique_value) { SecureRandom.uuid } + + before { allow(api_key).to receive(:generate_value).and_return(unique_value) } + + it 'sets result of #generate_value to the value' do + expect { subject }.to change(api_key, :value).to unique_value + end + end + + describe '#generate_value' do + subject { api_key.generate_value } + + let(:api_key) { build(:api_key) } + let(:used_value) { create(:api_key).value } + let(:unique_value) { SecureRandom.uuid } + + before do + allow(SecureRandom).to receive(:uuid).and_return(used_value, unique_value) + end + + it 'returns unique value between all ApiKeys' do + expect(subject).to eq unique_value + end + end +end diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index e368f8a663c7..ce76c234a026 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -16,6 +16,7 @@ it { is_expected.to have_many(:gocardless_payment_providers) } it { is_expected.to have_many(:adyen_payment_providers) } + it { is_expected.to have_many(:api_keys) } it { is_expected.to have_many(:webhook_endpoints) } it { is_expected.to have_many(:webhooks).through(:webhook_endpoints) } it { is_expected.to have_many(:hubspot_integrations) } @@ -138,11 +139,43 @@ end end - describe 'Callbacks' do - it 'generates the api key' do - organization.save! + describe '#save' do + subject { organization.save! } - expect(organization.api_key).to be_present + before do + allow(organization).to receive(:generate_document_number_prefix).and_call_original # rubocop:disable RSpec/SubjectStub + subject + end + + context 'with a new record' do + let(:organization) { build(:organization) } + + it 'calls #generate_document_number_prefix' do + expect(organization).to have_received(:generate_document_number_prefix) + end + end + + context 'with a persisted record' do + let(:organization) { create(:organization) } + + it 'does not call #generate_document_number_prefix' do + expect(organization).not_to have_received(:generate_document_number_prefix) + end + end + end + + describe '#generate_document_number_prefix' do + subject { organization.send(:generate_document_number_prefix) } + + let(:organization) { create(:organization) } + let(:prefix) { "#{organization.name.first(3).upcase}-#{organization.id.last(4).upcase}" } + + before { organization.update!(document_number_prefix: 'invalid') } + + it 'sets document number prefix of organization' do + expect { subject } + .to change(organization, :document_number_prefix) + .to prefix end end diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb index f5606dca2a59..8bf20d20ecf2 100644 --- a/spec/models/webhook_spec.rb +++ b/spec/models/webhook_spec.rb @@ -5,10 +5,9 @@ RSpec.describe Webhook, type: :model do subject(:webhook) { create(:webhook) } - let(:organization) { create(:organization, name: "sefsefs", api_key: 'the_key') } - it { is_expected.to belong_to(:webhook_endpoint) } it { is_expected.to belong_to(:object).optional } + it { is_expected.to have_one(:organization).through(:webhook_endpoint) } describe '#payload' do it 'returns a hash' do @@ -56,8 +55,8 @@ describe '#hmac_signature' do it 'generates a correct hmac signature' do - webhook.webhook_endpoint.organization.api_key = 'the_key' - hmac = OpenSSL::HMAC.digest('sha-256', 'the_key', webhook.payload.to_json) + api_key_value = webhook.organization.api_keys.first.value + hmac = OpenSSL::HMAC.digest('sha-256', api_key_value, webhook.payload.to_json) base64_hmac = Base64.strict_encode64(hmac) expect(base64_hmac).to eq(webhook.hmac_signature) diff --git a/spec/requests/api/base_controller_spec.rb b/spec/requests/api/base_controller_spec.rb index 7ebc89b35abd..0a4d289c31e5 100644 --- a/spec/requests/api/base_controller_spec.rb +++ b/spec/requests/api/base_controller_spec.rb @@ -14,10 +14,10 @@ def create end end - let(:organization) { create(:organization) } + let(:api_key) { create(:api_key) } it 'sets the context source to api' do - request.headers['Authorization'] = "Bearer #{organization.api_key}" + request.headers['Authorization'] = "Bearer #{api_key.value}" get :index @@ -25,25 +25,40 @@ def create end describe 'authenticate' do - it 'validates the organization api key' do - request.headers['Authorization'] = "Bearer #{organization.api_key}" - + before do + request.headers['Authorization'] = "Bearer #{token}" get :index + end + + context 'with valid authorization header' do + context 'when authorization token matching api key' do + let(:token) { api_key.value } + + it 'returns success response' do + expect(response).to have_http_status(:success) + end + end - expect(response).to have_http_status(:success) + context 'when authorization token matching organization api_key' do + let(:token) { create(:organization, api_key: 'api_key').api_key } + + it 'returns success response' do + expect(response).to have_http_status(:success) + end + end end - context 'without authentication header' do - it 'returns an authentication error' do - get :index + context 'with invalid authentication header' do + let(:token) { SecureRandom.uuid } + it 'returns an authentication error' do expect(response).to have_http_status(:unauthorized) end end end it 'catches the missing parameters error' do - request.headers['Authorization'] = "Bearer #{organization.api_key}" + request.headers['Authorization'] = "Bearer #{api_key.value}" post :create diff --git a/spec/scenarios/subscriptions/multiple_upgrade_spec.rb b/spec/scenarios/subscriptions/multiple_upgrade_spec.rb index d6a652f03c21..5f50b7826125 100644 --- a/spec/scenarios/subscriptions/multiple_upgrade_spec.rb +++ b/spec/scenarios/subscriptions/multiple_upgrade_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe 'Multiple Subscription Upgrade Scenario', :scenarios, type: :request do - let(:organization) { create(:organization, webhook_url: false, email_settings: []) } + let(:organization) { create(:organization, webhook_url: nil, email_settings: []) } let(:customer) { create(:customer, organization:) } let(:tax) { create(:tax, organization:, rate: 25, applied_to_organization: true) } diff --git a/spec/services/organizations/create_service_spec.rb b/spec/services/organizations/create_service_spec.rb new file mode 100644 index 000000000000..1de54ab9d986 --- /dev/null +++ b/spec/services/organizations/create_service_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Organizations::CreateService, type: :service do + describe '#call' do + subject(:service_result) { described_class.call(params) } + + context 'with valid params' do + let(:params) { attributes_for(:organization) } + + it 'creates an organization with provided params' do + expect { service_result }.to change(Organization, :count).by(1) + + expect(service_result.organization) + .to be_persisted + .and have_attributes(params) + end + + it 'creates an API key for created organization' do + expect { service_result }.to change(ApiKey, :count).by(1) + + expect(service_result.organization.api_keys).to all( + be_persisted.and(have_attributes(organization: service_result.organization)) + ) + end + end + + context 'with invalid params' do + let(:params) { {} } + + it 'does not create an organization' do + expect { service_result }.not_to change(Organization, :count) + end + + it 'does not create an API key' do + expect { service_result }.not_to change(ApiKey, :count) + end + + it 'returns an error' do + aggregate_failures do + expect(service_result).not_to be_success + expect(service_result.error).to be_a(BaseService::ValidationFailure) + expect(service_result.error.messages[:name]).to eq(["value_is_mandatory"]) + end + end + end + end +end diff --git a/spec/services/users_service_spec.rb b/spec/services/users_service_spec.rb index 08aed8af341f..818994217aa4 100644 --- a/spec/services/users_service_spec.rb +++ b/spec/services/users_service_spec.rb @@ -33,12 +33,11 @@ result = user_service.register('email', 'password', 'organization_name') expect(result.user).to be_present expect(result.membership).to be_present - expect(result.organization).to be_present expect(result.token).to be_present - org = Organization.find(result.organization.id) - expect(org.document_number_prefix).to eq("#{org.name.first(3).upcase}-#{org.id.last(4).upcase}") - expect(org.document_numbering).to eq('per_organization') + expect(result.organization) + .to be_present + .and have_attributes(name: 'organization_name', document_numbering: 'per_organization') end context 'when user already exists' do diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index 156dab9ac2ed..1e7db1829b5a 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -32,6 +32,6 @@ def json def set_headers(organization, headers) headers['Content-Type'] = 'application/json' headers['Accept'] = 'application/json' - headers['Authorization'] = "Bearer #{organization.api_key}" + headers['Authorization'] = "Bearer #{organization.api_keys.first.value}" end end