cyberark / conjur-policy-parser

Parser library for Conjur Policy YAML
MIT License
0 stars 1 forks source link

Fix handling of user with absolute ids #11

Closed dividedmind closed 6 years ago

dividedmind commented 6 years ago

Fixes cyberark/conjur#600 (waffle take notice!)

dividedmind commented 6 years ago

Can you give an example of the policy you have in mind?

apotterri commented 6 years ago

Here's one:

---
- !user alice
- !policy
  id: foo
  body:
    - !policy
      id: bar
      body:
        - !group users
        - !grant
          role: !group users
          members:
            - !user /alice

I also confirmed that it doesn't load:

{"error":{"code":"not_found","message":"User '/alice@foo-bar' not found in account 'cucumber'","target":"user","details":{"code":"not_found","target":"id","message":"cucumber:user:/alice@foo-bar"}}}
error: 404 Not Found
dividedmind commented 6 years ago

@apotterri thanks, I added that policy as a test verbatim. It loads fine with the fix.

dividedmind commented 6 years ago

There's an outstanding issue with this change, in particular ids beginning with / are always considered absolute, whereas before the change they were considered regular namespaced ids that just happen to start with /. I think this was confusing (the meaning was different depending on where the id was used) and in any case I don't think anybody ought to have relied on that, so I think we should make it an error. Waiting for input from @kgilpin however.

dividedmind commented 6 years ago

@apotterri, mind taking a look at that latest commit?

dividedmind commented 6 years ago

@apotterri Can you take another look please? :) (Don't worry about the conflict, I'll resolve it before merging.)

apotterri commented 6 years ago

Looks good, thanks for making these changes.