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

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

Merged
merged 5 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
12 changes: 12 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class UnprocessableEntity < RuntimeError
rescue_from Sequel::ForeignKeyConstraintViolation, with: :foreign_key_constraint_violation
rescue_from Conjur::PolicyParser::Invalid, with: :policy_invalid
rescue_from Exceptions::InvalidPolicyObject, with: :policy_invalid
rescue_from Exceptions::DisallowedPolicyOperation, with: :disallowed_policy_operation
rescue_from ArgumentError, with: :argument_error
rescue_from ActionController::ParameterMissing, with: :argument_error
rescue_from UnprocessableEntity, with: :unprocessable_entity
Expand Down Expand Up @@ -193,6 +194,17 @@ def policy_invalid e
render(json: { error: error }, status: :unprocessable_entity)
end

def disallowed_policy_operation e
Copy link

Choose a reason for hiding this comment

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

ApplicationController#disallowed_policy_operation has the parameter name 'e'

logger.debug("#{e}\n#{e.backtrace.join("\n")}")

render(json: {
error: {
code: "disallowed_policy_operation",
message: e.message
}
}, status: :unprocessable_entity)
end

def argument_error e
logger.debug("#{e}\n#{e.backtrace.join("\n")}")

Expand Down
11 changes: 11 additions & 0 deletions app/models/exceptions/disallowed_policy_operation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

module Exceptions
class DisallowedPolicyOperation < RuntimeError

def initialize
super("Updating existing resource disallowed in additive policy operation")
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing this. I was thinking it might be useful to provide any meaningful context around the error like how it's done here.

def initialize id, message: nil

Specifically I had in mind

  1. Policy ID
  2. Offending lines etc.

end

end
end
2 changes: 1 addition & 1 deletion app/models/loader/create_policy.rb
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
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 Exceptions::DisallowedPolicyOperation 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'


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
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
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
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