Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Kv policy verification #72

Merged
merged 40 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
e7615c7
Add better auth error propogation from interactor to controller; added
suprjinx Sep 6, 2024
0c2c8c5
Merge branch 'main' of github.com:suprjinx/astral
suprjinx Sep 13, 2024
9793f3f
Merge branch 'main' of github.com:suprjinx/astral
suprjinx Sep 17, 2024
a2b6f6a
Merge branch 'main' of github.com:suprjinx/astral
suprjinx Oct 21, 2024
08fa788
Merge branch 'main' of github.com:g-research/astral
suprjinx Oct 22, 2024
88b65d4
Merge branch 'main' of github.com:G-Research/astral
suprjinx Oct 22, 2024
d87c082
Merge branch 'main' of github.com:G-Research/astral
suprjinx Oct 22, 2024
06520b5
init
Oct 24, 2024
db95882
reorg
Oct 24, 2024
abd9816
user_config
Oct 24, 2024
8b045ca
removed data
Oct 25, 2024
6488ad9
basic policy creation working
Oct 25, 2024
04c97b9
Merge branch 'main' of github.com:g-research/astral
suprjinx Oct 25, 2024
d426921
removed extraneous policies
Oct 25, 2024
20a6504
changed intial email
Oct 25, 2024
5eb39f3
moved policy creation
Oct 28, 2024
f356d3e
fixed test
Oct 28, 2024
72a8bdc
added test
Oct 28, 2024
562a907
rubocop
Oct 28, 2024
4c4ab8b
removed user_config file
Oct 28, 2024
f139d7d
cleanup
Oct 28, 2024
52d4dca
Merge branch 'main' of github.com:G-Research/astral
suprjinx Oct 31, 2024
cf543b7
add identity to interactor calls
suprjinx Nov 1, 2024
cb9c8c4
move policy/entity methods to matching modules
suprjinx Nov 1, 2024
aa81e34
Merge branch 'main' of github.com:G-Research/astral
suprjinx Nov 1, 2024
bbbba20
Merge branch 'main' into kv_identity
suprjinx Nov 1, 2024
907374e
fix client unit test
suprjinx Nov 1, 2024
c8a4301
Simplify policy/entity usage
suprjinx Nov 1, 2024
75051c7
remove config_user from Cert (using assign_policy now); fix test
suprjinx Nov 1, 2024
6189a63
Add test of kv_write; verify policy creation
suprjinx Nov 2, 2024
952e342
add kv_delete test
suprjinx Nov 2, 2024
95c5b7b
PR changes
suprjinx Nov 4, 2024
f6b76ab
Merge branch 'main' of github.com:g-research/astral
suprjinx Nov 4, 2024
2a77c25
Merge branch 'main' into kv_identity
suprjinx Nov 4, 2024
2382e25
Add kv-read policy verification
suprjinx Nov 4, 2024
6bf2070
unneeded email var
suprjinx Nov 4, 2024
10fea6a
Add advisory lock to prevent policies race
suprjinx Nov 5, 2024
17cd743
Merge branch 'main' of github.com:G-Research/astral
suprjinx Nov 5, 2024
0902c95
Merge branch 'main' of github.com:suprjinx/astral
suprjinx Nov 5, 2024
792dc28
Merge branch 'main' into kv_identity_verify
suprjinx Nov 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -304,6 +307,7 @@ DEPENDENCIES
sqlite3 (>= 1.4)
tzinfo-data
vault
with_advisory_lock

BUNDLED WITH
2.5.11
3 changes: 2 additions & 1 deletion app/lib/clients/vault/key_value.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down
18 changes: 14 additions & 4 deletions app/lib/clients/vault/policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,20 @@ 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is unfortunate that this mechanism isn't provided by vault itself, because this won't prevent another user/app from modifying the entity directly

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, why not just use a ruby mutex: https://ruby-doc.org/core-2.5.1/Mutex.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i tried! but there are apparently multiple instances of the server running, which won't share the mutex attribute.

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)
sub = identity.sub
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
Expand Down
11 changes: 10 additions & 1 deletion test/lib/clients/vault_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,23 @@ 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
read_secret = @client.kv_read(@identity, path)
assert_nil read_secret
end


test "entity_alias methods" do
# confirm no entity yet
err = assert_raises RuntimeError do
Expand Down