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.

Sign up for GitHub

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

Jump to bottom

Spike: adding/updating annotations via PUT/POST/PATCH #2888

Merged
merged 5 commits into from
Aug 21, 2023
Merged

Spike: adding/updating annotations via PUT/POST/PATCH #2888

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7f2be27
Cukes: use CONJUR_APPLIANCE_URL before http://conjur
john-odonnell Jul 28, 2023
c50cdea
Capture current behavior of policy load to create/update annot
john-odonnell Jul 31, 2023
36c8dbe
Tag new Cucumber tests
john-odonnell Aug 14, 2023
9141a2d
Fail additive policy load requests that update existing resources
john-odonnell Aug 17, 2023
b1028a1
Create and use unique DisallowedPolicyOperation exception
john-odonnell Aug 18, 2023
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 CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
mitigates the possibility of a web worker becoming starved while waiting for
a connection to become available.
[cyberark/conjur#2875](https://github.com/cyberark/conjur/pull/2875)
- Additive policy requests submitted via POST are rejected with a 400 status if
they attempt to update an existing resource.
[cyberark/conjur#2888](https://github.com/cyberark/conjur/pull/2888)

### Fixed
- Support Authn-IAM regional requests when host value is missing from signed headers.
Expand Down
2 changes: 1 addition & 1 deletion app/models/loader/create_policy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def call

@loader.delete_shadowed_and_duplicate_rows

@loader.store_policy_in_db
@loader.store_policy_in_db(reject_duplicates: true)

@loader.release_db_connection
end
Expand Down
8 changes: 5 additions & 3 deletions app/models/loader/orchestrate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,9 @@ def delete_shadowed_and_duplicate_rows
end

# TODO: consider renaming this method
def store_policy_in_db
eliminate_duplicates_pk
def store_policy_in_db(reject_duplicates: false)
Copy link

Choose a reason for hiding this comment

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

Loader::Orchestrate#store_policy_in_db has boolean parameter 'reject_duplicates'

removed_duplicates_count = eliminate_duplicates_pk
raise ApplicationController::BadRequest, "Updating existing resource disallowed in additive policy operation" if removed_duplicates_count.positive? && reject_duplicates
Copy link

Choose a reason for hiding this comment

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

Loader::Orchestrate#store_policy_in_db is controlled by argument 'reject_duplicates'

Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to raise a local exception to avoid coupling the loader to the somewhat global ApplicationController. Have a look at Exceptions::InvalidPolicyObject for inspiration, and see how it is rescued from in ApplicationController, see

conjur/app/controllers/application_controller.rb

Line 61 in 956e8b6

rescue_from Exceptions::InvalidPolicyObject, with: :policy_invalid

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created Exceptions::DisallowedPolicyOperation


insert_new

Expand Down Expand Up @@ -243,8 +244,9 @@ def eliminate_duplicates_exact
end

# Delete rows from the new policy which have the same primary keys as existing rows.
# Returns the total number of deleted rows.
def eliminate_duplicates_pk
TABLES.each do |table|
TABLES.sum do |table|
eliminate_duplicates(table, Array(model_for_table(table).primary_key) + [ :policy_id ])
end
end
Expand Down
2 changes: 1 addition & 1 deletion config/environments/test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require 'test/audit_sink'

parallel_cuke_vars = {}
parallel_cuke_vars['CONJUR_APPLIANCE_URL'] = "http://conjur#{ENV['TEST_ENV_NUMBER']}"
parallel_cuke_vars['CONJUR_APPLIANCE_URL'] = ENV.fetch('CONJUR_APPLIANCE_URL', "http://conjur#{ENV['TEST_ENV_NUMBER']}")
parallel_cuke_vars['DATABASE_URL'] = "postgres://postgres@pg#{ENV['TEST_ENV_NUMBER']}/postgres"
parallel_cuke_vars['CONJUR_AUTHN_API_KEY'] = ENV["CONJUR_AUTHN_API_KEY#{ENV['TEST_ENV_NUMBER']}"]
parallel_cuke_vars['AUTHN_LOCAL_SOCKET'] = ENV["AUTHN_LOCAL_SOCKET#{ENV['TEST_ENV_NUMBER']}"]
Expand Down
2 changes: 1 addition & 1 deletion cucumber/_authenticators_common/features/support/env.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

parallel_cuke_vars = {}
parallel_cuke_vars['CONJUR_APPLIANCE_URL'] = "http://conjur#{ENV['TEST_ENV_NUMBER']}"
parallel_cuke_vars['CONJUR_APPLIANCE_URL'] = ENV.fetch('CONJUR_APPLIANCE_URL', "http://conjur#{ENV['TEST_ENV_NUMBER']}")
parallel_cuke_vars['DATABASE_URL'] = "postgres://postgres@pg#{ENV['TEST_ENV_NUMBER']}/postgres"
parallel_cuke_vars['CONJUR_AUTHN_API_KEY'] = ENV["CONJUR_AUTHN_API_KEY#{ENV['TEST_ENV_NUMBER']}"]

Expand Down
2 changes: 1 addition & 1 deletion cucumber/_authenticators_common/features/support/hooks.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# run independently.
Before do
parallel_cuke_vars = {}
parallel_cuke_vars['CONJUR_APPLIANCE_URL'] = "http://conjur#{ENV['TEST_ENV_NUMBER']}"
parallel_cuke_vars['CONJUR_APPLIANCE_URL'] = ENV.fetch('CONJUR_APPLIANCE_URL', "http://conjur#{ENV['TEST_ENV_NUMBER']}")
parallel_cuke_vars['DATABASE_URL'] = "postgres://postgres@pg#{ENV['TEST_ENV_NUMBER']}/postgres"
parallel_cuke_vars['CONJUR_AUTHN_API_KEY'] = ENV["CONJUR_AUTHN_API_KEY#{ENV['TEST_ENV_NUMBER']}"]

Expand Down
Loading
Loading