zuul
/
zuul
Code Issues Proposed changes

Authorization rules: add templating 

Operators can use the "{tenant.name}" special word when setting conditions'
values. This special word will be replaced at evaluation time by the
name of the tenant for which the authorization check is being done.

Change-Id: I6f1cf14ad29e775d9090e54b4a633384eef61085
This commit is contained in:
Matthieu Huin 2020-01-31 13:34:03 +01:00
parent ceb12831b1
commit c8aafb4ab3
20 changed files with 444 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
doc/source/reference/tenants.rst
View File

@ -460,3 +460,57 @@ Below are some examples of how access rules can be defined:
}
},
}
Access Rule Templating
----------------------
The special word "{tenant.name}" can be used in conditions' values. It will be automatically
substituted for the relevant tenant when evaluating authorizations for a given
set of claims. For example, consider the following rule:
.. code-block:: yaml
- admin-rule:
name: tenant_in_groups
conditions:
- groups: "{tenant.name}"
If applied to the following tenants:
.. code-block:: yaml
- tenant:
name: tenant-one
admin-rules:
- tenant_in_groups
- tenant:
name: tenant-two
admin-rules:
- tenant_in_groups
Then this set of claims will be allowed to perform protected actions on **tenant-one**:
.. code-block:: javascript
{
'iss': 'some_other_institution',
'aud': 'my_zuul_deployment',
'exp': 1234567890,
'sub': 'carol',
'iat': 1234556780,
'groups': ['tenant-one', 'some-other-group'],
}
And this set of claims will be allowed to perform protected actions on **tenant-one**
and **tenant-two**:
.. code-block:: javascript
{
'iss': 'some_other_institution',
'aud': 'my_zuul_deployment',
'exp': 1234567890,
'sub': 'carol',
'iat': 1234556780,
'groups': ['tenant-one', 'tenant-two'],
}

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/nonvoting-project-merge.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/nonvoting-project-test1.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/nonvoting-project-test2.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/project-merge.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/project-post.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/project-test1.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/project-test2.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/project-testfile.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

2
tests/fixtures/config/authorization/rules-templating/git/common-config/playbooks/project1-project2-integration.yaml vendored Normal file
View File

@ -0,0 +1,2 @@
- hosts: all
tasks: []

197
tests/fixtures/config/authorization/rules-templating/git/common-config/zuul.yaml vendored Normal file
View File

@ -0,0 +1,197 @@
- pipeline:
name: check
manager: independent
trigger:
gerrit:
- event: patchset-created
success:
gerrit:
Verified: 1
failure:
gerrit:
Verified: -1
- pipeline:
name: gate
manager: dependent
success-message: Build succeeded (gate).
trigger:
gerrit:
- event: comment-added
approval:
- Approved: 1
success:
gerrit:
Verified: 2
submit: true
failure:
gerrit:
Verified: -2
start:
gerrit:
Verified: 0
precedence: high
- pipeline:
name: post
manager: independent
trigger:
gerrit:
- event: ref-updated
ref: ^(?!refs/).*$
precedence: low
- job:
name: base
parent: null
- job:
name: project-merge
hold-following-changes: true
nodeset:
nodes:
- name: controller
label: label1
run: playbooks/project-merge.yaml
- job:
name: project-test1
attempts: 4
nodeset:
nodes:
- name: controller
label: label1
run: playbooks/project-test1.yaml
- job:
name: project-test1
branches: stable
nodeset:
nodes:
- name: controller
label: label2
run: playbooks/project-test1.yaml
- job:
name: project-post
nodeset:
nodes:
- name: static
label: ubuntu-xenial
run: playbooks/project-post.yaml
- job:
name: project-test2
nodeset:
nodes:
- name: controller
label: label1
run: playbooks/project-test2.yaml
- job:
name: project1-project2-integration
nodeset:
nodes:
- name: controller
label: label1
run: playbooks/project1-project2-integration.yaml
- job:
name: project-testfile
files:
- .*-requires
run: playbooks/project-testfile.yaml
- project:
name: org/project
check:
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
gate:
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project-testfile:
dependencies: project-merge
post:
jobs:
- project-post
- project:
name: org/project1
check:
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project1-project2-integration:
dependencies: project-merge
gate:
queue: integrated
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project1-project2-integration:
dependencies: project-merge
- project:
name: org/project2
check:
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project1-project2-integration:
dependencies: project-merge
gate:
queue: integrated
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project1-project2-integration:
dependencies: project-merge
- project:
name: common-config
check:
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project1-project2-integration:
dependencies: project-merge
gate:
queue: integrated
jobs:
- project-merge
- project-test1:
dependencies: project-merge
- project-test2:
dependencies: project-merge
- project1-project2-integration:
dependencies: project-merge
- job:
name: test-job
run: playbooks/project-merge.yaml
required-projects:
- org/project

1
tests/fixtures/config/authorization/rules-templating/git/org_project/README vendored Normal file
View File

@ -0,0 +1 @@
test

1
tests/fixtures/config/authorization/rules-templating/git/org_project1/README vendored Normal file
View File

@ -0,0 +1 @@
test

1
tests/fixtures/config/authorization/rules-templating/git/org_project2/README vendored Normal file
View File

@ -0,0 +1 @@
test

34
tests/fixtures/config/authorization/rules-templating/main.yaml vendored Normal file
View File

@ -0,0 +1,34 @@
- admin-rule:
name: tenant-admin
conditions:
- groups: "{tenant.name}"
- tenant:
name: tenant-zero
admin-rules:
- tenant-admin
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project
- tenant:
name: tenant-one
admin-rules:
- tenant-admin
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project1
- tenant:
name: tenant-two
admin-rules:
- tenant-admin
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project2

29
tests/fixtures/config/tenant-parser/authorizations-templating.yaml vendored Normal file
View File

@ -0,0 +1,29 @@
- admin-rule:
name: tenant-admin
conditions:
- group: "{tenant.name}-admin"
- admin-rule:
name: tenant-admin-complex
conditions:
- path.to.group: "{tenant.name}-admin"
- tenant:
name: tenant-one
admin-rules:
- tenant-admin
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project1
- tenant:
name: tenant-two
admin-rules:
- tenant-admin
- tenant-admin-complex
source:
gerrit:
config-projects:
- common-config
untrusted-projects:
- org/project2

56
tests/unit/test_configloader.py
View File

@ -554,6 +554,62 @@ class TestAuthorizationRuleParser(ZuulTestCase):
self.assertTrue(rule(claims))
class TestAuthorizationRuleParserWithTemplating(ZuulTestCase):
tenant_config_file = 'config/tenant-parser/authorizations-templating.yaml'
def test_rules_are_loaded(self):
rules = self.sched.abide.admin_rules
self.assertTrue('tenant-admin' in rules, self.sched.abide)
self.assertTrue('tenant-admin-complex' in rules, self.sched.abide)
def test_tenant_substitution(self):
claims_1 = {'group': 'tenant-one-admin'}
claims_2 = {'group': 'tenant-two-admin'}
rules = self.sched.abide.admin_rules
tenant_one = self.sched.abide.tenants.get('tenant-one')
tenant_two = self.sched.abide.tenants.get('tenant-two')
self.assertTrue(rules['tenant-admin'](claims_1,
tenant_one))
self.assertTrue(rules['tenant-admin'](claims_2,
tenant_two))
self.assertTrue(not rules['tenant-admin'](claims_1,
tenant_two))
self.assertTrue(not rules['tenant-admin'](claims_2,
tenant_one))
def test_tenant_substitution_in_list(self):
claims_1 = {'group': ['tenant-one-admin', 'some-other-tenant']}
claims_2 = {'group': ['tenant-two-admin', 'some-other-tenant']}
rules = self.sched.abide.admin_rules
tenant_one = self.sched.abide.tenants.get('tenant-one')
tenant_two = self.sched.abide.tenants.get('tenant-two')
self.assertTrue(rules['tenant-admin'](claims_1,
tenant_one))
self.assertTrue(rules['tenant-admin'](claims_2,
tenant_two))
self.assertTrue(not rules['tenant-admin'](claims_1,
tenant_two))
self.assertTrue(not rules['tenant-admin'](claims_2,
tenant_one))
def test_tenant_substitution_in_dict(self):
claims_2 = {
'path': {
'to': {
'group': 'tenant-two-admin'
}
}
}
rules = self.sched.abide.admin_rules
tenant_one = self.sched.abide.tenants.get('tenant-one')
tenant_two = self.sched.abide.tenants.get('tenant-two')
self.assertTrue(
not rules['tenant-admin-complex'](claims_2,
tenant_one))
self.assertTrue(
rules['tenant-admin-complex'](claims_2, tenant_two))
class TestTenantExtra(TenantParserTestCase):
tenant_config_file = 'config/tenant-parser/extra.yaml'

26
tests/unit/test_scheduler.py
View File

