Skip to content

Commit

Permalink
Fail additive policy load requests that update existing resources
Browse files Browse the repository at this point in the history
  • Loading branch information
john-odonnell committed Aug 18, 2023
1 parent 5ec5cc9 commit 2f9ed46
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 53 deletions.
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
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)
removed_duplicates_count = eliminate_duplicates_pk
raise ApplicationController::BadRequest, "Updating existing resource disallowed in additive policy operation" if removed_duplicates_count.positive? && 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
46 changes: 7 additions & 39 deletions cucumber/api/features/policy_load_annotations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Feature: Updating Policies with Annotations
- create / add new / POST : EXPECTED SUCCESS
- create / add new / PATCH : EXPECTED FAIL - 403 on policy load
- create / update existing / PUT : EXPECTED FAIL - 403 on policy load
- create / update existing / POST : EXPECTED FAIL - 20x on policy load, annot not updated
- create / update existing / POST : EXPECTED FAIL - 400 on policy load
- create / update existing / PATCH : EXPECTED FAIL - 403 on policy load
- update / add new / PUT : EXPECTED SUCCESS
- update / add new / POST : EXPECTED FAIL - 403 on policy load
Expand All @@ -26,13 +26,6 @@ Feature: Updating Policies with Annotations
- update / update existing / POST : EXPECTED FAIL - 403 on policy load
- update / update existing / PATCH : EXPECTED SUCCESS

All these outcomes align with our expectations, but one may not align with
user expectations: ( create / update existing / POST ). A user may expect that
a policy load that tries and fails to update the content of a given annotation
should either provide a warning or fail outright.

How can we update how we handle policy to fail in this case?

Background:
Given I am the super-user
And I successfully PUT "/policies/cucumber/policy/root" with body:
Expand Down Expand Up @@ -183,45 +176,20 @@ Feature: Updating Policies with Annotations

@negative
@acceptance
Scenario: User with create privilege CAN NOT update existing annotations with POST, but policy loads successfully
Scenario: User with create privilege CAN NOT update existing annotations with POST
When I login as "alice"
Then I successfully POST "/policies/cucumber/policy/hosts" with body:
And I save my place in the log file
Then I POST "/policies/cucumber/policy/hosts" with body:
"""
- !host
id: annotated
annotations:
description: Success
"""
And I successfully GET "/resources/cucumber/host/hosts%2Fannotated"
Then the JSON should be:
Then the HTTP response status code is 400
And The following appears in the log after my savepoint:
"""
{
"annotations": [
{
"name": "description",
"policy": "cucumber:policy:hosts",
"value": "Already annotated"
}
],
"id": "cucumber:host:hosts/annotated",
"owner": "cucumber:policy:hosts",
"permissions": [
{
"policy": "cucumber:policy:root",
"privilege": "read",
"role": "cucumber:user:bob"
},
{
"policy": "cucumber:policy:root",
"privilege": "read",
"role": "cucumber:user:alice"
}
],
"policy": "cucumber:policy:hosts",
"restricted_to": [
]
}
Updating existing resource disallowed in additive policy operation
"""

@negative
Expand Down
15 changes: 5 additions & 10 deletions cucumber/api/features/policy_load_modes.feature
Original file line number Diff line number Diff line change
Expand Up @@ -172,22 +172,17 @@ Feature: Updating policies

@acceptance
Scenario: POST cannot update existing policy records
When I successfully POST "/policies/cucumber/policy/dev/db" with body:
When I save my place in the log file
And I POST "/policies/cucumber/policy/dev/db" with body:
"""
- !variable
id: b
kind: private key
"""
When I successfully GET "/resources/cucumber/variable/dev/db/b"
Then the JSON at "annotations" should be:
Then the HTTP response status code is 400
And The following appears in the log after my savepoint:
"""
[
{
"name": "conjur/kind",
"policy": "cucumber:policy:dev/db",
"value": "password"
}
]
Updating existing resource disallowed in additive policy operation
"""

@negative @acceptance
Expand Down

0 comments on commit 2f9ed46

Please sign in to comment.