From ded236ffb667ecf48ee244098e1f2ea0f4b334ed Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Mon, 9 Sep 2024 21:13:04 -0400 Subject: [PATCH 01/21] WIP for app registry client --- Gemfile | 9 ++++++--- Gemfile.lock | 12 ++++++++++++ app/interactors/authorize_request.rb | 16 +++++++++++++++- app/interactors/fail_on_error.rb | 1 + app/interactors/issue_cert.rb | 3 ++- app/interactors/refresh_domain.rb | 14 ++++++++++++++ app/lib/services/domain_ownership_service.rb | 15 ++++++--------- app/models/domain.rb | 9 +++++---- db/migrate/20240904175652_create_domains.rb | 2 +- 9 files changed, 62 insertions(+), 19 deletions(-) create mode 100644 app/interactors/refresh_domain.rb diff --git a/Gemfile b/Gemfile index c062096..abcd601 100644 --- a/Gemfile +++ b/Gemfile @@ -6,8 +6,7 @@ gem "rails", "~> 7.2.1" gem "sqlite3", ">= 1.4" # Use the Puma web server [https://github.com/puma/puma] gem "puma", ">= 5.0" -# Build JSON APIs with ease [https://github.com/rails/jbuilder] -# gem "jbuilder" + # Use Redis adapter to run Action Cable in production # gem "redis", ">= 4.0.1" @@ -38,9 +37,13 @@ gem "vault" # Use the jwt gem to decode access tokens gem "jwt" -# Use the jbuilder gem +# Use the jbuilder gem to render JSON views gem "jbuilder" +# Use the faraday gem for http client operations +gem "faraday" +gem "faraday-retry" + group :development, :test do # See https://guides.rubyonrails.org/debugging_rails_applications.html#debugging-with-the-debug-gem gem "debug", platforms: %i[ mri mswin ], require: "debug/prelude" diff --git a/Gemfile.lock b/Gemfile.lock index 6ab9662..61be8b8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -91,6 +91,13 @@ GEM reline (>= 0.3.8) drb (2.2.1) erubi (1.13.0) + faraday (2.11.0) + faraday-net_http (>= 2.0, < 3.4) + logger + faraday-net_http (3.3.0) + net-http + faraday-retry (2.2.1) + faraday (~> 2.0) globalid (1.2.1) activesupport (>= 6.1) i18n (1.14.5) @@ -120,6 +127,8 @@ GEM mini_mime (1.1.5) minitest (5.25.1) msgpack (1.7.2) + net-http (0.4.1) + uri net-imap (0.4.14) date net-protocol @@ -245,6 +254,7 @@ GEM tzinfo (2.0.6) concurrent-ruby (~> 1.0) unicode-display_width (2.5.0) + uri (0.13.1) useragent (0.16.10) vault (0.18.2) aws-sigv4 @@ -274,6 +284,8 @@ DEPENDENCIES bootsnap brakeman debug + faraday + faraday-retry interactor (~> 3.0) jbuilder jwt diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index bbcef74..925eae3 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -3,6 +3,20 @@ class AuthorizeRequest include FailOnError def call - Services::DomainOwnershipService.new.authorize!(context.identity, context.request) + authorize!(context.identity, context.request) end + + private + + def authorize!(identity, cert_req) + cert_req.fqdns.each do |fqdn| + domain = Domain.where(fqdn: fqdn).first + raise AuthError unless domain.present? && + (domain.owner == identity.subject || + (domain.group_delegation && + (domain.groups & identity.groups).any?)) + end + nil + end + end diff --git a/app/interactors/fail_on_error.rb b/app/interactors/fail_on_error.rb index 76d5723..89249f9 100644 --- a/app/interactors/fail_on_error.rb +++ b/app/interactors/fail_on_error.rb @@ -5,6 +5,7 @@ module FailOnError around do |interactor| interactor.call rescue => e + Rails.logger.error("Error in #{self.class.name}: #{e.class.name} - #{e.message}") context.fail!(error: e) end end diff --git a/app/interactors/issue_cert.rb b/app/interactors/issue_cert.rb index 9a80f80..667043f 100644 --- a/app/interactors/issue_cert.rb +++ b/app/interactors/issue_cert.rb @@ -1,5 +1,6 @@ class IssueCert include Interactor::Organizer + include FailOnError - organize AuthorizeRequest, ObtainCert, Log + organize RefreshDomain, AuthorizeRequest, ObtainCert, Log end diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb new file mode 100644 index 0000000..b0011d5 --- /dev/null +++ b/app/interactors/refresh_domain.rb @@ -0,0 +1,14 @@ +class RefreshDomain + include Interactor + + def call + domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.fqdn) + Domain.first_or_create(fqdn: context.request.fqdn).update!( + group_delegation: domain_info["ownerDelegatedRequestsToTeam"] + groups: domain_info["autoApprovedGroups"] + users: domain_info["autoApprovedServiceAccounts"] + ) + rescue => e + Rails.logger.warn("Continuing after error in #{self.class.name}: #{e.class.name}: #{e.message}") + end +end diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index 1035e37..3b94ea1 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -1,14 +1,11 @@ module Services class DomainOwnershipService - def authorize!(identity, cert_req) - cert_req.fqdns.each do |fqdn| - domain = Domain.where(fqdn: fqdn).first - raise AuthError unless domain.present? && - (domain.owner == identity.subject || - (domain.group_delegation && - (domain.groups & identity.groups).any?)) - end - nil + + def initialize end + + def get_domain_info + end + end end diff --git a/app/models/domain.rb b/app/models/domain.rb index b0bd65c..cbeb688 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -1,10 +1,11 @@ class Domain < ApplicationRecord - serialize :groups, coder: YAML, type: Array - before_save :clean_groups + serialize :groups, :users, coder: YAML, type: Array + before_save :clean_users_groups - validates :fqdn, :owner, presence: true + validates :fqdn, presence: true - def clean_groups + def clean_users_groups self.groups = groups.sort.uniq + self.users = users.sort.uniq end end diff --git a/db/migrate/20240904175652_create_domains.rb b/db/migrate/20240904175652_create_domains.rb index 1225da9..ed1d286 100644 --- a/db/migrate/20240904175652_create_domains.rb +++ b/db/migrate/20240904175652_create_domains.rb @@ -2,7 +2,7 @@ class CreateDomains < ActiveRecord::Migration[7.2] def change create_table :domains do |t| t.string :fqdn, null: false - t.string :owner, null: false + t.text :users t.text :groups t.boolean :group_delegation, default: false t.timestamps From 9ba8f9e699aba7e2e46de1353688d6dc75ad8452 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Tue, 10 Sep 2024 10:17:32 -0400 Subject: [PATCH 02/21] Convert Domain.owner -> users --- app/interactors/authorize_request.rb | 16 ++----- app/interactors/fail_on_error.rb | 2 + app/interactors/refresh_domain.rb | 13 ++++-- app/lib/services/domain_ownership_service.rb | 2 - app/models/domain.rb | 12 ++--- db/schema.rb | 2 +- db/seeds.rb | 2 +- test/fixtures/domains.yml | 12 +++-- test/interactors/authorize_request_test.rb | 44 ++++++++++--------- .../services/domain_ownership_service_test.rb | 31 ------------- test/models/domain_test.rb | 16 +++---- 11 files changed, 60 insertions(+), 92 deletions(-) diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index 925eae3..c9a77af 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -3,20 +3,12 @@ class AuthorizeRequest include FailOnError def call - authorize!(context.identity, context.request) - end - - private - - def authorize!(identity, cert_req) - cert_req.fqdns.each do |fqdn| + context.request.fqdns.each do |fqdn| domain = Domain.where(fqdn: fqdn).first - raise AuthError unless domain.present? && - (domain.owner == identity.subject || - (domain.group_delegation && - (domain.groups & identity.groups).any?)) + raise AuthError unless domain.present? + raise AuthError unless (domain.users_array & [ context.identity.subject ]).any? || + (domain.group_delegation && (domain.groups_array & context.identity.groups).any?) end nil end - end diff --git a/app/interactors/fail_on_error.rb b/app/interactors/fail_on_error.rb index 89249f9..d164847 100644 --- a/app/interactors/fail_on_error.rb +++ b/app/interactors/fail_on_error.rb @@ -4,6 +4,8 @@ module FailOnError included do around do |interactor| interactor.call + rescue Interactor::Failure => e + raise e rescue => e Rails.logger.error("Error in #{self.class.name}: #{e.class.name} - #{e.message}") context.fail!(error: e) diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb index b0011d5..58ef8d1 100644 --- a/app/interactors/refresh_domain.rb +++ b/app/interactors/refresh_domain.rb @@ -3,9 +3,16 @@ class RefreshDomain def call domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.fqdn) - Domain.first_or_create(fqdn: context.request.fqdn).update!( - group_delegation: domain_info["ownerDelegatedRequestsToTeam"] - groups: domain_info["autoApprovedGroups"] + domain_record = Domain.first_or_create(fqdn: context.request.fqdn) + + if !domain_info || domain_info["isDeleted"] + domain_record.delete + return + end + + domain_record.update!( + group_delegation: domain_info["ownerDelegatedRequestsToTeam"], + groups: domain_info["autoApprovedGroups"], users: domain_info["autoApprovedServiceAccounts"] ) rescue => e diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index 3b94ea1..8dea08d 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -1,11 +1,9 @@ module Services class DomainOwnershipService - def initialize end def get_domain_info end - end end diff --git a/app/models/domain.rb b/app/models/domain.rb index cbeb688..72003bb 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -1,11 +1,11 @@ class Domain < ApplicationRecord - serialize :groups, :users, coder: YAML, type: Array - before_save :clean_users_groups - validates :fqdn, presence: true - def clean_users_groups - self.groups = groups.sort.uniq - self.users = users.sort.uniq + def groups_array + (groups || "").split(",").sort.uniq + end + + def users_array + (users || "").split(",").sort.uniq end end diff --git a/db/schema.rb b/db/schema.rb index 4e5f14d..e14708e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -13,7 +13,7 @@ ActiveRecord::Schema[7.2].define(version: 2024_09_04_175652) do create_table "domains", force: :cascade do |t| t.string "fqdn", null: false - t.string "owner", null: false + t.text "users" t.text "groups" t.boolean "group_delegation", default: false t.datetime "created_at", null: false diff --git a/db/seeds.rb b/db/seeds.rb index 11fded0..4c0f4e6 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -10,5 +10,5 @@ # this seed is for development only if Rails.env.development? - Domain.first_or_create!(fqdn: "example.com", owner: "john.doe@example.com") + Domain.first_or_create!(fqdn: "example.com", users: "john.doe@example.com") end diff --git a/test/fixtures/domains.yml b/test/fixtures/domains.yml index a0ecfad..59e3aa0 100644 --- a/test/fixtures/domains.yml +++ b/test/fixtures/domains.yml @@ -1,18 +1,16 @@ owner_match: fqdn: example.com - owner: john.doe@example.com + users: john.doe@example.com group_delegation: false group_match: fqdn: example2.com - owner: some.other@example2.com + users: some.other@example2.com group_delegation: true - groups: - - "group1" + groups: group1 no_match: fqdn: example3.com - owner: some.other@example2.com + users: some.other@example2.com group_delegation: true - groups: - - "group3" + groups: group3 diff --git a/test/interactors/authorize_request_test.rb b/test/interactors/authorize_request_test.rb index 30cd3e6..0af0d3f 100644 --- a/test/interactors/authorize_request_test.rb +++ b/test/interactors/authorize_request_test.rb @@ -3,31 +3,35 @@ class AuthorizeRequestTest < ActiveSupport::TestCase def setup @domain = domains(:group_match) - @identity = Identity.new(subject: @domain.owner) + @identity = Identity.new(subject: @domain.users_array.first) @cr = CertIssueRequest.new(common_name: @domain.fqdn) @interactor = AuthorizeRequest end - test "successful call" do - request = CertIssueRequest.new(common_name: @domain.fqdn) - srv = Minitest::Mock.new - srv.expect :authorize!, nil, [ @identity, @cr ] - Services::DomainOwnershipService.stub :new, srv do - context = @interactor.call(identity: @identity, request: @cr) - assert context.success? - end + test ".call with matching owner" do + rslt = @interactor.call(identity: @identity, request: @cr) + assert rslt.success? end - test "unsuccessful call" do - request = CertIssueRequest.new(common_name: @domain.fqdn) - srv = Services::DomainOwnershipService.new - Services::DomainOwnershipService.stub :new, srv do - err = ->(_, _) { raise AuthError.new "no can do" } - srv.stub :authorize!, err do - context = @interactor.call(identity: @identity, request: @cr) - assert_not context.success? - assert_kind_of AuthError, context.error - end - end + test ".call with non-matching owner" do + @identity.subject = "different_owner@example.com" + rslt = @interactor.call(identity: @identity, request: @cr) + assert_not rslt.success? + assert_kind_of AuthError, rslt.error + end + + test ".call with matching group" do + @domain.update(users: "different_owner@example.com") + @identity.groups = @domain.groups_array + rslt = @interactor.call(identity: @identity, request: @cr) + assert rslt.success? + end + + test ".call with non-matching group" do + @domain.update(users: "different_owner@example.com") + @identity.groups = [ "different_group" ] + rslt = @interactor.call(identity: @identity, request: @cr) + assert_not rslt.success? + assert_kind_of AuthError, rslt.error end end diff --git a/test/lib/services/domain_ownership_service_test.rb b/test/lib/services/domain_ownership_service_test.rb index 4f5815a..a3c856f 100644 --- a/test/lib/services/domain_ownership_service_test.rb +++ b/test/lib/services/domain_ownership_service_test.rb @@ -1,35 +1,4 @@ require "test_helper" class DomainOwnershipServiceTest < ActiveSupport::TestCase - def setup - @domain = domains(:group_match) - @identity = Identity.new(subject: @domain.owner) - @cr = CertIssueRequest.new(common_name: @domain.fqdn) - @ds = Services::DomainOwnershipService.new - end - - test "#authorize! with matching owner" do - assert_nil(@ds.authorize!(@identity, @cr)) - end - - test "#authorize! with non-matching owner" do - @identity.subject = "different_owner@example.com" - assert_raises(AuthError) do - @ds.authorize!(@identity, @cr) - end - end - - test "#authorize! with matching group" do - @domain.update(owner: "different_owner@example.com") - @identity.groups = @domain.groups - assert_nil(@ds.authorize!(@identity, @cr)) - end - - test "#authorize! with non-matching group" do - @domain.update(owner: "different_owner@example.com") - @identity.groups = [ "different_group" ] - assert_raises(AuthError) do - @ds.authorize!(@identity, @cr) - end - end end diff --git a/test/models/domain_test.rb b/test/models/domain_test.rb index 059b142..7f4f215 100644 --- a/test/models/domain_test.rb +++ b/test/models/domain_test.rb @@ -5,7 +5,7 @@ class DomainTest < ActiveSupport::TestCase def setup @attributes = { fqdn: "example4.com", - owner: "john.doe@example.com" + users: "john.doe@example.com" } @domain = Domain.new(@attributes) end @@ -26,15 +26,13 @@ def setup assert_includes @domain.errors[:fqdn], "can't be blank" end - test "#valid? should require an owner" do - @domain.owner = nil - assert_not @domain.valid? - assert_includes @domain.errors[:owner], "can't be blank" + test "#groups_array should sort dedupe groups" do + @domain.groups = "two,two,one" + assert_equal [ "one", "two" ], @domain.groups_array end - test "before_save should sort and dedupe groups" do - @domain.groups = [ "two", "two", "one" ] - @domain.save - assert_equal [ "one", "two" ], @domain.groups + test "#users_array should sort dedupe users" do + @domain.users = "two,two,one" + assert_equal [ "one", "two" ], @domain.users_array end end From 4397c5e9f58466d1959df1965ca5e9278ffde55a Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Tue, 10 Sep 2024 10:34:22 -0400 Subject: [PATCH 03/21] Simplify domain ownership verificaiton --- app/interactors/authorize_request.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index c9a77af..a885f6d 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -4,10 +4,9 @@ class AuthorizeRequest def call context.request.fqdns.each do |fqdn| - domain = Domain.where(fqdn: fqdn).first - raise AuthError unless domain.present? - raise AuthError unless (domain.users_array & [ context.identity.subject ]).any? || - (domain.group_delegation && (domain.groups_array & context.identity.groups).any?) + domain = Domain.where(fqdn: fqdn).first! + raise AuthError unless domain.users_array.include?(context.identity.subject) || + (domain.group_delegation? && (domain.groups_array & context.identity.groups).any?) end nil end From 902fee087b38e1968e0cb31fc3bdb9fca74d8e20 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Tue, 10 Sep 2024 10:48:31 -0400 Subject: [PATCH 04/21] Test tweaks --- test/interactors/authorize_request_test.rb | 2 +- test/models/domain_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/interactors/authorize_request_test.rb b/test/interactors/authorize_request_test.rb index 0af0d3f..b878f3d 100644 --- a/test/interactors/authorize_request_test.rb +++ b/test/interactors/authorize_request_test.rb @@ -22,7 +22,7 @@ def setup test ".call with matching group" do @domain.update(users: "different_owner@example.com") - @identity.groups = @domain.groups_array + @identity.groups = [ @domain.groups_array.first ] rslt = @interactor.call(identity: @identity, request: @cr) assert rslt.success? end diff --git a/test/models/domain_test.rb b/test/models/domain_test.rb index 7f4f215..ec8f8c1 100644 --- a/test/models/domain_test.rb +++ b/test/models/domain_test.rb @@ -26,12 +26,12 @@ def setup assert_includes @domain.errors[:fqdn], "can't be blank" end - test "#groups_array should sort dedupe groups" do + test "#groups_array should convert to array, sort, and dedupe groups" do @domain.groups = "two,two,one" assert_equal [ "one", "two" ], @domain.groups_array end - test "#users_array should sort dedupe users" do + test "#users_array should convert to array, sort, and dedupe users" do @domain.users = "two,two,one" assert_equal [ "one", "two" ], @domain.users_array end From 0ffc7bbb285ddcc8cb77a040b0378b0eb1e25c9f Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Tue, 10 Sep 2024 11:23:28 -0400 Subject: [PATCH 05/21] WIP wiring in app-registry --- app/interactors/refresh_domain.rb | 8 ++++---- app/lib/services/auth_service.rb | 2 +- app/lib/services/domain_ownership_service.rb | 20 ++++++++++++++++++++ app/lib/services/vault_service.rb | 6 +++--- config/astral.yml | 2 ++ db/migrate/20240904175652_create_domains.rb | 3 +-- 6 files changed, 31 insertions(+), 10 deletions(-) diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb index 58ef8d1..0c1c9b9 100644 --- a/app/interactors/refresh_domain.rb +++ b/app/interactors/refresh_domain.rb @@ -5,15 +5,15 @@ def call domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.fqdn) domain_record = Domain.first_or_create(fqdn: context.request.fqdn) - if !domain_info || domain_info["isDeleted"] + if !domain_info domain_record.delete return end domain_record.update!( - group_delegation: domain_info["ownerDelegatedRequestsToTeam"], - groups: domain_info["autoApprovedGroups"], - users: domain_info["autoApprovedServiceAccounts"] + group_delegation: domain_info.group_delegation, + groups: domain_info.groups, + users: domain_info.users ) rescue => e Rails.logger.warn("Continuing after error in #{self.class.name}: #{e.class.name}: #{e.message}") diff --git a/app/lib/services/auth_service.rb b/app/lib/services/auth_service.rb index 2d1bd7b..1006e8f 100644 --- a/app/lib/services/auth_service.rb +++ b/app/lib/services/auth_service.rb @@ -19,7 +19,7 @@ def authorize!(identity, cert_issue_req) def decode(token) # Decode a JWT access token using the configured base. - body = JWT.decode(token, Rails.application.config.astral[:jwt_signing_key])[0] + body = JWT.decode(token, Rails.application.configuration.astral[:jwt_signing_key])[0] Identity.new(body) rescue => e Rails.logger.warn "Unable to decode token: #{e}" diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index 8dea08d..b2e1cb9 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -1,9 +1,29 @@ module Services class DomainOwnershipService + attr_reader :client + def initialize + @client = Faraday.new(url: Rails.configuration.astral[:app_registry_uri]) do |faraday| + faraday.response :raise_error, include_request: true + end end def get_domain_info end + + private + + def convert(input) + if !input || input["isDeleted"] + return nil + end + + OpenStruct.new( + fqdn: domain_info["fullyQualifiedDomainName"], + group_delegation: domain_info["ownerDelegatedRequestsToTeam"], + groups: domain_info["autoApprovedGroups"], + users: domain_info["autoApprovedServiceAccounts"] + ) + end end end diff --git a/app/lib/services/vault_service.rb b/app/lib/services/vault_service.rb index d6e77f8..46327ea 100644 --- a/app/lib/services/vault_service.rb +++ b/app/lib/services/vault_service.rb @@ -3,15 +3,15 @@ class VaultService def initialize # TODO create a new token for use in the session @client = Vault::Client.new( - address: Rails.application.config.astral[:vault_addr], - token: Rails.application.config.astral[:vault_token] + address: Rails.application.configuration.astral[:vault_addr], + token: Rails.application.configuration.astral[:vault_token] ) end def issue_cert(cert_issue_request) opts = cert_issue_request.attributes # Generate the TLS certificate using the intermediate CA - tls_cert = @client.logical.write(Rails.application.config.astral[:vault_cert_path], opts) + tls_cert = @client.logical.write(Rails.application.configuration.astral[:vault_cert_path], opts) OpenStruct.new tls_cert.data end end diff --git a/config/astral.yml b/config/astral.yml index c708d75..0438643 100644 --- a/config/astral.yml +++ b/config/astral.yml @@ -4,6 +4,8 @@ shared: vault_cert_path: "pki_int/issue/learn" jwt_signing_key: <%= ENV["JWT_SIGNING_KEY"] %> cert_ttl: <%= ENV["CERT_TTL"] %> + app_registry_addr: <%= ENV["APP_REGISTRY_ADDR"] %> + app_registry_token: <%= ENV["APP_REGISTRY_TOKEN"] %> test: cert_ttl: <%= 24.hours.in_seconds %> diff --git a/db/migrate/20240904175652_create_domains.rb b/db/migrate/20240904175652_create_domains.rb index ed1d286..9070c1e 100644 --- a/db/migrate/20240904175652_create_domains.rb +++ b/db/migrate/20240904175652_create_domains.rb @@ -1,12 +1,11 @@ class CreateDomains < ActiveRecord::Migration[7.2] def change create_table :domains do |t| - t.string :fqdn, null: false + t.string :fqdn, null: false, index: { unique: true } t.text :users t.text :groups t.boolean :group_delegation, default: false t.timestamps - t.index :fqdn, unique: true end end end From 751f02822c0a9eae751e3544720c4ee35cd0caee Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Wed, 11 Sep 2024 12:24:27 -0400 Subject: [PATCH 06/21] Add json-server mock for AppRegistry --- .devcontainer/app_reg_db.json | 11 ++++++ .devcontainer/app_reg_routes.json | 3 ++ .devcontainer/docker-compose.yml | 16 +++++++++ app/interactors/refresh_domain.rb | 1 - app/lib/services/auth_service.rb | 2 +- app/lib/services/domain_ownership_service.rb | 36 ++++++++++++++++---- app/lib/services/vault_service.rb | 6 ++-- config/astral.yml | 3 ++ 8 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 .devcontainer/app_reg_db.json create mode 100644 .devcontainer/app_reg_routes.json diff --git a/.devcontainer/app_reg_db.json b/.devcontainer/app_reg_db.json new file mode 100644 index 0000000..c5a452e --- /dev/null +++ b/.devcontainer/app_reg_db.json @@ -0,0 +1,11 @@ +{ + "domain-names": [ + { + "id": "example.com", + "fullyQualifiedDomainName": "example.com", + "ownerDelegatedRequestToTeam": true, + "autoApprovedGroups": "group1", + "autoApprovedServiceAccounts": "john.doe@example.com" + } + ] +} \ No newline at end of file diff --git a/.devcontainer/app_reg_routes.json b/.devcontainer/app_reg_routes.json new file mode 100644 index 0000000..ba3845e --- /dev/null +++ b/.devcontainer/app_reg_routes.json @@ -0,0 +1,3 @@ +{ + "/api/v1beta1/*": "/$1" +} diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index 56426d5..350315d 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -19,6 +19,8 @@ services: VAULT_ADDR: http://10.1.10.100:8200 VAULT_TOKEN: root_token JWT_SIGNING_KEY: jwt_secret + APP_REGISTRY_ADDR: http://10.1.10.150:8800 + APP_REGISTRY_TOKEN: app_reg_token vault: image: hashicorp/vault:latest @@ -32,6 +34,20 @@ services: astral: ipv4_address: "10.1.10.100" + app_registry: + image: node:latest + restart: unless-stopped + ports: + - 8800:8800 + volumes: + - .:/data + networks: + astral: + ipv4_address: "10.1.10.150" + command: > + sh -c "npm install -g json-server@0.17.4 && + json-server /data/app_reg_db.json --routes /data/app_reg_routes.json --port 8800 --host 0.0.0.0" + networks: astral: ipam: diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb index 0c1c9b9..f9ea517 100644 --- a/app/interactors/refresh_domain.rb +++ b/app/interactors/refresh_domain.rb @@ -4,7 +4,6 @@ class RefreshDomain def call domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.fqdn) domain_record = Domain.first_or_create(fqdn: context.request.fqdn) - if !domain_info domain_record.delete return diff --git a/app/lib/services/auth_service.rb b/app/lib/services/auth_service.rb index 1006e8f..c0b723f 100644 --- a/app/lib/services/auth_service.rb +++ b/app/lib/services/auth_service.rb @@ -19,7 +19,7 @@ def authorize!(identity, cert_issue_req) def decode(token) # Decode a JWT access token using the configured base. - body = JWT.decode(token, Rails.application.configuration.astral[:jwt_signing_key])[0] + body = JWT.decode(token, Rails.configuration.astral[:jwt_signing_key])[0] Identity.new(body) rescue => e Rails.logger.warn "Unable to decode token: #{e}" diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index b2e1cb9..33bd7fc 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -3,12 +3,17 @@ class DomainOwnershipService attr_reader :client def initialize - @client = Faraday.new(url: Rails.configuration.astral[:app_registry_uri]) do |faraday| + @client = Faraday.new(ssl: ssl_opts, url: Rails.configuration.astral[:app_registry_addr]) do |faraday| + faraday.request :authorization, "Bearer", -> { Rails.configuration.astral[:app_registry_token] } + faraday.request :retry, retry_opts + faraday.response :json faraday.response :raise_error, include_request: true end end - def get_domain_info + def get_domain_info(fqdn) + rslt = client.get("/api/v1beta1/domain-names/#{fqdn}").body + convert(rslt) end private @@ -19,11 +24,28 @@ def convert(input) end OpenStruct.new( - fqdn: domain_info["fullyQualifiedDomainName"], - group_delegation: domain_info["ownerDelegatedRequestsToTeam"], - groups: domain_info["autoApprovedGroups"], - users: domain_info["autoApprovedServiceAccounts"] - ) + fqdn: domain_info["fullyQualifiedDomainName"], + group_delegation: domain_info["ownerDelegatedRequestsToTeam"], + groups: domain_info["autoApprovedGroups"], + users: domain_info["autoApprovedServiceAccounts"] + ) + end + + def ssl_opts + { + ca_file: Rails.configuration.astral[:app_registry_ca_file], + client_cert: Rails.configuration.astral[:app_registry_client_cert], + client_key: Rails.configuration.astral[:app_registry_client_key] + } + end + + def retry_opts + { + max: 3, + interval: 0.05, + interval_randomness: 0.5, + backoff_factor: 2 + } end end end diff --git a/app/lib/services/vault_service.rb b/app/lib/services/vault_service.rb index 46327ea..3e8b383 100644 --- a/app/lib/services/vault_service.rb +++ b/app/lib/services/vault_service.rb @@ -3,15 +3,15 @@ class VaultService def initialize # TODO create a new token for use in the session @client = Vault::Client.new( - address: Rails.application.configuration.astral[:vault_addr], - token: Rails.application.configuration.astral[:vault_token] + address: Rails.configuration.astral[:vault_addr], + token: Rails.configuration.astral[:vault_token] ) end def issue_cert(cert_issue_request) opts = cert_issue_request.attributes # Generate the TLS certificate using the intermediate CA - tls_cert = @client.logical.write(Rails.application.configuration.astral[:vault_cert_path], opts) + tls_cert = @client.logical.write(Rails.configuration.astral[:vault_cert_path], opts) OpenStruct.new tls_cert.data end end diff --git a/config/astral.yml b/config/astral.yml index 0438643..f4d61c6 100644 --- a/config/astral.yml +++ b/config/astral.yml @@ -6,6 +6,9 @@ shared: cert_ttl: <%= ENV["CERT_TTL"] %> app_registry_addr: <%= ENV["APP_REGISTRY_ADDR"] %> app_registry_token: <%= ENV["APP_REGISTRY_TOKEN"] %> + app_registry_ca_file: <%= ENV["APP_REGISTRY_CA_FILE"] %> + app_registry_client_cert: <%= ENV["APP_REGISTRY_CLIENT_CERT"] %> + app_registry_client_key: <%= ENV["APP_REGISTRY_CLIENT_KEY"] %> test: cert_ttl: <%= 24.hours.in_seconds %> From f42ba46a4026a43f03871eca45bc565c01e659c9 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Wed, 11 Sep 2024 17:20:46 -0400 Subject: [PATCH 07/21] tweak fixtures to show multiple users/groups --- test/fixtures/domains.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/fixtures/domains.yml b/test/fixtures/domains.yml index 59e3aa0..82dc778 100644 --- a/test/fixtures/domains.yml +++ b/test/fixtures/domains.yml @@ -1,16 +1,16 @@ owner_match: fqdn: example.com - users: john.doe@example.com + users: john.doe@example.com,some.other@example.com group_delegation: false group_match: fqdn: example2.com users: some.other@example2.com group_delegation: true - groups: group1 + groups: group1,group2 no_match: fqdn: example3.com - users: some.other@example2.com + users: some.other@example2.com,yet.another@example2.com group_delegation: true - groups: group3 + groups: group3,group4 From 12e78333c94e5e2fd4d1dbf9c1c8d1c157541781 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Wed, 11 Sep 2024 17:36:44 -0400 Subject: [PATCH 08/21] Testing for DomainOwnershipService --- .devcontainer/app_reg_db.json | 2 +- app/lib/services/domain_ownership_service.rb | 6 ++++-- .../services/domain_ownership_service_test.rb | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.devcontainer/app_reg_db.json b/.devcontainer/app_reg_db.json index c5a452e..3f08c71 100644 --- a/.devcontainer/app_reg_db.json +++ b/.devcontainer/app_reg_db.json @@ -3,7 +3,7 @@ { "id": "example.com", "fullyQualifiedDomainName": "example.com", - "ownerDelegatedRequestToTeam": true, + "ownerDelegatedRequestsToTeam": true, "autoApprovedGroups": "group1", "autoApprovedServiceAccounts": "john.doe@example.com" } diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index 33bd7fc..80db2e4 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -14,12 +14,14 @@ def initialize def get_domain_info(fqdn) rslt = client.get("/api/v1beta1/domain-names/#{fqdn}").body convert(rslt) + rescue Faraday::ResourceNotFound => e + nil end private - def convert(input) - if !input || input["isDeleted"] + def convert(domain_info) + if !domain_info || domain_info["isDeleted"] return nil end diff --git a/test/lib/services/domain_ownership_service_test.rb b/test/lib/services/domain_ownership_service_test.rb index a3c856f..e87481b 100644 --- a/test/lib/services/domain_ownership_service_test.rb +++ b/test/lib/services/domain_ownership_service_test.rb @@ -1,4 +1,21 @@ require "test_helper" class DomainOwnershipServiceTest < ActiveSupport::TestCase + setup do + @subject = Services::DomainOwnershipService.new + end + + test "#get_domain_info fetches from configured api server" do + domain_info = @subject.get_domain_info(domains(:owner_match).fqdn) + assert_not_nil domain_info + assert_equal "group1", domain_info.groups + assert_equal "john.doe@example.com", domain_info.users + assert_equal "example.com", domain_info.fqdn + assert domain_info.group_delegation + end + + test "#get_domain_info returns nil for unmatched fqdn" do + domain_info = @subject.get_domain_info(domains(:group_match).fqdn) + assert_nil domain_info + end end From d0c9bd3f73eebb609fe86a44d4460be8daf4befa Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Wed, 11 Sep 2024 21:37:09 -0400 Subject: [PATCH 09/21] New test for RefreshDomain interactor --- .devcontainer/app_reg_db.json | 7 ++++ app/interactors/authorize_request.rb | 6 ++- app/interactors/refresh_domain.rb | 6 +-- test/interactors/refresh_domain_test.rb | 41 +++++++++++++++++++ .../services/domain_ownership_service_test.rb | 6 +-- 5 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 test/interactors/refresh_domain_test.rb diff --git a/.devcontainer/app_reg_db.json b/.devcontainer/app_reg_db.json index 3f08c71..5197214 100644 --- a/.devcontainer/app_reg_db.json +++ b/.devcontainer/app_reg_db.json @@ -6,6 +6,13 @@ "ownerDelegatedRequestsToTeam": true, "autoApprovedGroups": "group1", "autoApprovedServiceAccounts": "john.doe@example.com" + }, + { + "id": "example2.com", + "fullyQualifiedDomainName": "example2.com", + "ownerDelegatedRequestsToTeam": true, + "autoApprovedGroups": "group1", + "autoApprovedServiceAccounts": "john.doe@example.com" } ] } \ No newline at end of file diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index a885f6d..d768383 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -4,8 +4,10 @@ class AuthorizeRequest def call context.request.fqdns.each do |fqdn| - domain = Domain.where(fqdn: fqdn).first! - raise AuthError unless domain.users_array.include?(context.identity.subject) || + domain = Domain.where(fqdn: fqdn).first + raise AuthError.new("Common or alt name not recognized") unless domain + raise AuthError.new("No subject or group authorization") unless + domain.users_array.include?(context.identity.subject) || (domain.group_delegation? && (domain.groups_array & context.identity.groups).any?) end nil diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb index f9ea517..9ecbfbb 100644 --- a/app/interactors/refresh_domain.rb +++ b/app/interactors/refresh_domain.rb @@ -2,10 +2,10 @@ class RefreshDomain include Interactor def call - domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.fqdn) - domain_record = Domain.first_or_create(fqdn: context.request.fqdn) + domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.common_name) + domain_record = Domain.find_or_create_by!(fqdn: context.request.common_name) if !domain_info - domain_record.delete + domain_record.destroy! return end diff --git a/test/interactors/refresh_domain_test.rb b/test/interactors/refresh_domain_test.rb new file mode 100644 index 0000000..98d1507 --- /dev/null +++ b/test/interactors/refresh_domain_test.rb @@ -0,0 +1,41 @@ +require "test_helper" + +class RefreshDomainTest < ActiveSupport::TestCase + def setup + @domain = domains(:owner_match) + @identity = Identity.new(subject: @domain.users_array.first) + @cr = CertIssueRequest.new(common_name: @domain.fqdn) + @interactor = RefreshDomain + end + + test ".call updates db record with service response when 200" do + rslt = @interactor.call(identity: @identity, request: @cr) + assert rslt.success? + reloaded = Domain.where(fqdn: @domain.fqdn).first! + assert_not_equal @domain.users, reloaded.users + assert_not_equal @domain.groups, reloaded.groups + assert_not_equal @domain.group_delegation, reloaded.group_delegation + end + + test ".call deletes db record when service 404" do + @domain = domains(:no_match) # this fixture should have no match + @cr = CertIssueRequest.new(common_name: @domain.fqdn) + rslt = @interactor.call(identity: @identity, request: @cr) + assert rslt.success? + reloaded = Domain.where(fqdn: @domain.fqdn).first + assert_nil reloaded + end + + test ".call leaves db record as-is when service has error" do + mock = Services::DomainOwnershipService.new + err = ->(_){ raise Faraday::TimeoutError.new } + mock.stub(:get_domain_info, err) do + Services::DomainOwnershipService.stub :new, mock do + rslt = @interactor.call(identity: @identity, request: @cr) + assert rslt.success? + reloaded = Domain.where(fqdn: @domain.fqdn).first! + assert_equal @domain.users, reloaded.users + end + end + end +end diff --git a/test/lib/services/domain_ownership_service_test.rb b/test/lib/services/domain_ownership_service_test.rb index e87481b..36d9e33 100644 --- a/test/lib/services/domain_ownership_service_test.rb +++ b/test/lib/services/domain_ownership_service_test.rb @@ -2,11 +2,11 @@ class DomainOwnershipServiceTest < ActiveSupport::TestCase setup do - @subject = Services::DomainOwnershipService.new + @service = Services::DomainOwnershipService.new end test "#get_domain_info fetches from configured api server" do - domain_info = @subject.get_domain_info(domains(:owner_match).fqdn) + domain_info = @service.get_domain_info(domains(:owner_match).fqdn) assert_not_nil domain_info assert_equal "group1", domain_info.groups assert_equal "john.doe@example.com", domain_info.users @@ -15,7 +15,7 @@ class DomainOwnershipServiceTest < ActiveSupport::TestCase end test "#get_domain_info returns nil for unmatched fqdn" do - domain_info = @subject.get_domain_info(domains(:group_match).fqdn) + domain_info = @service.get_domain_info(domains(:no_match).fqdn) assert_nil domain_info end end From b957e44a2c847d5eaa56d1e7bc2ce5c8311f6a3c Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Wed, 11 Sep 2024 21:40:40 -0400 Subject: [PATCH 10/21] fix lint --- test/interactors/refresh_domain_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/interactors/refresh_domain_test.rb b/test/interactors/refresh_domain_test.rb index 98d1507..51605b0 100644 --- a/test/interactors/refresh_domain_test.rb +++ b/test/interactors/refresh_domain_test.rb @@ -28,8 +28,8 @@ def setup test ".call leaves db record as-is when service has error" do mock = Services::DomainOwnershipService.new - err = ->(_){ raise Faraday::TimeoutError.new } - mock.stub(:get_domain_info, err) do + err = ->(_) { raise Faraday::TimeoutError.new } + mock.stub(:get_domain_info, err) do Services::DomainOwnershipService.stub :new, mock do rslt = @interactor.call(identity: @identity, request: @cr) assert rslt.success? From a5b205f658b6c65976530e644da0d718171cdb98 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Wed, 11 Sep 2024 21:47:13 -0400 Subject: [PATCH 11/21] seeds adjust --- db/seeds.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/seeds.rb b/db/seeds.rb index 4c0f4e6..fa0adc1 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -10,5 +10,5 @@ # this seed is for development only if Rails.env.development? - Domain.first_or_create!(fqdn: "example.com", users: "john.doe@example.com") + Domain.find_or_create_by!(fqdn: "example.com", users: "john.doe@example.com") end From 53094c3266368ce921798c3f16ae01b58f186c85 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Thu, 12 Sep 2024 10:31:12 -0400 Subject: [PATCH 12/21] A bit of refactoring domain_ownership_service to abstraction --- app/lib/services/app_registry_service.rb | 53 +++++++++++++++++++ app/lib/services/auth_service.rb | 8 ++- app/lib/services/certificate_service.rb | 4 +- app/lib/services/domain_ownership_service.rb | 47 ++-------------- app/lib/services/vault_service.rb | 4 +- ...e_test.rb => app_registry_service_test.rb} | 4 +- 6 files changed, 68 insertions(+), 52 deletions(-) create mode 100644 app/lib/services/app_registry_service.rb rename test/lib/services/{domain_ownership_service_test.rb => app_registry_service_test.rb} (83%) diff --git a/app/lib/services/app_registry_service.rb b/app/lib/services/app_registry_service.rb new file mode 100644 index 0000000..2cd8671 --- /dev/null +++ b/app/lib/services/app_registry_service.rb @@ -0,0 +1,53 @@ +module Services + class AppRegistryService + attr_reader :client + + def initialize + @client = Faraday.new(ssl: ssl_opts, url: Rails.configuration.astral[:app_registry_addr]) do |faraday| + faraday.request :authorization, "Bearer", -> { Rails.configuration.astral[:app_registry_token] } + faraday.request :retry, retry_opts + faraday.response :json + faraday.response :raise_error, include_request: true + end + end + + def get_domain_info(fqdn) + rslt = client.get("/api/v1beta1/domain-names/#{fqdn}").body + convert(rslt) + rescue Faraday::ResourceNotFound => e + nil + end + + private + + def convert(domain_info) + if !domain_info || domain_info["isDeleted"] + return nil + end + + OpenStruct.new( + fqdn: domain_info["fullyQualifiedDomainName"], + group_delegation: domain_info["ownerDelegatedRequestsToTeam"], + groups: domain_info["autoApprovedGroups"], + users: domain_info["autoApprovedServiceAccounts"] + ) + end + + def ssl_opts + { + ca_file: Rails.configuration.astral[:app_registry_ca_file], + client_cert: Rails.configuration.astral[:app_registry_client_cert], + client_key: Rails.configuration.astral[:app_registry_client_key] + } + end + + def retry_opts + { + max: 3, + interval: 0.05, + interval_randomness: 0.5, + backoff_factor: 2 + } + end + end +end diff --git a/app/lib/services/auth_service.rb b/app/lib/services/auth_service.rb index c0b723f..dc5a983 100644 --- a/app/lib/services/auth_service.rb +++ b/app/lib/services/auth_service.rb @@ -1,7 +1,9 @@ module Services class AuthService + attr_reader :impl + def initialize - @domain_ownership_service = DomainOwnershipService.new + # TODO this should select an external service impl when needed end def authenticate!(token) @@ -11,10 +13,6 @@ def authenticate!(token) identity end - def authorize!(identity, cert_issue_req) - @domain_ownership_service.authorize!(identity, cert_issue_req) - end - private def decode(token) diff --git a/app/lib/services/certificate_service.rb b/app/lib/services/certificate_service.rb index 3b63dfa..785e6a6 100644 --- a/app/lib/services/certificate_service.rb +++ b/app/lib/services/certificate_service.rb @@ -1,12 +1,14 @@ module Services class CertificateService + attr_reader :impl + def initialize # TODO this should select an implementation service based on config @impl = VaultService.new end def issue_cert(cert_issue_request) - @impl.issue_cert(cert_issue_request) + impl.issue_cert(cert_issue_request) end end end diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index 80db2e4..cc494ca 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -1,53 +1,14 @@ module Services class DomainOwnershipService - attr_reader :client + attr_reader :impl def initialize - @client = Faraday.new(ssl: ssl_opts, url: Rails.configuration.astral[:app_registry_addr]) do |faraday| - faraday.request :authorization, "Bearer", -> { Rails.configuration.astral[:app_registry_token] } - faraday.request :retry, retry_opts - faraday.response :json - faraday.response :raise_error, include_request: true - end + # TODO this should select an implementation service based on config + @impl = AppRegistryService.new end def get_domain_info(fqdn) - rslt = client.get("/api/v1beta1/domain-names/#{fqdn}").body - convert(rslt) - rescue Faraday::ResourceNotFound => e - nil - end - - private - - def convert(domain_info) - if !domain_info || domain_info["isDeleted"] - return nil - end - - OpenStruct.new( - fqdn: domain_info["fullyQualifiedDomainName"], - group_delegation: domain_info["ownerDelegatedRequestsToTeam"], - groups: domain_info["autoApprovedGroups"], - users: domain_info["autoApprovedServiceAccounts"] - ) - end - - def ssl_opts - { - ca_file: Rails.configuration.astral[:app_registry_ca_file], - client_cert: Rails.configuration.astral[:app_registry_client_cert], - client_key: Rails.configuration.astral[:app_registry_client_key] - } - end - - def retry_opts - { - max: 3, - interval: 0.05, - interval_randomness: 0.5, - backoff_factor: 2 - } + impl.get_domain_info(fqdn) end end end diff --git a/app/lib/services/vault_service.rb b/app/lib/services/vault_service.rb index 3e8b383..3f15518 100644 --- a/app/lib/services/vault_service.rb +++ b/app/lib/services/vault_service.rb @@ -1,5 +1,7 @@ module Services class VaultService + attr_reader :client + def initialize # TODO create a new token for use in the session @client = Vault::Client.new( @@ -11,7 +13,7 @@ def initialize def issue_cert(cert_issue_request) opts = cert_issue_request.attributes # Generate the TLS certificate using the intermediate CA - tls_cert = @client.logical.write(Rails.configuration.astral[:vault_cert_path], opts) + tls_cert = client.logical.write(Rails.configuration.astral[:vault_cert_path], opts) OpenStruct.new tls_cert.data end end diff --git a/test/lib/services/domain_ownership_service_test.rb b/test/lib/services/app_registry_service_test.rb similarity index 83% rename from test/lib/services/domain_ownership_service_test.rb rename to test/lib/services/app_registry_service_test.rb index 36d9e33..30931da 100644 --- a/test/lib/services/domain_ownership_service_test.rb +++ b/test/lib/services/app_registry_service_test.rb @@ -1,8 +1,8 @@ require "test_helper" -class DomainOwnershipServiceTest < ActiveSupport::TestCase +class AppRegistryServiceTest < ActiveSupport::TestCase setup do - @service = Services::DomainOwnershipService.new + @service = Services::AppRegistryService.new end test "#get_domain_info fetches from configured api server" do From f255a450ac0ca20364753f75f5e42a4137c664d7 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Thu, 12 Sep 2024 10:52:36 -0400 Subject: [PATCH 13/21] small test name normalization --- test/interactors/authenticate_identity_test.rb | 6 +++--- test/interactors/obtain_cert_test.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/interactors/authenticate_identity_test.rb b/test/interactors/authenticate_identity_test.rb index c9361c9..2c4e571 100644 --- a/test/interactors/authenticate_identity_test.rb +++ b/test/interactors/authenticate_identity_test.rb @@ -6,7 +6,7 @@ def setup @identity = Identity.new(subject: "test@example.com", groups: [ "admin_group" ]) end - test "successful call" do + test ".call success" do request = OpenStruct.new(headers: { "Authorization" => "Bearer valid_token" }) srv = Minitest::Mock.new srv.expect :authenticate!, @identity, [ "valid_token" ] @@ -17,13 +17,13 @@ def setup end end - test "unsuccessful call" do + test ".call failure" do request = OpenStruct.new(headers: { "Authorization" => "Bearer invalid_token" }) srv = Minitest::Mock.new srv.expect :authenticate!, nil, [ "invalid_token" ] Services::AuthService.stub :new, srv do context = @interactor.call(request: request) - assert_not context.success? + assert context.failure? assert_nil context.identity end end diff --git a/test/interactors/obtain_cert_test.rb b/test/interactors/obtain_cert_test.rb index 3211ae2..002a7fb 100644 --- a/test/interactors/obtain_cert_test.rb +++ b/test/interactors/obtain_cert_test.rb @@ -6,7 +6,7 @@ def setup @cert = OpenStruct.new(certificate: "certificate", ca_chain: "ca_chain") end - test "successful call" do + test ".call success" do request = CertIssueRequest.new srv = Minitest::Mock.new srv.expect :issue_cert, @cert, [ request ] @@ -17,7 +17,7 @@ def setup end end - test "unsuccessful call" do + test ".call failure" do request = CertIssueRequest.new srv = Minitest::Mock.new srv.expect :issue_cert, nil, [ request ] From a271f8a9359784439d12577fd46a28b152ee6725 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Thu, 12 Sep 2024 15:19:37 -0400 Subject: [PATCH 14/21] wip for audit logging --- app/interactors/audit_logging.rb | 16 ++++++++++++++++ app/interactors/authenticate_identity.rb | 1 + app/interactors/authorize_request.rb | 2 ++ app/interactors/issue_cert.rb | 2 +- app/interactors/obtain_cert.rb | 1 + app/interactors/refresh_domain.rb | 1 + app/lib/audit_log_formatter.rb | 13 +++++++++++++ app/lib/audit_logger.rb | 6 ++++++ config/astral.yml | 1 + 9 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 app/interactors/audit_logging.rb create mode 100644 app/lib/audit_log_formatter.rb create mode 100644 app/lib/audit_logger.rb diff --git a/app/interactors/audit_logging.rb b/app/interactors/audit_logging.rb new file mode 100644 index 0000000..3f7198b --- /dev/null +++ b/app/interactors/audit_logging.rb @@ -0,0 +1,16 @@ +module AuditLogging + extend ActiveSupport::Concern + + included do + around do |interactor| + logger = AuditLogger.new + logger.info("#{self.class.name} begin") + interactor.call + if context.failed? + logger.error("#{self.class.name} failed") + else + logger.info("#{self.class.name} succeeded") + end + end + end +end diff --git a/app/interactors/authenticate_identity.rb b/app/interactors/authenticate_identity.rb index 8626708..9e26e1a 100644 --- a/app/interactors/authenticate_identity.rb +++ b/app/interactors/authenticate_identity.rb @@ -1,5 +1,6 @@ class AuthenticateIdentity include Interactor + include AuditLogging include FailOnError before do diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index d768383..d89aeb0 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -1,7 +1,9 @@ class AuthorizeRequest include Interactor + include AuditLogging include FailOnError + def call context.request.fqdns.each do |fqdn| domain = Domain.where(fqdn: fqdn).first diff --git a/app/interactors/issue_cert.rb b/app/interactors/issue_cert.rb index 667043f..d2513f8 100644 --- a/app/interactors/issue_cert.rb +++ b/app/interactors/issue_cert.rb @@ -2,5 +2,5 @@ class IssueCert include Interactor::Organizer include FailOnError - organize RefreshDomain, AuthorizeRequest, ObtainCert, Log + organize RefreshDomain, AuthorizeRequest, ObtainCert end diff --git a/app/interactors/obtain_cert.rb b/app/interactors/obtain_cert.rb index 8cd9b87..2cda14e 100644 --- a/app/interactors/obtain_cert.rb +++ b/app/interactors/obtain_cert.rb @@ -1,5 +1,6 @@ class ObtainCert include Interactor + include AuditLogging include FailOnError def call diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb index 9ecbfbb..9befe82 100644 --- a/app/interactors/refresh_domain.rb +++ b/app/interactors/refresh_domain.rb @@ -1,5 +1,6 @@ class RefreshDomain include Interactor + include AuditLogging def call domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.common_name) diff --git a/app/lib/audit_log_formatter.rb b/app/lib/audit_log_formatter.rb new file mode 100644 index 0000000..7f88cd0 --- /dev/null +++ b/app/lib/audit_log_formatter.rb @@ -0,0 +1,13 @@ +class AuditLogFormatter < ActiveSupport::Logger::SimpleFormatter + def call(severity, timestamp, _progname, message) + # request_id is unique to the life of the api request + request_id = ENV["action_dispatch.request_id"] + json = { + type: severity, + time: timestamp, + request_id: request_id, + message: message + }.to_json + "#{json}\n" + end +end diff --git a/app/lib/audit_logger.rb b/app/lib/audit_logger.rb new file mode 100644 index 0000000..b048687 --- /dev/null +++ b/app/lib/audit_logger.rb @@ -0,0 +1,6 @@ +class AuditLogger < ActiveSupport::Logger + def initialize + super(Rails.configuration.astral[:audit_log_file]) + self.formatter = AuditLogFormatter.new + end +end diff --git a/config/astral.yml b/config/astral.yml index f4d61c6..76592b4 100644 --- a/config/astral.yml +++ b/config/astral.yml @@ -9,6 +9,7 @@ shared: app_registry_ca_file: <%= ENV["APP_REGISTRY_CA_FILE"] %> app_registry_client_cert: <%= ENV["APP_REGISTRY_CLIENT_CERT"] %> app_registry_client_key: <%= ENV["APP_REGISTRY_CLIENT_KEY"] %> + audit_log_file: <%= ENV["AUDIT_LOG_FILE"] || "#{Rails.root.join('log')}/astral-audit.log" %> test: cert_ttl: <%= 24.hours.in_seconds %> From f98a6e80e1b2e2b5c39872698b8d3c412cd20ebb Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 09:38:34 -0400 Subject: [PATCH 15/21] request_id logging; accept hash for log message --- app/controllers/application_controller.rb | 5 +++++ app/interactors/audit_logging.rb | 9 ++++++--- app/lib/audit_log_formatter.rb | 16 ++++++++++------ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 02bc7ff..1fb0a84 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,5 +1,6 @@ class ApplicationController < ActionController::API before_action :set_default_format + before_action :store_request_id rescue_from StandardError, with: :handle_standard_error rescue_from AuthError, with: :handle_auth_error rescue_from BadRequestError, with: :handle_bad_request_error @@ -22,6 +23,10 @@ def set_default_format request.format = :json end + def store_request_id + Thread.current[:request_id] = request.uuid + end + def handle_standard_error(exception) render json: { error: exception.message }, status: :internal_server_error end diff --git a/app/interactors/audit_logging.rb b/app/interactors/audit_logging.rb index 3f7198b..2ebf58f 100644 --- a/app/interactors/audit_logging.rb +++ b/app/interactors/audit_logging.rb @@ -4,13 +4,16 @@ module AuditLogging included do around do |interactor| logger = AuditLogger.new - logger.info("#{self.class.name} begin") + logger.info(message: "#{self.class.name} begin") interactor.call if context.failed? - logger.error("#{self.class.name} failed") + logger.error(message: "#{self.class.name} failed") else - logger.info("#{self.class.name} succeeded") + logger.info(message: "#{self.class.name} succeeded") end + rescue => e + logger.error(message: "#{self.class.name} failed") + raise e end end end diff --git a/app/lib/audit_log_formatter.rb b/app/lib/audit_log_formatter.rb index 7f88cd0..c756b56 100644 --- a/app/lib/audit_log_formatter.rb +++ b/app/lib/audit_log_formatter.rb @@ -1,13 +1,17 @@ class AuditLogFormatter < ActiveSupport::Logger::SimpleFormatter def call(severity, timestamp, _progname, message) # request_id is unique to the life of the api request - request_id = ENV["action_dispatch.request_id"] - json = { + request_id = Thread.current[:request_id] + json = { type: severity, time: timestamp, - request_id: request_id, - message: message - }.to_json - "#{json}\n" + request_id: request_id + } + if message.is_a? Hash + json = json.merge(message) + else + json[:message] = message + end + "#{json.to_json}\n" end end From 6b9c4be36aa4fd782d1fdc717be6e5e9d9a004b1 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 09:40:07 -0400 Subject: [PATCH 16/21] WIP for class method services --- app/interactors/authenticate_identity.rb | 2 +- app/interactors/obtain_cert.rb | 2 +- app/interactors/refresh_domain.rb | 2 +- app/lib/services/app_registry_service.rb | 4 ++++ app/lib/services/auth_service.rb | 4 ++++ app/lib/services/certificate_service.rb | 6 ++++++ app/lib/services/domain_ownership_service.rb | 6 ++++++ app/lib/services/vault_service.rb | 7 ++++++- 8 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/interactors/authenticate_identity.rb b/app/interactors/authenticate_identity.rb index 8626708..b7462e8 100644 --- a/app/interactors/authenticate_identity.rb +++ b/app/interactors/authenticate_identity.rb @@ -8,7 +8,7 @@ class AuthenticateIdentity end def call - if identity = Services::AuthService.new.authenticate!(context.token) + if identity = Services::AuthService.authenticate!(context.token) context.identity = identity else context.fail!(message: "Invalid token") diff --git a/app/interactors/obtain_cert.rb b/app/interactors/obtain_cert.rb index 8cd9b87..54c893a 100644 --- a/app/interactors/obtain_cert.rb +++ b/app/interactors/obtain_cert.rb @@ -3,7 +3,7 @@ class ObtainCert include FailOnError def call - if cert = Services::CertificateService.new.issue_cert(context.request) + if cert = Services::CertificateService.issue_cert(context.request) context.cert = cert else context.fail!(message: "Failed to issue certificate") diff --git a/app/interactors/refresh_domain.rb b/app/interactors/refresh_domain.rb index 9ecbfbb..bae405a 100644 --- a/app/interactors/refresh_domain.rb +++ b/app/interactors/refresh_domain.rb @@ -2,7 +2,7 @@ class RefreshDomain include Interactor def call - domain_info = Services::DomainOwnershipService.new.get_domain_info(context.request.common_name) + domain_info = Services::DomainOwnershipService.get_domain_info(context.request.common_name) domain_record = Domain.find_or_create_by!(fqdn: context.request.common_name) if !domain_info domain_record.destroy! diff --git a/app/lib/services/app_registry_service.rb b/app/lib/services/app_registry_service.rb index 2cd8671..b28fb2c 100644 --- a/app/lib/services/app_registry_service.rb +++ b/app/lib/services/app_registry_service.rb @@ -17,6 +17,10 @@ def get_domain_info(fqdn) rescue Faraday::ResourceNotFound => e nil end + + def self.get_domain_info(fqdn) + new.get_domain_info(fqdn) + end private diff --git a/app/lib/services/auth_service.rb b/app/lib/services/auth_service.rb index dc5a983..1fb8f3d 100644 --- a/app/lib/services/auth_service.rb +++ b/app/lib/services/auth_service.rb @@ -13,6 +13,10 @@ def authenticate!(token) identity end + def self.authenticate!(token) + new.authenticate!(token) + end + private def decode(token) diff --git a/app/lib/services/certificate_service.rb b/app/lib/services/certificate_service.rb index 785e6a6..0f53d2a 100644 --- a/app/lib/services/certificate_service.rb +++ b/app/lib/services/certificate_service.rb @@ -2,6 +2,12 @@ module Services class CertificateService attr_reader :impl + def self.issue_cert(cert_issue_request) + new.issue_cert(cert_issue_request) + end + + private + def initialize # TODO this should select an implementation service based on config @impl = VaultService.new diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index cc494ca..689a48f 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -2,6 +2,12 @@ module Services class DomainOwnershipService attr_reader :impl + def self.get_domain_info(fqdn) + new.get_domain_info(fqdn) + end + + private + def initialize # TODO this should select an implementation service based on config @impl = AppRegistryService.new diff --git a/app/lib/services/vault_service.rb b/app/lib/services/vault_service.rb index 3f15518..c855f6d 100644 --- a/app/lib/services/vault_service.rb +++ b/app/lib/services/vault_service.rb @@ -1,7 +1,12 @@ module Services class VaultService + def self.issue_cert(cert_issue_request) + new.issue_cert(cert_issue_request) + end + + private attr_reader :client - + def initialize # TODO create a new token for use in the session @client = Vault::Client.new( From 9b8ee056f869795ac422bde5dab6cd274ae524a2 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 10:18:11 -0400 Subject: [PATCH 17/21] fix up tests for class-method services --- app/lib/services/app_registry_service.rb | 86 +++++++++---------- app/lib/services/auth_service.rb | 40 ++++----- app/lib/services/certificate_service.rb | 22 ++--- app/lib/services/domain_ownership_service.rb | 22 ++--- app/lib/services/vault_service.rb | 35 ++++---- .../interactors/authenticate_identity_test.rb | 12 +-- test/interactors/obtain_cert_test.rb | 12 +-- test/interactors/refresh_domain_test.rb | 6 +- .../lib/services/app_registry_service_test.rb | 2 +- 9 files changed, 107 insertions(+), 130 deletions(-) diff --git a/app/lib/services/app_registry_service.rb b/app/lib/services/app_registry_service.rb index b28fb2c..3424fc8 100644 --- a/app/lib/services/app_registry_service.rb +++ b/app/lib/services/app_registry_service.rb @@ -1,57 +1,53 @@ module Services class AppRegistryService - attr_reader :client - - def initialize - @client = Faraday.new(ssl: ssl_opts, url: Rails.configuration.astral[:app_registry_addr]) do |faraday| - faraday.request :authorization, "Bearer", -> { Rails.configuration.astral[:app_registry_token] } - faraday.request :retry, retry_opts - faraday.response :json - faraday.response :raise_error, include_request: true + class << self + def get_domain_info(fqdn) + rslt = client.get("/api/v1beta1/domain-names/#{fqdn}").body + convert(rslt) + rescue Faraday::ResourceNotFound => e + nil end - end - - def get_domain_info(fqdn) - rslt = client.get("/api/v1beta1/domain-names/#{fqdn}").body - convert(rslt) - rescue Faraday::ResourceNotFound => e - nil - end - - def self.get_domain_info(fqdn) - new.get_domain_info(fqdn) - end - private + private - def convert(domain_info) - if !domain_info || domain_info["isDeleted"] - return nil + def client + Faraday.new(ssl: ssl_opts, url: Rails.configuration.astral[:app_registry_addr]) do |faraday| + faraday.request :authorization, "Bearer", -> { Rails.configuration.astral[:app_registry_token] } + faraday.request :retry, retry_opts + faraday.response :json + faraday.response :raise_error, include_request: true + end end - OpenStruct.new( - fqdn: domain_info["fullyQualifiedDomainName"], - group_delegation: domain_info["ownerDelegatedRequestsToTeam"], - groups: domain_info["autoApprovedGroups"], - users: domain_info["autoApprovedServiceAccounts"] - ) - end + def convert(domain_info) + if !domain_info || domain_info["isDeleted"] + return nil + end - def ssl_opts - { - ca_file: Rails.configuration.astral[:app_registry_ca_file], - client_cert: Rails.configuration.astral[:app_registry_client_cert], - client_key: Rails.configuration.astral[:app_registry_client_key] - } - end + OpenStruct.new( + fqdn: domain_info["fullyQualifiedDomainName"], + group_delegation: domain_info["ownerDelegatedRequestsToTeam"], + groups: domain_info["autoApprovedGroups"], + users: domain_info["autoApprovedServiceAccounts"] + ) + end - def retry_opts - { - max: 3, - interval: 0.05, - interval_randomness: 0.5, - backoff_factor: 2 - } + def ssl_opts + { + ca_file: Rails.configuration.astral[:app_registry_ca_file], + client_cert: Rails.configuration.astral[:app_registry_client_cert], + client_key: Rails.configuration.astral[:app_registry_client_key] + } + end + + def retry_opts + { + max: 3, + interval: 0.05, + interval_randomness: 0.5, + backoff_factor: 2 + } + end end end end diff --git a/app/lib/services/auth_service.rb b/app/lib/services/auth_service.rb index 1fb8f3d..037b2f2 100644 --- a/app/lib/services/auth_service.rb +++ b/app/lib/services/auth_service.rb @@ -1,31 +1,23 @@ module Services class AuthService - attr_reader :impl + class << self + def authenticate!(token) + identity = decode(token) + raise AuthError unless identity + # TODO verify identity with authority? + identity + end - def initialize - # TODO this should select an external service impl when needed - end - - def authenticate!(token) - identity = decode(token) - raise AuthError unless identity - # TODO verify identity with authority? - identity - end - - def self.authenticate!(token) - new.authenticate!(token) - end - - private + private - def decode(token) - # Decode a JWT access token using the configured base. - body = JWT.decode(token, Rails.configuration.astral[:jwt_signing_key])[0] - Identity.new(body) - rescue => e - Rails.logger.warn "Unable to decode token: #{e}" - nil + def decode(token) + # Decode a JWT access token using the configured base. + body = JWT.decode(token, Rails.configuration.astral[:jwt_signing_key])[0] + Identity.new(body) + rescue => e + Rails.logger.warn "Unable to decode token: #{e}" + nil + end end end end diff --git a/app/lib/services/certificate_service.rb b/app/lib/services/certificate_service.rb index 0f53d2a..15141c8 100644 --- a/app/lib/services/certificate_service.rb +++ b/app/lib/services/certificate_service.rb @@ -1,20 +1,16 @@ module Services class CertificateService - attr_reader :impl + class << self + def issue_cert(cert_issue_request) + impl.issue_cert(cert_issue_request) + end - def self.issue_cert(cert_issue_request) - new.issue_cert(cert_issue_request) - end - - private - - def initialize - # TODO this should select an implementation service based on config - @impl = VaultService.new - end + private - def issue_cert(cert_issue_request) - impl.issue_cert(cert_issue_request) + def impl + # TODO this should select an implementation service based on config + VaultService + end end end end diff --git a/app/lib/services/domain_ownership_service.rb b/app/lib/services/domain_ownership_service.rb index 689a48f..6ecfd14 100644 --- a/app/lib/services/domain_ownership_service.rb +++ b/app/lib/services/domain_ownership_service.rb @@ -1,20 +1,16 @@ module Services class DomainOwnershipService - attr_reader :impl + class << self + def get_domain_info(fqdn) + impl.get_domain_info(fqdn) + end - def self.get_domain_info(fqdn) - new.get_domain_info(fqdn) - end - - private - - def initialize - # TODO this should select an implementation service based on config - @impl = AppRegistryService.new - end + private - def get_domain_info(fqdn) - impl.get_domain_info(fqdn) + def impl + # TODO this should select an implementation service based on config + AppRegistryService + end end end end diff --git a/app/lib/services/vault_service.rb b/app/lib/services/vault_service.rb index c855f6d..889e5c3 100644 --- a/app/lib/services/vault_service.rb +++ b/app/lib/services/vault_service.rb @@ -1,25 +1,22 @@ module Services class VaultService - def self.issue_cert(cert_issue_request) - new.issue_cert(cert_issue_request) - end - - private - attr_reader :client - - def initialize - # TODO create a new token for use in the session - @client = Vault::Client.new( - address: Rails.configuration.astral[:vault_addr], - token: Rails.configuration.astral[:vault_token] - ) - end + class << self + def issue_cert(cert_issue_request) + opts = cert_issue_request.attributes + # Generate the TLS certificate using the intermediate CA + tls_cert = client.logical.write(Rails.configuration.astral[:vault_cert_path], opts) + OpenStruct.new tls_cert.data + end + + private - def issue_cert(cert_issue_request) - opts = cert_issue_request.attributes - # Generate the TLS certificate using the intermediate CA - tls_cert = client.logical.write(Rails.configuration.astral[:vault_cert_path], opts) - OpenStruct.new tls_cert.data + def client + # TODO create a new token for use in the session + Vault::Client.new( + address: Rails.configuration.astral[:vault_addr], + token: Rails.configuration.astral[:vault_token] + ) + end end end end diff --git a/test/interactors/authenticate_identity_test.rb b/test/interactors/authenticate_identity_test.rb index 2c4e571..eb0c8d9 100644 --- a/test/interactors/authenticate_identity_test.rb +++ b/test/interactors/authenticate_identity_test.rb @@ -8,9 +8,9 @@ def setup test ".call success" do request = OpenStruct.new(headers: { "Authorization" => "Bearer valid_token" }) - srv = Minitest::Mock.new - srv.expect :authenticate!, @identity, [ "valid_token" ] - Services::AuthService.stub :new, srv do + mock = Minitest::Mock.new + mock.expect :call, @identity, [ "valid_token" ] + Services::AuthService.stub :authenticate!, mock do context = @interactor.call(request: request) assert context.success? assert_equal @identity, context.identity @@ -19,9 +19,9 @@ def setup test ".call failure" do request = OpenStruct.new(headers: { "Authorization" => "Bearer invalid_token" }) - srv = Minitest::Mock.new - srv.expect :authenticate!, nil, [ "invalid_token" ] - Services::AuthService.stub :new, srv do + mock = Minitest::Mock.new + mock.expect :call, nil, [ "invalid_token" ] + Services::AuthService.stub :authenticate!, mock do context = @interactor.call(request: request) assert context.failure? assert_nil context.identity diff --git a/test/interactors/obtain_cert_test.rb b/test/interactors/obtain_cert_test.rb index 002a7fb..efd79b7 100644 --- a/test/interactors/obtain_cert_test.rb +++ b/test/interactors/obtain_cert_test.rb @@ -8,9 +8,9 @@ def setup test ".call success" do request = CertIssueRequest.new - srv = Minitest::Mock.new - srv.expect :issue_cert, @cert, [ request ] - Services::CertificateService.stub :new, srv do + mock = Minitest::Mock.new + mock.expect :call, @cert, [ request ] + Services::CertificateService.stub :issue_cert, mock do context = @interactor.call(request: request) assert context.success? assert_equal @cert, context.cert @@ -19,9 +19,9 @@ def setup test ".call failure" do request = CertIssueRequest.new - srv = Minitest::Mock.new - srv.expect :issue_cert, nil, [ request ] - Services::CertificateService.stub :new, srv do + mock = Minitest::Mock.new + mock.expect :call, nil, [ request ] + Services::CertificateService.stub :issue_cert, mock do context = @interactor.call(request: request) assert context.failure? assert_nil context.cert diff --git a/test/interactors/refresh_domain_test.rb b/test/interactors/refresh_domain_test.rb index 51605b0..07ba149 100644 --- a/test/interactors/refresh_domain_test.rb +++ b/test/interactors/refresh_domain_test.rb @@ -27,10 +27,10 @@ def setup end test ".call leaves db record as-is when service has error" do - mock = Services::DomainOwnershipService.new + mock = Minitest::Mock.new err = ->(_) { raise Faraday::TimeoutError.new } - mock.stub(:get_domain_info, err) do - Services::DomainOwnershipService.stub :new, mock do + mock.expect(:call, err) do + Services::DomainOwnershipService.stub :get_domain_info, mock do rslt = @interactor.call(identity: @identity, request: @cr) assert rslt.success? reloaded = Domain.where(fqdn: @domain.fqdn).first! diff --git a/test/lib/services/app_registry_service_test.rb b/test/lib/services/app_registry_service_test.rb index 30931da..3cda7b8 100644 --- a/test/lib/services/app_registry_service_test.rb +++ b/test/lib/services/app_registry_service_test.rb @@ -2,7 +2,7 @@ class AppRegistryServiceTest < ActiveSupport::TestCase setup do - @service = Services::AppRegistryService.new + @service = Services::AppRegistryService end test "#get_domain_info fetches from configured api server" do From d8801671e2196815a70bbca6a5955ba9d81ebfd2 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 11:24:49 -0400 Subject: [PATCH 18/21] Simplify audit logging --- app/interactors/audit_logging.rb | 25 +++++++++++++++++-------- app/lib/audit_log_formatter.rb | 2 +- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/app/interactors/audit_logging.rb b/app/interactors/audit_logging.rb index 2ebf58f..8fb8dd7 100644 --- a/app/interactors/audit_logging.rb +++ b/app/interactors/audit_logging.rb @@ -3,17 +3,26 @@ module AuditLogging included do around do |interactor| - logger = AuditLogger.new - logger.info(message: "#{self.class.name} begin") interactor.call - if context.failed? - logger.error(message: "#{self.class.name} failed") - else - logger.info(message: "#{self.class.name} succeeded") - end + log rescue => e - logger.error(message: "#{self.class.name} failed") + log raise e end end + + private + + def log + msg = context.success? ? "succeeded" : "failed" + level = context.success? ? :info : :error + payload = { + action: "#{self.class.name}", + success: context.success?, + action_error: context.error&.message, + subject: context.identity&.subject, + cert_common_name: context.request&.try(:common_name) + } + AuditLogger.new.send(level, payload) + end end diff --git a/app/lib/audit_log_formatter.rb b/app/lib/audit_log_formatter.rb index c756b56..39672fe 100644 --- a/app/lib/audit_log_formatter.rb +++ b/app/lib/audit_log_formatter.rb @@ -2,7 +2,7 @@ class AuditLogFormatter < ActiveSupport::Logger::SimpleFormatter def call(severity, timestamp, _progname, message) # request_id is unique to the life of the api request request_id = Thread.current[:request_id] - json = { + json = { type: severity, time: timestamp, request_id: request_id From 4bd9c4a7a35d7da7ad4d3e902edd82654f44fc18 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 14:28:05 -0400 Subject: [PATCH 19/21] Unit test --- app/lib/audit_log_formatter.rb | 2 +- test/lib/audit_log_formatter_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 test/lib/audit_log_formatter_test.rb diff --git a/app/lib/audit_log_formatter.rb b/app/lib/audit_log_formatter.rb index 39672fe..0304ea4 100644 --- a/app/lib/audit_log_formatter.rb +++ b/app/lib/audit_log_formatter.rb @@ -4,7 +4,7 @@ def call(severity, timestamp, _progname, message) request_id = Thread.current[:request_id] json = { type: severity, - time: timestamp, + time: "#{timestamp}", request_id: request_id } if message.is_a? Hash diff --git a/test/lib/audit_log_formatter_test.rb b/test/lib/audit_log_formatter_test.rb new file mode 100644 index 0000000..79e6333 --- /dev/null +++ b/test/lib/audit_log_formatter_test.rb @@ -0,0 +1,24 @@ +require "test_helper" + +class AuditLogFormatterTest < ActiveSupport::TestCase + test "#call formats logformatter inputs as json" do + t = Time.now + result = AuditLogFormatter.new.call("info", t, nil, "some message") + assert_equal %Q({"type":"info","time":"#{t}","request_id":null,"message":"some message"}\n), result + end + + test "#call accepts and merges a Hash type for the message" do + t = Time.now + result = AuditLogFormatter.new.call("info", t, nil, { key: "some message", key2: "another" }) + assert_equal %Q({"type":"info","time":"#{t}","request_id":null,"key":"some message","key2":"another"}\n), result + end + + test "#call can render a thread local request_id" do + t = Time.now + req_id = SecureRandom.hex + Thread.stub :current, { request_id: req_id } do + result = AuditLogFormatter.new.call("info", t, nil, { key: "some message" }) + assert_equal %Q({"type":"info","time":"#{t}","request_id":"#{req_id}","key":"some message"}\n), result + end + end +end From f15a1a9d5022015078649b24dd46a14771879a3e Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 15:14:45 -0400 Subject: [PATCH 20/21] Add interactor concern test --- test/interactors/audit_logging_test.rb | 43 ++++++++++++++++++++++++++ test/lib/audit_log_formatter_test.rb | 4 +++ 2 files changed, 47 insertions(+) create mode 100644 test/interactors/audit_logging_test.rb diff --git a/test/interactors/audit_logging_test.rb b/test/interactors/audit_logging_test.rb new file mode 100644 index 0000000..ad864a9 --- /dev/null +++ b/test/interactors/audit_logging_test.rb @@ -0,0 +1,43 @@ +require "test_helper" + +class AuditLoggingTest < ActiveSupport::TestCase + def setup + @domain = domains(:owner_match) + @identity = Identity.new(subject: @domain.users_array.first) + @cr = CertIssueRequest.new(common_name: @domain.fqdn) + @log = Tempfile.new("log-test") + Rails.configuration.astral[:audit_log_file] = @log.path + end + + def teardown + @log.close + @log.unlink + end + + test ".call will be logged as success" do + Object.const_set("SuccessAction", Class.new do + include Interactor + include AuditLogging + + def call + end + end) + rslt = SuccessAction.call(identity: @identity, request: @cr) + assert rslt.success? + assert_match %Q("action":"SuccessAction","result":"success","error":null,"subject":"john.doe@example.com","cert_common_name":"example.com"), @log.readlines.last + end + + test ".call will be logged as failure" do + Object.const_set("FailAction", Class.new do + include Interactor + include AuditLogging + + def call + context.fail! + end + end) + rslt = FailAction.call(identity: @identity, request: @cr) + assert_not rslt.success? + assert_match %Q("action":"FailAction","result":"failure","error":null,"subject":"john.doe@example.com","cert_common_name":"example.com"), @log.readlines.last + end +end diff --git a/test/lib/audit_log_formatter_test.rb b/test/lib/audit_log_formatter_test.rb index 79e6333..e23e185 100644 --- a/test/lib/audit_log_formatter_test.rb +++ b/test/lib/audit_log_formatter_test.rb @@ -1,6 +1,10 @@ require "test_helper" class AuditLogFormatterTest < ActiveSupport::TestCase + setup do + Thread.current[:request_id] = nil + end + test "#call formats logformatter inputs as json" do t = Time.now result = AuditLogFormatter.new.call("info", t, nil, "some message") From a0e76983968b6aedb5e8d0de8323537b9372cb10 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 13 Sep 2024 18:36:27 -0400 Subject: [PATCH 21/21] move logging include after FailOnError --- app/interactors/authenticate_identity.rb | 2 +- app/interactors/authorize_request.rb | 2 +- app/interactors/obtain_cert.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/interactors/authenticate_identity.rb b/app/interactors/authenticate_identity.rb index 2abf8cf..c274d64 100644 --- a/app/interactors/authenticate_identity.rb +++ b/app/interactors/authenticate_identity.rb @@ -1,7 +1,7 @@ class AuthenticateIdentity include Interactor - include AuditLogging include FailOnError + include AuditLogging before do token = context.request.headers["Authorization"] diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index d89aeb0..85ae6bb 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -1,7 +1,7 @@ class AuthorizeRequest include Interactor - include AuditLogging include FailOnError + include AuditLogging def call diff --git a/app/interactors/obtain_cert.rb b/app/interactors/obtain_cert.rb index a635e9a..bb07a2d 100644 --- a/app/interactors/obtain_cert.rb +++ b/app/interactors/obtain_cert.rb @@ -1,7 +1,7 @@ class ObtainCert include Interactor - include AuditLogging include FailOnError + include AuditLogging def call if cert = Services::CertificateService.issue_cert(context.request)