@ -138,6 +138,32 @@ class TestAuthorizeViaRPC(ZuulTestCase):
self.assertTrue(json.loads(authorized))
class TestAuthorizeWithTemplatingViaRPC(ZuulTestCase):
tenant_config_file = 'config/authorization/rules-templating/main.yaml'
def test_authorize_via_rpc(self):
client = zuul.rpcclient.RPCClient('127.0.0.1',
self.gearman_server.port)
self.addCleanup(client.shutdown)
tenants = ['tenant-zero', 'tenant-one', 'tenant-two']
for t_claim in tenants:
claims = {'groups': [t_claim, ]}
for tenant in tenants:
authorized = client.submitJob('zuul:authorize_user',
{'tenant': tenant,
'claims': claims}).data[0]
if t_claim == tenant:
self.assertTrue(
json.loads(authorized),
"Failed for t_claim: %s, tenant: %s" % (t_claim,
tenant))
else:
self.assertTrue(
not json.loads(authorized),
"Failed for t_claim: %s, tenant: %s" % (t_claim,
tenant))
class TestSchedulerAutoholdHoldExpiration(ZuulTestCase):
'''
This class of tests validates the autohold node expiration values

41
zuul/model.py
View File

@ -4839,15 +4839,22 @@ class ClaimRule(AuthZRule):
self.claim = claim or 'sub'
self.value = value
def _match_jsonpath(self, claims):
def templated(self, value, tenant=None):
template_dict = {}
if tenant is not None:
template_dict['tenant'] = tenant.getSafeAttributes()
return value.format(**template_dict)
def _match_jsonpath(self, claims, tenant):
matches = [match.value
for match in jsonpath_rw.parse(self.claim).find(claims)]
t_value = self.templated(self.value, tenant)
if len(matches) == 1:
match = matches[0]
if isinstance(match, list):
return self.value in match
return t_value in match
elif isinstance(match, str):
return self.value == match
return t_value == match
else:
# unsupported type - don't raise, but this should be notified
return False
@ -4855,15 +4862,16 @@ class ClaimRule(AuthZRule):
# TODO we should differentiate no match and 2+ matches
return False
def _match_dict(self, claims):
def _match_dict(self, claims, tenant):
def _compare(value, claim):
if isinstance(value, list):
t_value = map(self.templated, value, [tenant] * len(value))
if isinstance(claim, list):
# if the claim is empty, the value must be empty too:
if claim == []:
return value == []
return t_value == []
else:
return (set(claim) <= set(value))
return (set(claim) <= set(t_value))
else:
return claim in value
elif isinstance(value, dict):
@ -4875,15 +4883,16 @@ class ClaimRule(AuthZRule):
return all(_compare(value[x], claim.get(x, {}))
for x in value.keys())
else:
return value == claim
t_value = self.templated(value, tenant)
return t_value == claim
return _compare(self.value, claims.get(self.claim, {}))
def __call__(self, claims):
def __call__(self, claims, tenant=None):
if isinstance(self.value, dict):
return self._match_dict(claims)
return self._match_dict(claims, tenant)
else:
return self._match_jsonpath(claims)
return self._match_jsonpath(claims, tenant)
def __eq__(self, other):
if not isinstance(other, ClaimRule):
@ -4903,8 +4912,8 @@ class OrRule(AuthZRule):
super(OrRule, self).__init__()
self.rules = set(subrules)
def __call__(self, claims):
return any(rule(claims) for rule in self.rules)
def __call__(self, claims, tenant=None):
return any(rule(claims, tenant) for rule in self.rules)
def __eq__(self, other):
if not isinstance(other, OrRule):
@ -4924,8 +4933,8 @@ class AndRule(AuthZRule):
super(AndRule, self).__init__()
self.rules = set(subrules)
def __call__(self, claims):
return all(rule(claims) for rule in self.rules)
def __call__(self, claims, tenant=None):
return all(rule(claims, tenant) for rule in self.rules)
def __eq__(self, other):
if not isinstance(other, AndRule):
@ -4946,8 +4955,8 @@ class AuthZRuleTree(object):
# initialize actions as unauthorized
self.ruletree = None
def __call__(self, claims):
return self.ruletree(claims)
def __call__(self, claims, tenant=None):
return self.ruletree(claims, tenant)
def __eq__(self, other):
if not isinstance(other, AuthZRuleTree):

3
zuul/rpclistener.py
View File

@ -303,7 +303,8 @@ class RPCListener(object):
'"%s" to claims %s')
self.log.debug(
debug_msg % (rule, tenant, json.dumps(claims)))
authorized = self.sched.abide.admin_rules[rule](claims)
authorized = self.sched.abide.admin_rules[rule](claims,
tenant)
if authorized:
if '__zuul_uid_claim' in claims:
uid = claims['__zuul_uid_claim']