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

Combinations of allow/deny rules with actions and repGlob are not fully set #48

Closed
gruberroland-netcentric opened this issue Nov 16, 2015 · 10 comments
Labels

Comments

@gruberroland-netcentric
Copy link

gruberroland-netcentric commented Nov 16, 2015

Example:

  • path: /content
    permission: deny
    actions: read,acl_read
    privileges:
    repGlob:
  • path: /content
    permission: allow
    actions: read,acl_read
    privileges:
    repGlob: ""
  • path: /content
    permission: allow
    actions: read,acl_read
    privileges:
    repGlob: /jcr:*

​I just get the two allow rules. The deny rule is not set.
Probably, this is because when setting the first allow rule without repglob then the deny rule is removed. I saw this kind of effect when I debugged the tool some time ago, OAK seems to remove rules if they are obsoleted by another rule.
Probably, here the second rule with allow removes the first rule since it is added without repglob first (added in a second code step).

@kwin
Copy link
Member

kwin commented Nov 16, 2015

The reason for that is that the Security API for setting actions does not support restrictions. Therefore what we do now in the implementation is

  1. set the action without the restriction (through the Security API) (that one remove duplicate actions) and then
  2. add the restriction on the lastly added action.

In your example the first action and the last action without restrictions are redundant.
As soon as you modify the order, you should not run into that problem.
From an implementation perspective this is not that easy to fix, as the official API does simply not support setting restrictions on actions and reimplementing actions on something we should avoid.

@gruberroland-netcentric
Copy link
Author

But if I change the order then the rights are not the same.

@kwin
Copy link
Member

kwin commented Nov 17, 2015

Yes, changing the order will change the semantics here. But as a workaround you can split the deny and allow parts into different groups, where one is member of the other. That way the actions are not removed before setting the restrictions.

@seuck
Copy link

seuck commented Nov 18, 2015

A similar bug is happening with just these two rules:

- path: /content
  permission: deny
  actions:
  privileges: jcr:all
  repGlob:

- path: /content
  permission: allow
  actions: read
  privileges:
  repGlob: ""

Read privilege will not be included in deny rule.

@kwin
Copy link
Member

kwin commented Nov 27, 2015

Another workaround is to use privileges instead of actions.

@albertoalmagro
Copy link

Workaround verified. Actions done:

  • Move allow permissions to fragment-basic-allow.
  • Use privileges instead of actions.

Before:
`

- fragment-basic-allow:

   - path: /
     permission: allow
     actions: read
     privileges: 
     repGlob: 

- fragment-restrict-for-everyone:

   - path: /content
     permission: deny
     actions: read,acl_read
     privileges: 
     repGlob: 

   - path: /content
     permission: allow
     actions: read,acl_read
     privileges: 
     repGlob: ""

   - path: /content
     permission: allow
     actions: read,acl_read
     privileges: 
     repGlob: '/jcr:*'`

After:
`

- fragment-basic-allow:

   - path: /
     permission: allow
     actions: read
     privileges: 
     repGlob: 

   - path: /content
     permission: allow
     actions: 
     privileges: jcr:readAccessControl,jcr:read
     repGlob: ""

   - path: /content
     permission: allow
     actions: 
     privileges: jcr:readAccessControl,jcr:read
     repGlob: '/jcr:*'

- fragment-restrict-for-everyone:

   - path: /content
     permission: deny
     actions: read,acl_read
     privileges: 
     repGlob: `

@gruberroland-netcentric
Copy link
Author

Added comment to README that privileges should be used.

@kwin
Copy link
Member

kwin commented Apr 4, 2016

I would prefer to keep this issue open and reference this issue in the readme (to provide a littlebit more information around that limitation)

@kwin kwin reopened this Apr 4, 2016
gruberroland-netcentric pushed a commit that referenced this issue Apr 4, 2016
@kwin kwin changed the title Combinations of allow/deny rules with repGlob are not fully set Combinations of allow/deny rules with actions and repGlob are not fully set Jun 28, 2016
@ghenzler
Copy link
Member

ghenzler commented Jul 14, 2016

Another possible workaround is to use repGlob: "*":

   - path: /content
     permission: deny
     actions: read,acl_read
     privileges: 
     repGlob: "*"

   - path: /content
     permission: allow
     actions: read,acl_read
     privileges: 
     repGlob: ""

   - path: /content
     permission: allow
     actions: read,acl_read
     privileges: 
     repGlob: '/jcr:*'`

@ghenzler
Copy link
Member

With the changes from #155, the following example works perfectly fine (and produces three ACEs):

- group-test-issue48:
        - name: "Test Group for testing #48"

- ace_config:

    - group-test-issue48:

        - path: /content/geometrixx
          permission: deny
          actions: read,acl_read
          repGlob:

        - path: /content/geometrixx
          permission: allow
          actions: read,acl_read
          repGlob: ''

        - path: /content/geometrixx
          permission: allow
          actions: read,acl_read
          repGlob: '/jcr:*'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants