From e7615c71fcf3d9ba11ba7602fddea1d27aac2584 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 6 Sep 2024 15:34:42 -0400 Subject: [PATCH 01/25] Add better auth error propogation from interactor to controller; added interactor unit test --- app/controllers/certificates_controller.rb | 4 +-- app/interactors/authorize_request.rb | 3 ++ test/interactors/authorize_request_test.rb | 33 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 test/interactors/authorize_request_test.rb diff --git a/app/controllers/certificates_controller.rb b/app/controllers/certificates_controller.rb index 5355f80..f4f204a 100644 --- a/app/controllers/certificates_controller.rb +++ b/app/controllers/certificates_controller.rb @@ -6,9 +6,9 @@ def create if !req.valid? raise BadRequestError.new req.errors.full_messages end - result = IssueCert.call(request: req, identity: @identity) + result = IssueCert.call(request: req, identity: identity) if result.failure? - raise StandardError.new result.message + raise (result.error || StandardError.new(result.message)) end @cert = result.cert end diff --git a/app/interactors/authorize_request.rb b/app/interactors/authorize_request.rb index 8f033c5..1f55c7e 100644 --- a/app/interactors/authorize_request.rb +++ b/app/interactors/authorize_request.rb @@ -3,5 +3,8 @@ class AuthorizeRequest def call Services::DomainOwnershipService.new.authorize!(context.identity, context.request) + rescue AuthError => e + context.error = e + context.fail! end end diff --git a/test/interactors/authorize_request_test.rb b/test/interactors/authorize_request_test.rb new file mode 100644 index 0000000..30cd3e6 --- /dev/null +++ b/test/interactors/authorize_request_test.rb @@ -0,0 +1,33 @@ +require "test_helper" + +class AuthorizeRequestTest < ActiveSupport::TestCase + def setup + @domain = domains(:group_match) + @identity = Identity.new(subject: @domain.owner) + @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 + 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 + end +end From 06520b5e140b30f25400fbae9fb42d414f61ce50 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Wed, 23 Oct 2024 17:54:36 -0700 Subject: [PATCH 02/25] init --- app/interactors/obtain_cert.rb | 1 + app/lib/clients/vault.rb | 1 + app/lib/clients/vault/entity.rb | 14 +++++++++++--- app/lib/services/user_config.rb | 16 ++++++++++++++++ app/models/identity.rb | 1 + config/astral.yml | 1 + 6 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 app/lib/services/user_config.rb diff --git a/app/interactors/obtain_cert.rb b/app/interactors/obtain_cert.rb index e0c8383..345815a 100644 --- a/app/interactors/obtain_cert.rb +++ b/app/interactors/obtain_cert.rb @@ -5,6 +5,7 @@ def call else context.fail!(message: "Failed to issue certificate") end + Services::UserConfig.config(context.identity, context.cert) ensure audit_log end diff --git a/app/lib/clients/vault.rb b/app/lib/clients/vault.rb index 5a68e85..d679348 100644 --- a/app/lib/clients/vault.rb +++ b/app/lib/clients/vault.rb @@ -6,6 +6,7 @@ class Vault extend Clients::Vault::Entity extend Clients::Vault::EntityAlias extend Clients::Vault::Oidc + extend Clients::Vault::UserConfig class_attribute :token diff --git a/app/lib/clients/vault/entity.rb b/app/lib/clients/vault/entity.rb index 8e2e89c..01013ce 100644 --- a/app/lib/clients/vault/entity.rb +++ b/app/lib/clients/vault/entity.rb @@ -1,14 +1,22 @@ module Clients class Vault module Entity - def put_entity(name, policies) + def put_entity(name, policies, metadata={}) client.logical.write("identity/entity", name: name, - policies: policies) + policies: policies, + metadata: metadata) end def read_entity(name) - client.logical.read("identity/entity/name/#{name}") + entity = client.logical.read("identity/entity/name/#{name}") + if entity.nil? + {data: + {policies: [], + metadata: {}}} + else + entity + end end def delete_entity(name) diff --git a/app/lib/services/user_config.rb b/app/lib/services/user_config.rb new file mode 100644 index 0000000..2f086ed --- /dev/null +++ b/app/lib/services/user_config.rb @@ -0,0 +1,16 @@ +module Services + class UserConfig + class << self + def config(identity, cert) + impl.config_user(identity, cert) + end + + private + + def impl + # TODO this should select an implementation service based on config + Clients::Vault + end + end + end +end diff --git a/app/models/identity.rb b/app/models/identity.rb index 20c7491..ffdd080 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -9,5 +9,6 @@ class Identity attribute :groups, array: :string, default: [] alias_attribute :sub, :subject + alias_attribute :email, :subject alias_attribute :roles, :groups end diff --git a/config/astral.yml b/config/astral.yml index adc2a9c..eb4b1be 100644 --- a/config/astral.yml +++ b/config/astral.yml @@ -55,6 +55,7 @@ test: cert_ttl: <%= 24.hours.in_seconds %> development: + cert_ttl: <%= 24.hours.in_seconds %> production: vault_create_root: false From db95882564526eb0e3a3fa335aafc54898b9a0f4 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Thu, 24 Oct 2024 12:46:05 -0700 Subject: [PATCH 03/25] reorg --- app/lib/clients/vault/certificate.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index f3bc31a..ec61bbb 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -17,8 +17,6 @@ def configure_pki configure_ca end - private - def intermediate_ca_mount "pki_astral" end @@ -27,6 +25,9 @@ def cert_path "#{intermediate_ca_mount}/issue/astral" end + private + + def create_root? create_root_config = Config[:vault_create_root] !!ActiveModel::Type::Boolean.new.cast(create_root_config) From abd9816ebc28e21a448c975518264c0733dd945b Mon Sep 17 00:00:00 2001 From: George Jahad Date: Thu, 24 Oct 2024 15:49:25 -0700 Subject: [PATCH 04/25] user_config --- app/lib/clients/vault/user_config.rb | 48 ++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 app/lib/clients/vault/user_config.rb diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb new file mode 100644 index 0000000..8ecb5ab --- /dev/null +++ b/app/lib/clients/vault/user_config.rb @@ -0,0 +1,48 @@ +require "set" +module Clients + class Vault + module UserConfig + def config_user(identity, cert) + sub = identity.sub + email = identity.email + entity = read_entity(sub)[:data] + policy = create_cert_policy(cert) + client.sys.put_policy(policy_name(sub), policy) + client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) + policies = entity[:policies].append(policy_name(sub)).append(GENERIC_CERT_POLICY_NAME).to_set.to_a + put_entity(sub, policies, entity[:metadata]) + put_entity_alias(sub, email , "oidc") + end + + private + + def create_cert_policy(cert) + "path \"#{intermediate_ca_mount}/cert/#{serial_number(cert)}\" { + capabilities = [\"read\"] + }" + end + + def serial_number(cert) + cert[:serial_number] + end + + def policy_name(sub) + "astral-#{sub}-cert-policy" + end + + GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" + + def generic_cert_policy + policy = <<-EOH + path "#{cert_path}" { + capabilities = ["create", "update"] + } + + path "#{intermediate_ca_mount}/revoke" { + capabilities = ["update"] + } + EOH + end + end + end +end \ No newline at end of file From 8b045caf2cb92f6706184202ca1e4058610eca50 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Thu, 24 Oct 2024 17:36:06 -0700 Subject: [PATCH 05/25] removed data --- app/lib/clients/vault/entity.rb | 7 +++---- app/lib/clients/vault/user_config.rb | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/lib/clients/vault/entity.rb b/app/lib/clients/vault/entity.rb index 01013ce..2efab84 100644 --- a/app/lib/clients/vault/entity.rb +++ b/app/lib/clients/vault/entity.rb @@ -11,11 +11,10 @@ def put_entity(name, policies, metadata={}) def read_entity(name) entity = client.logical.read("identity/entity/name/#{name}") if entity.nil? - {data: - {policies: [], - metadata: {}}} + {policies: [], + metadata: {}} else - entity + entity.data end end diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb index 8ecb5ab..5a0e70f 100644 --- a/app/lib/clients/vault/user_config.rb +++ b/app/lib/clients/vault/user_config.rb @@ -5,7 +5,7 @@ module UserConfig def config_user(identity, cert) sub = identity.sub email = identity.email - entity = read_entity(sub)[:data] + entity = read_entity(sub) policy = create_cert_policy(cert) client.sys.put_policy(policy_name(sub), policy) client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) From 6488ad96a3195f40d9682233c68ee91895bff198 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Thu, 24 Oct 2024 17:50:02 -0700 Subject: [PATCH 06/25] basic policy creation working --- app/lib/clients/vault/entity.rb | 8 +------- app/lib/clients/vault/user_config.rb | 11 +++++++++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/lib/clients/vault/entity.rb b/app/lib/clients/vault/entity.rb index 2efab84..f7b6171 100644 --- a/app/lib/clients/vault/entity.rb +++ b/app/lib/clients/vault/entity.rb @@ -9,13 +9,7 @@ def put_entity(name, policies, metadata={}) end def read_entity(name) - entity = client.logical.read("identity/entity/name/#{name}") - if entity.nil? - {policies: [], - metadata: {}} - else - entity.data - end + client.logical.read("identity/entity/name/#{name}") end def delete_entity(name) diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb index 5a0e70f..f5bd3e1 100644 --- a/app/lib/clients/vault/user_config.rb +++ b/app/lib/clients/vault/user_config.rb @@ -6,11 +6,18 @@ def config_user(identity, cert) sub = identity.sub email = identity.email entity = read_entity(sub) + if entity.nil? + policies = [] + metadata = nil + else + policies = entity.data[:policies] + metadata = entity.data[:metadata] + end policy = create_cert_policy(cert) client.sys.put_policy(policy_name(sub), policy) client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) - policies = entity[:policies].append(policy_name(sub)).append(GENERIC_CERT_POLICY_NAME).to_set.to_a - put_entity(sub, policies, entity[:metadata]) + policies.append(policy_name(sub)).append(GENERIC_CERT_POLICY_NAME).to_set.to_a + put_entity(sub, policies, metadata) put_entity_alias(sub, email , "oidc") end From d4269212d743e76eca1c68752bea5044ecca9a04 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Thu, 24 Oct 2024 20:08:04 -0700 Subject: [PATCH 07/25] removed extraneous policies --- app/lib/clients/vault/user_config.rb | 32 ++++++++-------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb index f5bd3e1..ba9e792 100644 --- a/app/lib/clients/vault/user_config.rb +++ b/app/lib/clients/vault/user_config.rb @@ -5,36 +5,22 @@ module UserConfig def config_user(identity, cert) sub = identity.sub email = identity.email - entity = read_entity(sub) - if entity.nil? - policies = [] - metadata = nil - else - policies = entity.data[:policies] - metadata = entity.data[:metadata] - end - policy = create_cert_policy(cert) - client.sys.put_policy(policy_name(sub), policy) + policies, metadata = get_entity_data(sub) client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) - policies.append(policy_name(sub)).append(GENERIC_CERT_POLICY_NAME).to_set.to_a + policies.append(GENERIC_CERT_POLICY_NAME).to_set.to_a put_entity(sub, policies, metadata) put_entity_alias(sub, email , "oidc") end private - def create_cert_policy(cert) - "path \"#{intermediate_ca_mount}/cert/#{serial_number(cert)}\" { - capabilities = [\"read\"] - }" - end - - def serial_number(cert) - cert[:serial_number] - end - - def policy_name(sub) - "astral-#{sub}-cert-policy" + def get_entity_data(sub) + entity = read_entity(sub) + if entity.nil? + [[], nil] + else + [entity.data[:policies], entity.data[:metadata]] + end end GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" From 20a65045d8eb807a1f1e274d844c253478a07a50 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Fri, 25 Oct 2024 16:31:27 -0700 Subject: [PATCH 08/25] changed intial email --- config/astral.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/astral.yml b/config/astral.yml index eb4b1be..a52ebef 100644 --- a/config/astral.yml +++ b/config/astral.yml @@ -49,7 +49,7 @@ shared: initial_user_name: test initial_user_password: test - initial_user_email: test@example.com + initial_user_email: john.doe@example.com test: cert_ttl: <%= 24.hours.in_seconds %> From 5eb39f3db1312c7f0009510cdab1a2d9374d420c Mon Sep 17 00:00:00 2001 From: George Jahad Date: Mon, 28 Oct 2024 11:41:00 -0700 Subject: [PATCH 09/25] moved policy creation --- app/lib/clients/vault/certificate.rb | 26 +++++++++++++++++++++++--- app/lib/clients/vault/user_config.rb | 17 +---------------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index ec61bbb..c3e51f7 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -15,8 +15,13 @@ def configure_pki enable_ca sign_cert configure_ca + create_generic_cert_policy end + GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" + + private + def intermediate_ca_mount "pki_astral" end @@ -25,9 +30,6 @@ def cert_path "#{intermediate_ca_mount}/issue/astral" end - private - - def create_root? create_root_config = Config[:vault_create_root] !!ActiveModel::Type::Boolean.new.cast(create_root_config) @@ -118,6 +120,24 @@ def configure_ca ocsp_servers: "{{cluster_path}}/ocsp", enable_templating: true) end + + def create_generic_cert_policy + client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) + end + + def generic_cert_policy + policy = <<-EOH + + path "#{cert_path}" { + capabilities = ["create", "update"] + } + + path "#{intermediate_ca_mount}/revoke-with-key" { + capabilities = ["update"] + } + EOH + end + end end end diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb index ba9e792..4316083 100644 --- a/app/lib/clients/vault/user_config.rb +++ b/app/lib/clients/vault/user_config.rb @@ -6,8 +6,7 @@ def config_user(identity, cert) sub = identity.sub email = identity.email policies, metadata = get_entity_data(sub) - client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) - policies.append(GENERIC_CERT_POLICY_NAME).to_set.to_a + policies.append(Certificate::GENERIC_CERT_POLICY_NAME).to_set.to_a put_entity(sub, policies, metadata) put_entity_alias(sub, email , "oidc") end @@ -22,20 +21,6 @@ def get_entity_data(sub) [entity.data[:policies], entity.data[:metadata]] end end - - GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" - - def generic_cert_policy - policy = <<-EOH - path "#{cert_path}" { - capabilities = ["create", "update"] - } - - path "#{intermediate_ca_mount}/revoke" { - capabilities = ["update"] - } - EOH - end end end end \ No newline at end of file From f356d3e7f40dc0eaaf1ecdaf1a4dbf76a2d1bb39 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Mon, 28 Oct 2024 13:43:30 -0700 Subject: [PATCH 10/25] fixed test --- test/interactors/obtain_cert_test.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/interactors/obtain_cert_test.rb b/test/interactors/obtain_cert_test.rb index 7327b67..39c7c56 100644 --- a/test/interactors/obtain_cert_test.rb +++ b/test/interactors/obtain_cert_test.rb @@ -8,10 +8,12 @@ def setup test ".call success" do request = Requests::CertIssueRequest.new + identity = Identity.new + identity.sub = "testUser" mock = Minitest::Mock.new mock.expect :call, @cert, [ request ] Services::Certificate.stub :issue_cert, mock do - context = @interactor.call(request: request) + context = @interactor.call(identity: identity, request: request) assert context.success? assert_equal @cert, context.cert end From 72a8bdc4cd9c981af5c7fdadca3df92fa24ce5f4 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Mon, 28 Oct 2024 14:05:37 -0700 Subject: [PATCH 11/25] added test --- app/interactors/obtain_cert.rb | 2 +- app/lib/clients/vault/user_config.rb | 2 +- app/lib/services/user_config.rb | 4 ++-- app/lib/utils/oidc_provider.rb | 1 + test/lib/clients/vault_test.rb | 11 +++++++++++ 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/interactors/obtain_cert.rb b/app/interactors/obtain_cert.rb index 345815a..4a01811 100644 --- a/app/interactors/obtain_cert.rb +++ b/app/interactors/obtain_cert.rb @@ -5,7 +5,7 @@ def call else context.fail!(message: "Failed to issue certificate") end - Services::UserConfig.config(context.identity, context.cert) + Services::UserConfig.config(context.identity) ensure audit_log end diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb index 4316083..ee5c43d 100644 --- a/app/lib/clients/vault/user_config.rb +++ b/app/lib/clients/vault/user_config.rb @@ -2,7 +2,7 @@ module Clients class Vault module UserConfig - def config_user(identity, cert) + def config_user(identity) sub = identity.sub email = identity.email policies, metadata = get_entity_data(sub) diff --git a/app/lib/services/user_config.rb b/app/lib/services/user_config.rb index 2f086ed..430b93b 100644 --- a/app/lib/services/user_config.rb +++ b/app/lib/services/user_config.rb @@ -1,8 +1,8 @@ module Services class UserConfig class << self - def config(identity, cert) - impl.config_user(identity, cert) + def config(identity) + impl.config_user(identity) end private diff --git a/app/lib/utils/oidc_provider.rb b/app/lib/utils/oidc_provider.rb index 9c8c539..0a88ddf 100644 --- a/app/lib/utils/oidc_provider.rb +++ b/app/lib/utils/oidc_provider.rb @@ -19,6 +19,7 @@ def configure def get_client_info app = vault_client.logical.read(WEBAPP_NAME) + raise "oidc provider not configured." if app.nil? @client_id = app.data[:client_id] @client_secret = app.data[:client_secret] [ @client_id, @client_secret ] diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index 51ca0a8..ae6f534 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -18,6 +18,8 @@ class VaultTest < ActiveSupport::TestCase @policies = SecureRandom.hex(4) @entity_name = SecureRandom.hex(4) @alias_name = SecureRandom.hex(4) + @identity = Identity.new + @identity.sub = SecureRandom.hex(4) end teardown do @@ -110,6 +112,15 @@ class VaultTest < ActiveSupport::TestCase assert_match /no such alias/, err.message end + test "#config_user creates valid entity" do + @client.config_user(@identity) + entity = @client.read_entity(@identity.sub) + assert entity.data[:policies].any? { |p| + p == @client::Certificate::GENERIC_CERT_POLICY_NAME } + assert entity.data[:aliases].any? { |a| + a[:mount_type] == "oidc" && a[:name] == @identity.sub } + end + private def vault_client From 562a9073d68abccd35ebea5f0a64f91af968316f Mon Sep 17 00:00:00 2001 From: George Jahad Date: Mon, 28 Oct 2024 14:06:26 -0700 Subject: [PATCH 12/25] rubocop --- app/lib/clients/vault/certificate.rb | 1 - app/lib/clients/vault/entity.rb | 2 +- app/lib/clients/vault/user_config.rb | 8 ++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index c3e51f7..0d9ac45 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -137,7 +137,6 @@ def generic_cert_policy } EOH end - end end end diff --git a/app/lib/clients/vault/entity.rb b/app/lib/clients/vault/entity.rb index f7b6171..25b4faa 100644 --- a/app/lib/clients/vault/entity.rb +++ b/app/lib/clients/vault/entity.rb @@ -1,7 +1,7 @@ module Clients class Vault module Entity - def put_entity(name, policies, metadata={}) + def put_entity(name, policies, metadata = {}) client.logical.write("identity/entity", name: name, policies: policies, diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb index ee5c43d..45e835d 100644 --- a/app/lib/clients/vault/user_config.rb +++ b/app/lib/clients/vault/user_config.rb @@ -8,7 +8,7 @@ def config_user(identity) policies, metadata = get_entity_data(sub) policies.append(Certificate::GENERIC_CERT_POLICY_NAME).to_set.to_a put_entity(sub, policies, metadata) - put_entity_alias(sub, email , "oidc") + put_entity_alias(sub, email, "oidc") end private @@ -16,11 +16,11 @@ def config_user(identity) def get_entity_data(sub) entity = read_entity(sub) if entity.nil? - [[], nil] + [ [], nil ] else - [entity.data[:policies], entity.data[:metadata]] + [ entity.data[:policies], entity.data[:metadata] ] end end end end -end \ No newline at end of file +end From 4c4ab8b39b7c9152f8011c16e06e3512fec7a8f9 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Mon, 28 Oct 2024 14:55:25 -0700 Subject: [PATCH 13/25] removed user_config file --- app/interactors/obtain_cert.rb | 3 +-- app/lib/clients/vault.rb | 1 - app/lib/clients/vault/certificate.rb | 21 ++++++++++++++++++++- app/lib/clients/vault/user_config.rb | 26 -------------------------- app/lib/services/certificate.rb | 4 ++-- app/lib/services/user_config.rb | 16 ---------------- test/interactors/obtain_cert_test.rb | 9 +++++---- 7 files changed, 28 insertions(+), 52 deletions(-) delete mode 100644 app/lib/clients/vault/user_config.rb delete mode 100644 app/lib/services/user_config.rb diff --git a/app/interactors/obtain_cert.rb b/app/interactors/obtain_cert.rb index 4a01811..8592e86 100644 --- a/app/interactors/obtain_cert.rb +++ b/app/interactors/obtain_cert.rb @@ -1,11 +1,10 @@ class ObtainCert < ApplicationInteractor def call - if cert = Services::Certificate.issue_cert(context.request) + if cert = Services::Certificate.issue_cert(context.identity, context.request) context.cert = cert else context.fail!(message: "Failed to issue certificate") end - Services::UserConfig.config(context.identity) ensure audit_log end diff --git a/app/lib/clients/vault.rb b/app/lib/clients/vault.rb index d679348..5a68e85 100644 --- a/app/lib/clients/vault.rb +++ b/app/lib/clients/vault.rb @@ -6,7 +6,6 @@ class Vault extend Clients::Vault::Entity extend Clients::Vault::EntityAlias extend Clients::Vault::Oidc - extend Clients::Vault::UserConfig class_attribute :token diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index 0d9ac45..d215c72 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -1,10 +1,11 @@ module Clients class Vault module Certificate - def issue_cert(cert_issue_request) + def issue_cert(identity, cert_issue_request) opts = cert_issue_request.attributes # Generate the TLS certificate using the intermediate CA tls_cert = client.logical.write(cert_path, opts) + config_user(identity) OpenStruct.new tls_cert.data end @@ -18,6 +19,15 @@ def configure_pki create_generic_cert_policy end + def config_user(identity) + sub = identity.sub + email = identity.email + policies, metadata = get_entity_data(sub) + policies.append(Certificate::GENERIC_CERT_POLICY_NAME).to_set.to_a + put_entity(sub, policies, metadata) + put_entity_alias(sub, email, "oidc") + end + GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" private @@ -121,6 +131,15 @@ def configure_ca enable_templating: true) end + def get_entity_data(sub) + entity = read_entity(sub) + if entity.nil? + [ [], nil ] + else + [ entity.data[:policies], entity.data[:metadata] ] + end + end + def create_generic_cert_policy client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) end diff --git a/app/lib/clients/vault/user_config.rb b/app/lib/clients/vault/user_config.rb deleted file mode 100644 index 45e835d..0000000 --- a/app/lib/clients/vault/user_config.rb +++ /dev/null @@ -1,26 +0,0 @@ -require "set" -module Clients - class Vault - module UserConfig - def config_user(identity) - sub = identity.sub - email = identity.email - policies, metadata = get_entity_data(sub) - policies.append(Certificate::GENERIC_CERT_POLICY_NAME).to_set.to_a - put_entity(sub, policies, metadata) - put_entity_alias(sub, email, "oidc") - end - - private - - def get_entity_data(sub) - entity = read_entity(sub) - if entity.nil? - [ [], nil ] - else - [ entity.data[:policies], entity.data[:metadata] ] - end - end - end - end -end diff --git a/app/lib/services/certificate.rb b/app/lib/services/certificate.rb index d0f3b3a..929944d 100644 --- a/app/lib/services/certificate.rb +++ b/app/lib/services/certificate.rb @@ -1,8 +1,8 @@ module Services class Certificate class << self - def issue_cert(cert_issue_request) - impl.issue_cert(cert_issue_request) + def issue_cert(identity, cert_issue_request) + impl.issue_cert(identity, cert_issue_request) end private diff --git a/app/lib/services/user_config.rb b/app/lib/services/user_config.rb deleted file mode 100644 index 430b93b..0000000 --- a/app/lib/services/user_config.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Services - class UserConfig - class << self - def config(identity) - impl.config_user(identity) - end - - private - - def impl - # TODO this should select an implementation service based on config - Clients::Vault - end - end - end -end diff --git a/test/interactors/obtain_cert_test.rb b/test/interactors/obtain_cert_test.rb index 39c7c56..09bd2ee 100644 --- a/test/interactors/obtain_cert_test.rb +++ b/test/interactors/obtain_cert_test.rb @@ -9,9 +9,8 @@ def setup test ".call success" do request = Requests::CertIssueRequest.new identity = Identity.new - identity.sub = "testUser" mock = Minitest::Mock.new - mock.expect :call, @cert, [ request ] + mock.expect :call, @cert, [ identity, request ] Services::Certificate.stub :issue_cert, mock do context = @interactor.call(identity: identity, request: request) assert context.success? @@ -21,10 +20,12 @@ def setup test ".call failure" do request = Requests::CertIssueRequest.new + identity = Identity.new + identity.sub = "testUser" mock = Minitest::Mock.new - mock.expect :call, nil, [ request ] + mock.expect :call, nil, [ identity, request ] Services::Certificate.stub :issue_cert, mock do - context = @interactor.call(request: request) + context = @interactor.call({ identity: identity, request: request }) assert context.failure? assert_nil context.cert end From f139d7df598118edd24670b7e4f5b162ebf10e06 Mon Sep 17 00:00:00 2001 From: George Jahad Date: Mon, 28 Oct 2024 14:57:29 -0700 Subject: [PATCH 14/25] cleanup --- test/interactors/obtain_cert_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/interactors/obtain_cert_test.rb b/test/interactors/obtain_cert_test.rb index 09bd2ee..32086aa 100644 --- a/test/interactors/obtain_cert_test.rb +++ b/test/interactors/obtain_cert_test.rb @@ -21,7 +21,6 @@ def setup test ".call failure" do request = Requests::CertIssueRequest.new identity = Identity.new - identity.sub = "testUser" mock = Minitest::Mock.new mock.expect :call, nil, [ identity, request ] Services::Certificate.stub :issue_cert, mock do From cf543b7405ee49d14fdb4000ed3e859dd3e609b7 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 09:21:13 -0400 Subject: [PATCH 15/25] add identity to interactor calls --- app/interactors/read_secret.rb | 2 +- app/interactors/write_secret.rb | 2 +- app/lib/clients/vault/key_value.rb | 6 +++--- app/lib/services/key_value.rb | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/interactors/read_secret.rb b/app/interactors/read_secret.rb index b549276..f3fe466 100644 --- a/app/interactors/read_secret.rb +++ b/app/interactors/read_secret.rb @@ -1,6 +1,6 @@ class ReadSecret < ApplicationInteractor def call - if secret = Services::KeyValue.read(context.request.path) + if secret = Services::KeyValue.read(context.identity, context.request.path) context.secret = secret else context.fail!(message: "Failed to read secret: #{context.request.path}") diff --git a/app/interactors/write_secret.rb b/app/interactors/write_secret.rb index 7d3560a..ec4ea4c 100644 --- a/app/interactors/write_secret.rb +++ b/app/interactors/write_secret.rb @@ -1,6 +1,6 @@ class WriteSecret < ApplicationInteractor def call - if secret = Services::KeyValue.write(context.request.path, context.request.data) + if secret = Services::KeyValue.write(context.identity, context.request.path, context.request.data) context.secret = secret else context.fail!(message: "Failed to store secret") diff --git a/app/lib/clients/vault/key_value.rb b/app/lib/clients/vault/key_value.rb index dee6718..7ae6f7c 100644 --- a/app/lib/clients/vault/key_value.rb +++ b/app/lib/clients/vault/key_value.rb @@ -1,15 +1,15 @@ module Clients class Vault module KeyValue - def kv_read(path) + def kv_read(identity, path) client.kv(kv_mount).read(path) end - def kv_write(path, data) + def kv_write(identity, path, data) client.logical.write("#{kv_mount}/data/#{path}", data: data) end - def kv_delete(path) + def kv_delete(identity, path) client.logical.delete("#{kv_mount}/data/#{path}") end diff --git a/app/lib/services/key_value.rb b/app/lib/services/key_value.rb index d2bca9a..6ddf20a 100644 --- a/app/lib/services/key_value.rb +++ b/app/lib/services/key_value.rb @@ -1,15 +1,15 @@ module Services class KeyValue class << self - def read(path) - impl.kv_read(path) + def read(identity, path) + impl.kv_read(identity, path) end - def write(path, data) + def write(identity, path, data) impl.kv_write(path, data) end - def delete(path) + def delete(identity, path) impl.kv_delete(path) end From cb9c8c4e9a2b9959f4f2ac0ad7806032a39787fb Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 14:32:44 -0400 Subject: [PATCH 16/25] move policy/entity methods to matching modules --- app/interactors/delete_secret.rb | 2 +- app/lib/clients/vault/certificate.rb | 27 ++++++--------------------- app/lib/clients/vault/entity.rb | 9 +++++++++ app/lib/clients/vault/key_value.rb | 22 ++++++++++++++++++++++ app/lib/clients/vault/policy.rb | 9 +++++++++ app/lib/services/key_value.rb | 4 ++-- 6 files changed, 49 insertions(+), 24 deletions(-) diff --git a/app/interactors/delete_secret.rb b/app/interactors/delete_secret.rb index e864b29..7d346bc 100644 --- a/app/interactors/delete_secret.rb +++ b/app/interactors/delete_secret.rb @@ -1,6 +1,6 @@ class DeleteSecret < ApplicationInteractor def call - Services::KeyValue.delete(context.request.path) + Services::KeyValue.delete(context.identity, context.request.path) ensure audit_log end diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index d215c72..7f5a892 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -1,11 +1,16 @@ module Clients class Vault module Certificate + extend Clients::Vault::Entity + extend Clients::Vault::Policy + + GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" + def issue_cert(identity, cert_issue_request) opts = cert_issue_request.attributes # Generate the TLS certificate using the intermediate CA tls_cert = client.logical.write(cert_path, opts) - config_user(identity) + assign_policy(identity, GENERIC_CERT_POLICY_NAME) OpenStruct.new tls_cert.data end @@ -19,17 +24,6 @@ def configure_pki create_generic_cert_policy end - def config_user(identity) - sub = identity.sub - email = identity.email - policies, metadata = get_entity_data(sub) - policies.append(Certificate::GENERIC_CERT_POLICY_NAME).to_set.to_a - put_entity(sub, policies, metadata) - put_entity_alias(sub, email, "oidc") - end - - GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" - private def intermediate_ca_mount @@ -131,15 +125,6 @@ def configure_ca enable_templating: true) end - def get_entity_data(sub) - entity = read_entity(sub) - if entity.nil? - [ [], nil ] - else - [ entity.data[:policies], entity.data[:metadata] ] - end - end - def create_generic_cert_policy client.sys.put_policy(GENERIC_CERT_POLICY_NAME, generic_cert_policy) end diff --git a/app/lib/clients/vault/entity.rb b/app/lib/clients/vault/entity.rb index 25b4faa..6216d31 100644 --- a/app/lib/clients/vault/entity.rb +++ b/app/lib/clients/vault/entity.rb @@ -15,6 +15,15 @@ def read_entity(name) def delete_entity(name) client.logical.delete("identity/entity/name/#{name}") end + + def get_entity_data(sub) + entity = read_entity(sub) + if entity.nil? + [ [], nil ] + else + [ entity.data[:policies], entity.data[:metadata] ] + end + end end end end diff --git a/app/lib/clients/vault/key_value.rb b/app/lib/clients/vault/key_value.rb index 7ae6f7c..bce265e 100644 --- a/app/lib/clients/vault/key_value.rb +++ b/app/lib/clients/vault/key_value.rb @@ -1,11 +1,16 @@ module Clients class Vault module KeyValue + extend Clients::Vault::Entity + extend Clients::Vault::Policy + def kv_read(identity, path) client.kv(kv_mount).read(path) end def kv_write(identity, path, data) + create_kv_policy(path) + assign_policy(identity, policy_path(path)) client.logical.write("#{kv_mount}/data/#{path}", data: data) end @@ -28,6 +33,23 @@ def kv_mount def kv_engine_type "kv-v2" end + + + def create_kv_policy(path) + client.sys.put_policy(policy_path(path), kv_policy(path)) + end + + def policy_path(path) + "kv_policy/#{path}" + end + + def kv_policy(path) + policy = <<-EOH + path "#{path}" { + capabilities = ["create", "read", "update", "delete"] + } + EOH + end end end end diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index d2b12d0..ba49847 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -7,6 +7,15 @@ def rotate_token Clients::Vault.token = token end + def assign_policy(identity, policy_path) + sub = identity.sub + email = identity.email + policies, metadata = get_entity_data(sub) + policies.append(policy_path).to_set.to_a + put_entity(sub, policies, metadata) + put_entity_alias(sub, email, "oidc") + end + private def create_astral_policy diff --git a/app/lib/services/key_value.rb b/app/lib/services/key_value.rb index 6ddf20a..9bf2ad4 100644 --- a/app/lib/services/key_value.rb +++ b/app/lib/services/key_value.rb @@ -6,11 +6,11 @@ def read(identity, path) end def write(identity, path, data) - impl.kv_write(path, data) + impl.kv_write(identity, path, data) end def delete(identity, path) - impl.kv_delete(path) + impl.kv_delete(identity, path) end private From 907374e95b435b3687628f967cc96aa02c76578b Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 15:03:01 -0400 Subject: [PATCH 17/25] fix client unit test --- test/lib/clients/vault_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index ae6f534..cf4e0c2 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -68,7 +68,7 @@ class VaultTest < ActiveSupport::TestCase # now has a new token assert_not_equal vault_token, @client.token # ensure we can write with the new token - assert_instance_of Vault::Secret, @client.kv_write("testing/secret", { password: "sicr3t" }) + assert_instance_of Vault::Secret, @client.kv_write(@identity, "testing/secret", { password: "sicr3t" }) end test "#entity" do From c8a4301541542f2e40623f1b5d684731f3ac8804 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 15:19:20 -0400 Subject: [PATCH 18/25] Simplify policy/entity usage --- app/lib/clients/vault/certificate.rb | 3 +-- app/lib/clients/vault/key_value.rb | 3 +-- app/lib/clients/vault/policy.rb | 2 ++ 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index b701239..11b0b54 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -1,8 +1,7 @@ module Clients class Vault module Certificate - extend Clients::Vault::Entity - extend Clients::Vault::Policy + extend Policy GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" diff --git a/app/lib/clients/vault/key_value.rb b/app/lib/clients/vault/key_value.rb index bce265e..ac02d10 100644 --- a/app/lib/clients/vault/key_value.rb +++ b/app/lib/clients/vault/key_value.rb @@ -1,8 +1,7 @@ module Clients class Vault module KeyValue - extend Clients::Vault::Entity - extend Clients::Vault::Policy + extend Policy def kv_read(identity, path) client.kv(kv_mount).read(path) diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index ba49847..ce0268f 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -1,6 +1,8 @@ module Clients class Vault module Policy + extend Entity + def rotate_token create_astral_policy token = create_astral_token From 75051c7a870efc27761360ab3c032efd85df224a Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 16:44:54 -0400 Subject: [PATCH 19/25] remove config_user from Cert (using assign_policy now); fix test --- app/lib/clients/vault/certificate.rb | 11 ----------- test/lib/clients/vault_test.rb | 10 +++++----- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/app/lib/clients/vault/certificate.rb b/app/lib/clients/vault/certificate.rb index 11b0b54..5f82ac5 100644 --- a/app/lib/clients/vault/certificate.rb +++ b/app/lib/clients/vault/certificate.rb @@ -23,17 +23,6 @@ def configure_pki create_generic_cert_policy end - def config_user(identity) - sub = identity.sub - email = identity.email - policies, metadata = get_entity_data(sub) - policies.append(Certificate::GENERIC_CERT_POLICY_NAME).to_set.to_a - put_entity(sub, policies, metadata) - put_entity_alias(sub, email, "oidc") - end - - GENERIC_CERT_POLICY_NAME = "astral-generic-cert-policy" - private def intermediate_ca_mount diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index cf4e0c2..58395ff 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -71,7 +71,7 @@ class VaultTest < ActiveSupport::TestCase assert_instance_of Vault::Secret, @client.kv_write(@identity, "testing/secret", { password: "sicr3t" }) end - test "#entity" do + test "entity methods" do entity = @client.read_entity(@entity_name) assert_nil entity @@ -84,7 +84,7 @@ class VaultTest < ActiveSupport::TestCase assert_nil entity end - test "#entity_alias" do + test "entity_alias methods" do # confirm no entity yet err = assert_raises RuntimeError do @client.read_entity_alias(@entity_name, @alias_name) @@ -112,11 +112,11 @@ class VaultTest < ActiveSupport::TestCase assert_match /no such alias/, err.message end - test "#config_user creates valid entity" do - @client.config_user(@identity) + test ".assign_policy creates valid entity" do + @client.assign_policy(@identity, "test_path") entity = @client.read_entity(@identity.sub) assert entity.data[:policies].any? { |p| - p == @client::Certificate::GENERIC_CERT_POLICY_NAME } + p == "test_path" } assert entity.data[:aliases].any? { |a| a[:mount_type] == "oidc" && a[:name] == @identity.sub } end From 6189a637946462bfcb539ed7bd92137a81d9e0fd Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 22:25:10 -0400 Subject: [PATCH 20/25] Add test of kv_write; verify policy creation --- test/lib/clients/vault_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index 58395ff..b0459af 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -84,6 +84,22 @@ class VaultTest < ActiveSupport::TestCase assert_nil entity end + test "kv methods" do + # check kv_write + path = "test/path/#{SecureRandom.hex}" + secret = @client.kv_write(@identity, path, { data: "data" }) + assert_kind_of Vault::Secret, secret + + # check kv_read + read_secret = @client.kv_read(@identity, path) + assert_kind_of Vault::Secret, read_secret + + # check policy is created + entity = @client.read_entity(@identity.sub) + assert_equal "kv_policy/#{path}", entity.data[:policies][0] + end + + test "entity_alias methods" do # confirm no entity yet err = assert_raises RuntimeError do From 952e342fbd4cd522a0f7b98df4c546a46338cee0 Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Fri, 1 Nov 2024 22:27:23 -0400 Subject: [PATCH 21/25] add kv_delete test --- test/lib/clients/vault_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index b0459af..cd23027 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -97,6 +97,12 @@ class VaultTest < ActiveSupport::TestCase # check policy is created entity = @client.read_entity(@identity.sub) assert_equal "kv_policy/#{path}", entity.data[:policies][0] + + # check kv_delete + del_secret = @client.kv_delete(@identity, path) + assert del_secret + read_secret = @client.kv_read(@identity, path) + assert_nil read_secret end From 95c5b7b820ab8151536a3b6b5eeff9bf7019602e Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Mon, 4 Nov 2024 10:30:59 -0500 Subject: [PATCH 22/25] PR changes --- app/lib/clients/vault/policy.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index ce0268f..c3dd0ea 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -9,11 +9,11 @@ def rotate_token Clients::Vault.token = token end - def assign_policy(identity, policy_path) + def assign_policy(identity, policy_name) sub = identity.sub email = identity.email policies, metadata = get_entity_data(sub) - policies.append(policy_path).to_set.to_a + policies.append(policy_name).uniq! put_entity(sub, policies, metadata) put_entity_alias(sub, email, "oidc") end From 2382e259c5028cc9cb88bd56f33ad36194b8e3cd Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Mon, 4 Nov 2024 11:19:52 -0500 Subject: [PATCH 23/25] Add kv-read policy verification --- app/lib/clients/vault/key_value.rb | 3 ++- app/lib/clients/vault/policy.rb | 9 +++++++++ test/lib/clients/vault_test.rb | 11 ++++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/lib/clients/vault/key_value.rb b/app/lib/clients/vault/key_value.rb index ac02d10..9f5b59e 100644 --- a/app/lib/clients/vault/key_value.rb +++ b/app/lib/clients/vault/key_value.rb @@ -4,6 +4,7 @@ module KeyValue extend Policy def kv_read(identity, path) + verify_policy(identity, policy_path(path)) client.kv(kv_mount).read(path) end @@ -14,6 +15,7 @@ def kv_write(identity, path, data) end def kv_delete(identity, path) + verify_policy(identity, policy_path(path)) client.logical.delete("#{kv_mount}/data/#{path}") end @@ -33,7 +35,6 @@ def kv_engine_type "kv-v2" end - def create_kv_policy(path) client.sys.put_policy(policy_path(path), kv_policy(path)) end diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index c3dd0ea..c14dbc4 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -18,6 +18,15 @@ def assign_policy(identity, policy_name) put_entity_alias(sub, email, "oidc") end + def verify_policy(identity, policy_name) + sub = identity.sub + email = identity.email + policies, _ = get_entity_data(sub) + unless policies.any? { |p| p == policy_name } + raise AuthError.new("Policy has not been granted to the identity") + end + end + private def create_astral_policy diff --git a/test/lib/clients/vault_test.rb b/test/lib/clients/vault_test.rb index cd23027..f34f5eb 100644 --- a/test/lib/clients/vault_test.rb +++ b/test/lib/clients/vault_test.rb @@ -98,6 +98,16 @@ class VaultTest < ActiveSupport::TestCase entity = @client.read_entity(@identity.sub) assert_equal "kv_policy/#{path}", entity.data[:policies][0] + # check kv_read denied to other identity + alt_identity = Identity.new + alt_identity.sub = SecureRandom.hex(4) + err = assert_raises { @client.kv_read(alt_identity, path) } + assert_kind_of AuthError, err + + # check kv_delete denied to other identity + err = assert_raises { @client.kv_delete(alt_identity, path) } + assert_kind_of AuthError, err + # check kv_delete del_secret = @client.kv_delete(@identity, path) assert del_secret @@ -105,7 +115,6 @@ class VaultTest < ActiveSupport::TestCase assert_nil read_secret end - test "entity_alias methods" do # confirm no entity yet err = assert_raises RuntimeError do From 6bf2070653ca20c6439c999264b221d7bf61093c Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Mon, 4 Nov 2024 11:45:03 -0500 Subject: [PATCH 24/25] unneeded email var --- app/lib/clients/vault/policy.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index c14dbc4..0ee93f0 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -20,7 +20,6 @@ def assign_policy(identity, policy_name) def verify_policy(identity, policy_name) sub = identity.sub - email = identity.email policies, _ = get_entity_data(sub) unless policies.any? { |p| p == policy_name } raise AuthError.new("Policy has not been granted to the identity") From 10fea6a5b5549590e09333675016760a9aa1eb0c Mon Sep 17 00:00:00 2001 From: Geoff Wilson Date: Tue, 5 Nov 2024 09:34:09 -0500 Subject: [PATCH 25/25] Add advisory lock to prevent policies race --- Gemfile | 3 +++ Gemfile.lock | 4 ++++ app/lib/clients/vault/policy.rb | 10 ++++++---- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 75562b2..6899b36 100644 --- a/Gemfile +++ b/Gemfile @@ -48,6 +48,9 @@ gem "jbuilder" gem "faraday" gem "faraday-retry" +# Use with_advisory_lock for multiprocess mutex +gem "with_advisory_lock" + 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 2f63d3c..6f4557d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -269,6 +269,9 @@ GEM websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) + with_advisory_lock (5.1.0) + activerecord (>= 6.1) + zeitwerk (>= 2.6) zeitwerk (2.7.1) PLATFORMS @@ -304,6 +307,7 @@ DEPENDENCIES sqlite3 (>= 1.4) tzinfo-data vault + with_advisory_lock BUNDLED WITH 2.5.11 diff --git a/app/lib/clients/vault/policy.rb b/app/lib/clients/vault/policy.rb index 0ee93f0..1e6eab3 100644 --- a/app/lib/clients/vault/policy.rb +++ b/app/lib/clients/vault/policy.rb @@ -12,10 +12,12 @@ def rotate_token def assign_policy(identity, policy_name) sub = identity.sub email = identity.email - policies, metadata = get_entity_data(sub) - policies.append(policy_name).uniq! - put_entity(sub, policies, metadata) - put_entity_alias(sub, email, "oidc") + Domain.with_advisory_lock(sub) do + policies, metadata = get_entity_data(sub) + policies.append(policy_name).uniq! + put_entity(sub, policies, metadata) + put_entity_alias(sub, email, "oidc") + end end def verify_policy(identity, policy_name)