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

Enhance ACL policies to enable access rules that apply to users not in a group or not by username. #4769

Merged
merged 5 commits into from May 22, 2019

Conversation

@jtobard
Copy link
Contributor

commented Apr 30, 2019

A new ACL policy that applies to all users who are not in the specified group or username (including regular expression).

The new syntax replaces by: for notBy:

acl editor

acl page

As an example:

notBy:
  username: dev
description: run-job
for:
  job:
  - allow:
    - run
    equals:
      name: cleanTempFolders
context:
  project: example

Everyone that is not the user dev can run a job called cleanTempFolders in the project example

fix #3810

jtobard added some commits Apr 22, 2019

@jtobard jtobard added this to the 3.1.0 milestone May 1, 2019

@gschueler

This comment has been minimized.

Copy link
Member

commented May 9, 2019

@jtobard what is the behavior in a notBy clause with multiple usernames?

@jtobard

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

@gschueler if the username does not correspond to the regular expression it takes the permissions.

using

notBy: 
     username: deva|devb

The user deva is excluded also devb, but user devc gets the permissions.
I'm going to include a test for this case.

@jtobard jtobard requested a review from gschueler May 12, 2019

@gschueler

This comment has been minimized.

Copy link
Member

commented May 18, 2019

I wonder if we need some safety rails on this. the behavior for acls is "reject unless explicitly allowed". however with the notBy clause it might be easy to accidentally grant access, since you can easily do allow: ..., notBy: username: 'bob', which would grant it to everyone else. thoughts @ahonor ?

@ahonor

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

True it does go a bit against the deny be default. What safety rails were you considering?

@gschueler

This comment has been minimized.

Copy link
Member

commented May 20, 2019

An idea is to make notBy only valid for deny rules. that would make it able to only restrict access, and not increase access to anyone. I think that is in the spirit of the original use case in #3810

@jtobard

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

I will modify the Pull request to reflect that restriction

@jtobard jtobard merged commit 7f0913b into master May 22, 2019

21 checks passed

Mergeable Mergeable Run has been Completed!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
security/snyk - build.gradle (rundeck) No manifest changes detected
security/snyk - core/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/copyfile-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/flow-control-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/git-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/jasypt-encryption-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/job-state-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/localexec-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/orchestrator-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/script-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/source-refresh-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/stub-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - plugins/upvar-plugin/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeck-storage/build.gradle (rundeck) No manifest changes detected
security/snyk - rundeckapp/build.gradle (rundeck) No new issues
Details
security/snyk - rundeckapp/grails-spa/package.json (rundeck) No new issues
Details
security/snyk - rundeckapp/metricsweb/build.gradle (rundeck) No manifest changes detected

@jtobard jtobard deleted the issue/3810 branch Jul 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.