From 89d3034336c300b9d522b35d8d681d0d13e0a4cc Mon Sep 17 00:00:00 2001 From: Matthieu Huin Date: Wed, 25 Sep 2019 17:44:46 +0200 Subject: [PATCH] Authorization rules: support YAML nested dictionaries Allow operators to define complex rules either using the flattened JSONPath syntax, or the more YAML-friendly nested dicts. Change-Id: Iabf65313e114dcb15788844a1f0095ae52567275 --- doc/source/admin/tenants.rst | 89 +++++++++++++++++++-------------- tests/unit/test_configloader.py | 27 +++++++++- zuul/model.py | 32 +++++++++++- 3 files changed, 109 insertions(+), 39 deletions(-) diff --git a/doc/source/admin/tenants.rst b/doc/source/admin/tenants.rst index d8f4272227..8281d06316 100644 --- a/doc/source/admin/tenants.rst +++ b/doc/source/admin/tenants.rst @@ -355,7 +355,9 @@ Below are some examples of how access rules can be defined: - admin-rule: name: affiliate_or_admin conditions: - - resources_access.account.roles: "affiliate" + - resources_access: + account: + roles: "affiliate" iss: external_institution - resources_access.account.roles: "admin" - admin-rule: @@ -381,7 +383,8 @@ Below are some examples of how access rules can be defined: This is the list of conditions that define a rule. A JWT must match **at least one** of the conditions for the rule to apply. A condition is a dictionary where keys are claims. **All** the associated values must - match the claims in the user's token. + match the claims in the user's token; in other words the condition dictionary + must be a "sub-dictionary" of the user's JWT. Zuul's authorization engine will adapt matching tests depending on the nature of the claim in the token, eg: @@ -391,45 +394,57 @@ Below are some examples of how access rules can be defined: * if the claim is a string, check that the condition value is equal to the claim's value - In order to allow the parsing of claims with complex structures like - dictionaries, claim names can be written in the XPath format. + The claim names can also be written in the XPath format for clarity: the + condition - The special ``zuul_uid`` claim refers to the ``uid_claim`` setting in an - authenticator's configuration. By default it refers to the ``sub`` claim - of a token. For more details see the :ref:`configuration section - ` for Zuul web server. + .. code-block:: yaml - Under the above example, the following token would match rules - ``affiliate_or_admin`` and ``alice_or_bob``: + resources_access: + account: + roles: "affiliate" - .. code-block:: javascript + is equivalent to the condition - { - 'iss': 'external_institution', - 'aud': 'my_zuul_deployment', - 'exp': 1234567890, - 'iat': 1234556780, - 'sub': 'alice', - 'resources_access': { - 'account': { - 'roles': ['affiliate', 'other_role'] - } - }, - } + .. code-block:: yaml - And this token would only match rule ``affiliate_or_admin``: + resources_access.account.roles: "affiliate" - .. code-block:: javascript + The special ``zuul_uid`` claim refers to the ``uid_claim`` setting in an + authenticator's configuration. By default it refers to the ``sub`` claim + of a token. For more details see the :ref:`configuration section + ` for Zuul web server. - { - 'iss': 'some_other_institution', - 'aud': 'my_zuul_deployment', - 'exp': 1234567890, - 'sub': 'carol', - 'iat': 1234556780, - 'resources_access': { - 'account': { - 'roles': ['admin', 'other_role'] - } - }, - } + Under the above example, the following token would match rules + ``affiliate_or_admin`` and ``alice_or_bob``: + + .. code-block:: javascript + + { + 'iss': 'external_institution', + 'aud': 'my_zuul_deployment', + 'exp': 1234567890, + 'iat': 1234556780, + 'sub': 'alice', + 'resources_access': { + 'account': { + 'roles': ['affiliate', 'other_role'] + } + }, + } + + And this token would only match rule ``affiliate_or_admin``: + + .. code-block:: javascript + + { + 'iss': 'some_other_institution', + 'aud': 'my_zuul_deployment', + 'exp': 1234567890, + 'sub': 'carol', + 'iat': 1234556780, + 'resources_access': { + 'account': { + 'roles': ['admin', 'other_role'] + } + }, + } diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py index e2a8111331..4521ac7110 100644 --- a/tests/unit/test_configloader.py +++ b/tests/unit/test_configloader.py @@ -512,7 +512,7 @@ class TestAuthorizationRuleParser(ZuulTestCase): 'groups': ['admin', 'ghostbusters']} self.assertTrue(rule(claims)) - def test_check_complex_rule_from_yaml(self): + def test_check_complex_rule_from_yaml_jsonpath(self): rule_d = {'name': 'my-rule', 'conditions': [{'hello.this.is': 'a complex value'}, ], @@ -523,6 +523,31 @@ class TestAuthorizationRuleParser(ZuulTestCase): 'hello': { 'this': { 'is': 'a complex value' + }, + 'and': { + 'this one': 'too' + } + } + } + self.assertTrue(rule(claims)) + + def test_check_complex_rule_from_yaml_nested_dict(self): + rule_d = {'name': 'my-rule', + 'conditions': [{'hello': {'this': {'is': 'a complex value' + } + } + }, + ], + } + rule = AuthorizationRuleParser().fromYaml(rule_d) + self.assertEqual('my-rule', rule.name) + claims = {'iss': 'my-idp', + 'hello': { + 'this': { + 'is': 'a complex value' + }, + 'and': { + 'this one': 'too' } } } diff --git a/zuul/model.py b/zuul/model.py index 0a0ba3add3..510be755a3 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -4821,7 +4821,7 @@ class ClaimRule(AuthZRule): self.claim = claim or 'sub' self.value = value - def __call__(self, claims): + def _match_jsonpath(self, claims): matches = [match.value for match in jsonpath_rw.parse(self.claim).find(claims)] if len(matches) == 1: @@ -4837,6 +4837,36 @@ class ClaimRule(AuthZRule): # TODO we should differentiate no match and 2+ matches return False + def _match_dict(self, claims): + def _compare(value, claim): + if isinstance(value, list): + if isinstance(claim, list): + # if the claim is empty, the value must be empty too: + if claim == []: + return value == [] + else: + return (set(claim) <= set(value)) + else: + return claim in value + elif isinstance(value, dict): + if not isinstance(claim, dict): + return False + elif value == {}: + return claim == {} + else: + return all(_compare(value[x], claim.get(x, {})) + for x in value.keys()) + else: + return value == claim + + return _compare(self.value, claims.get(self.claim, {})) + + def __call__(self, claims): + if isinstance(self.value, dict): + return self._match_dict(claims) + else: + return self._match_jsonpath(claims) + def __eq__(self, other): if not isinstance(other, ClaimRule): return False