diff --git a/CHANGELOG.md b/CHANGELOG.md index 57a664e7e3d2986d56e44ce6c57234502105b87e..3aa7276edc8403f37ed55694aa7a8160881096f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### +* [caosdb-pylib#106](https://gitlab.indiscale.com/caosdb/src/caosdb-pylib/-/issues/106) + Parsing Error in class caosdb.common.models.ACL + ### Security ### ### Documentation ### diff --git a/src/caosdb/common/models.py b/src/caosdb/common/models.py index 80a6ee11e707fb3776fc96b42a16b649ac575f66..181750aae6fd3e1aeab2c61b59f53d8b8111d5bd 100644 --- a/src/caosdb/common/models.py +++ b/src/caosdb/common/models.py @@ -5,9 +5,9 @@ # # Copyright (C) 2018 Research Group Biomedical Physics, # Max-Planck-Institute for Dynamics and Self-Organization Göttingen -# Copyright (C) 2020 Indiscale GmbH <info@indiscale.com> +# Copyright (C) 2020-2022 Indiscale GmbH <info@indiscale.com> # Copyright (C) 2020 Florian Spreckelsen <f.spreckelsen@indiscale.com> -# Copyright (C) 2020 Timm Fitschen <t.fitschen@indiscale.com> +# Copyright (C) 2020-2022 Timm Fitschen <t.fitschen@indiscale.com> # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as @@ -25,7 +25,14 @@ # ** end header # -"""missing docstring.""" +""" +Collection of the central classes of the CaosDB client, namely the Entity class +and all of its subclasses and the Container class which is used to carry out +transactions. + +All additional classes are either important for the entities or the +transactions. +""" from __future__ import print_function, unicode_literals import re @@ -269,14 +276,74 @@ class Entity(object): self.__pickup = new_pickup def grant(self, realm=None, username=None, role=None, - permission=None, priority=False): + permission=None, priority=False, revoke_denial=True): + """Grant a permission to a user or role for this entity. + + You must specify either only the username and the realm, or only the + role. + + By default a previously existing denial rule would be revoked, because + otherwise this grant wouldn't have any effect. However, for keeping + contradicting rules pass revoke_denial=False. + + Parameters + ---------- + permission: str + The permission to be granted. + username : str, optional + The username. Exactly one is required, either the `username` or the + `role`. + realm: str, optional + The user's realm. Required when username is not None. + role: str, optional + The role (as in Role-Based Access Control). Exactly one is + required, either the `username` or the `role`. + priority: bool, default False + Whether this permission is granted with priority over non-priority + rules. + revoke_denial: bool, default True + Whether a contradicting denial (with same priority flag) in this + ACL will be revoked. + """ + # @review Florian Spreckelsen 2022-03-17 self.acl.grant(realm=realm, username=username, role=role, - permission=permission, priority=priority) + permission=permission, priority=priority, + revoke_denial=revoke_denial) def deny(self, realm=None, username=None, role=None, - permission=None, priority=False): + permission=None, priority=False, revoke_grant=True): + """Deny a permission to a user or role for this entity. + + You must specify either only the username and the realm, or only the + role. + + By default a previously existing grant rule would be revoked, because + otherwise this denial would override the grant rules anyways. However, + for keeping contradicting rules pass revoke_grant=False. + + Parameters + ---------- + permission: str + The permission to be denied. + username : str, optional + The username. Exactly one is required, either the `username` or the + `role`. + realm: str, optional + The user's realm. Required when username is not None. + role: str, optional + The role (as in Role-Based Access Control). Exactly one is + required, either the `username` or the `role`. + priority: bool, default False + Whether this permission is denied with priority over non-priority + rules. + revoke_grant: bool, default True + Whether a contradicting grant (with same priority flag) in this + ACL will be revoked. + """ + # @review Florian Spreckelsen 2022-03-17 self.acl.deny(realm=realm, username=username, role=role, - permission=permission, priority=priority) + permission=permission, priority=priority, + revoke_grant=revoke_grant) def revoke_denial(self, realm=None, username=None, role=None, permission=None, priority=False): @@ -3636,13 +3703,15 @@ class ACI(): self.permission = permission def __hash__(self): - return hash(str(self.realm) + ":" + str(self.username) + - ":" + str(self.role) + ":" + str(self.permission)) + return hash(self.__repr__()) def __eq__(self, other): return isinstance(other, ACI) and (self.role is None and self.username == other.username and self.realm == other.realm) or self.role == other.role and self.permission == other.permission + def __repr__(self): + return str(self.realm) + ":" + str(self.username) + ":" + str(self.role) + ":" + str(self.permission) + def add_to_element(self, e): if self.role is not None: e.set("role", self.role) @@ -3667,10 +3736,35 @@ class ACL(): self.clear() def parse_xml(self, xml): + """Clear this ACL and parse the xml. + + Iterate over the rules in the xml and add each rule to this ACL. + + Contradicting rules will both be kept. + + Parameters + ---------- + xml : lxml.etree.Element + The xml element containing the ACL rules, i.e. <Grant> and <Deny> + rules. + """ self.clear() self._parse_xml(xml) def _parse_xml(self, xml): + """Parse the xml. + + Iterate over the rules in the xml and add each rule to this ACL. + + Contradicting rules will both be kept. + + Parameters + ---------- + xml : lxml.etree.Element + The xml element containing the ACL rules, i.e. <Grant> and <Deny> + rules. + """ + # @review Florian Spreckelsen 2022-03-17 for e in xml: role = e.get("role") username = e.get("username") @@ -3683,10 +3777,12 @@ class ACL(): if e.tag == "Grant": self.grant(username=username, realm=realm, role=role, - permission=permission, priority=priority) + permission=permission, priority=priority, + revoke_denial=False) elif e.tag == "Deny": self.deny(username=username, realm=realm, role=role, - permission=permission, priority=priority) + permission=permission, priority=priority, + revoke_grant=False) def combine(self, other): """ Combine and return new instance.""" @@ -3764,12 +3860,42 @@ class ACL(): if item in self._denials: self._denials.remove(item) - def grant(self, username=None, realm=None, role=None, - permission=None, priority=False): + def grant(self, permission, username=None, realm=None, role=None, + priority=False, revoke_denial=True): + """Grant a permission to a user or role. + + You must specify either only the username and the realm, or only the + role. + + By default a previously existing denial rule would be revoked, because + otherwise this grant wouldn't have any effect. However, for keeping + contradicting rules pass revoke_denial=False. + + Parameters + ---------- + permission: str + The permission to be granted. + username : str, optional + The username. Exactly one is required, either the `username` or the + `role`. + realm: str, optional + The user's realm. Required when username is not None. + role: str, optional + The role (as in Role-Based Access Control). Exactly one is + required, either the `username` or the `role`. + priority: bool, default False + Whether this permission is granted with priority over non-priority + rules. + revoke_denial: bool, default True + Whether a contradicting denial (with same priority flag) in this + ACL will be revoked. + """ + # @review Florian Spreckelsen 2022-03-17 priority = self._get_boolean_priority(priority) item = ACI(role=role, username=username, realm=realm, permission=permission) - self._remove_item(item, priority) + if revoke_denial: + self._remove_item(item, priority) if priority is True: self._priority_grants.add(item) @@ -3777,11 +3903,41 @@ class ACL(): self._grants.add(item) def deny(self, username=None, realm=None, role=None, - permission=None, priority=False): + permission=None, priority=False, revoke_grant=True): + """Deny a permission to a user or role for this entity. + + You must specify either only the username and the realm, or only the + role. + + By default a previously existing grant rule would be revoked, because + otherwise this denial would override the grant rules anyways. However, + for keeping contradicting rules pass revoke_grant=False. + + Parameters + ---------- + permission: str + The permission to be denied. + username : str, optional + The username. Exactly one is required, either the `username` or the + `role`. + realm: str, optional + The user's realm. Required when username is not None. + role: str, optional + The role (as in Role-Based Access Control). Exactly one is + required, either the `username` or the `role`. + priority: bool, default False + Whether this permission is denied with priority over non-priority + rules. + revoke_grant: bool, default True + Whether a contradicting grant (with same priority flag) in this + ACL will be revoked. + """ + # @review Florian Spreckelsen 2022-03-17 priority = self._get_boolean_priority(priority) item = ACI(role=role, username=username, realm=realm, permission=permission) - self._remove_item(item, priority) + if revoke_grant: + self._remove_item(item, priority) if priority is True: self._priority_denials.add(item) diff --git a/unittests/test_acl.py b/unittests/test_acl.py new file mode 100644 index 0000000000000000000000000000000000000000..633c25ad5c4046c0fa41b66049bdf56aa695f482 --- /dev/null +++ b/unittests/test_acl.py @@ -0,0 +1,55 @@ +# -*- encoding: utf-8 -*- +# +# This file is a part of the CaosDB Project. +# +# Copyright (C) 2022 Indiscale GmbH <info@indiscale.com> +# Copyright (C) 2022 Timm Fitschen <f.fitschen@indiscale.com> +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see <https://www.gnu.org/licenses/>. +# +import caosdb as db +from lxml import etree + + +def test_parse_xml(): + # @review Florian Spreckelsen 2022-03-17 + xml_str = """ + <EntityACL> + <Grant priority="False" role="role1"> + <Permission name="RETRIEVE:ENTITY"/> + </Grant> + <Deny priority="False" role="role1"> + <Permission name="RETRIEVE:ENTITY"/> + </Deny> + <Grant priority="True" role="role1"> + <Permission name="RETRIEVE:ENTITY"/> + </Grant> + <Deny priority="True" role="role1"> + <Permission name="RETRIEVE:ENTITY"/> + </Deny> + </EntityACL>""" + xml = etree.fromstring(xml_str) + left_acl = db.ACL(xml) + + right_acl = db.ACL() + right_acl.grant(role="role1", permission="RETRIEVE:ENTITY", + revoke_denial=False) + right_acl.deny(role="role1", permission="RETRIEVE:ENTITY", + revoke_grant=False) + right_acl.grant(role="role1", permission="RETRIEVE:ENTITY", + priority=True, revoke_denial=False) + right_acl.deny(role="role1", permission="RETRIEVE:ENTITY", + priority=True, revoke_grant=False) + + assert left_acl == right_acl