Fix rawAcl database scope (#1514)

CHANGELOG.md

@@ -17,6 +17,14 @@ Changes are grouped as follows
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
+## [7.2.1] - 2023-11-17
+### Fixed
+- The new compare methods for capabilities in major version 7, `IAMAPI.verify_capabilities` and `IAMAPI.compare_capabilities`
+  now works correctly for rawAcl with database scope ("all tables").
+### Removed
+- Capability scopes no longer have the `is_within` method, and capabilities no longer have `has_capability`. Use the more
+  general `IAMAPI.compare_capabilities` instead.
+
 ## [7.2.0] - 2023-11-16
 ### Added
 - The `trigger` method of the Workflow Execution API, now accepts a `client_credentials` to allow specifying specific
@@ -169,9 +177,9 @@ with no easy way to add a prefix. Also, it no longer expands metadata by default
 ## [6.39.6] - 2023-11-13
 ## Fixed
 - HTTP status code retry strategy for RAW and labels. `/rows/insert` and `/rows/delete` will now
-be retried for all status codes in `config.status_forcelist` (default 429, 502, 503, 504), while
-`/dbs/{db}` and `/tables/{table}` will now only be retried for 429s and connection errors as those
-endpoints are not idempotent.
+  be retried for all status codes in `config.status_forcelist` (default 429, 502, 503, 504), while
+  `/dbs/{db}` and `/tables/{table}` will now only be retried for 429s and connection errors as those
+  endpoints are not idempotent.
 - Also, `labels/list` will now also be retried.
 
 ## [6.39.5] - 2023-11-12

cognite/client/_api/iam.py

@@ -22,9 +22,11 @@
     SessionList,
 )
 from cognite.client.data_classes.capabilities import (
+    AllScope,
     Capability,
     ProjectCapability,
     ProjectCapabilityList,
+    RawAcl,
 )
 from cognite.client.data_classes.iam import TokenInspection
 from cognite.client.utils._identifier import IdentifierSequence
@@ -46,8 +48,6 @@
 
 
 def _convert_capability_to_tuples(capabilities: ComparableCapability, project: str | None = None) -> set[tuple]:
-    from cognite.client.data_classes import Group, GroupList
-
     if isinstance(capabilities, ProjectCapability):
         return ProjectCapabilityList([capabilities]).as_tuples(project)
     if isinstance(capabilities, ProjectCapabilityList):
@@ -59,7 +59,6 @@ def _convert_capability_to_tuples(capabilities: ComparableCapability, project: s
         capabilities = capabilities.capabilities or []
     elif isinstance(capabilities, GroupList):
         capabilities = [cap for grp in capabilities for cap in grp.capabilities or []]
-
     if isinstance(capabilities, Sequence):
         tpls: set[tuple] = set()
         for cap in capabilities:
@@ -100,8 +99,10 @@ def compare_capabilities(
             project (str | None): If a ProjectCapability or ProjectCapabilityList is passed, we need to know which CDF project
                 to pull capabilities from (existing might be from several). If project is not passed, and ProjectCapabilityList
                 is used, it will be inferred from the CogniteClient used to call retrieve it via token/inspect.
-            ignore_allscope_meaning (bool): Option on how to treat allScopes. When True, this function will return
-                e.g. an Acl scoped to a dataset even if the user have the same Acl scoped to all. Defaults to False.
+            ignore_allscope_meaning (bool): Option on how to treat scopes that encompass other scopes, like allScope. When True,
+                this function will return e.g. an Acl scoped to a dataset even if the user have the same Acl scoped to all. The
+                same logic applies to RawAcl scoped to a specific database->table, even when the user have access to all tables
+                in that database. Defaults to False.
 
         Returns:
             list[Capability]: A flattened list of the missing capabilities, meaning they each have exactly 1 action, 1 scope, 1 id etc.
@@ -147,18 +148,30 @@ def compare_capabilities(
         to_check = _convert_capability_to_tuples(desired_capabilities, project)
         missing = to_check - has_capabilties
 
-        if ignore_allscope_meaning:
-            return [Capability.from_tuple(tpl) for tpl in missing]
-
         has_capabilties_lookup = {k: set(grp) for k, grp in groupby(sorted(has_capabilties), key=itemgetter(slice(2)))}
         to_check_lookup = {k: set(grp) for k, grp in groupby(sorted(missing), key=itemgetter(slice(2)))}
 
         missing.clear()
+        raw_group, raw_check_grp = set(), set()
         for key, check_grp in to_check_lookup.items():
             group = has_capabilties_lookup.get(key, set())
-            # If allScope exists for capability, we skip the missing:
-            if not any(grp[2] == "all" for grp in group):
+            if any(AllScope._scope_name == grp[2] for grp in group):
+                continue  # If allScope exists for capability, we safely skip ahead
+            elif RawAcl._capability_name == next(iter(check_grp))[0]:
+                raw_group.update(group)
+                raw_check_grp.update(check_grp)
+            else:
                 missing.update(check_grp)
+
+        # Special handling of rawAcl which has a "hidden" database scope between "all" and "tables":
+        raw_to_check = {k: sorted(grp) for k, grp in groupby(sorted(raw_check_grp), key=itemgetter(slice(4)))}
+        raw_has_capabs = {k: sorted(grp) for k, grp in groupby(sorted(raw_group), key=itemgetter(slice(4)))}
+        for key, check_db_grp in raw_to_check.items():
+            if (db_group := raw_has_capabs.get(key)) and not db_group[0][-1]:
+                # [0] because empty string sorts first; [-1] is table; if no table -> db scope -> skip ahead
+                continue
+            missing.update(check_db_grp)
+
         return [Capability.from_tuple(tpl) for tpl in missing]
 
     def verify_capabilities(

cognite/client/_version.py

@@ -1,4 +1,4 @@
 from __future__ import annotations
 
-__version__ = "7.2.0"
+__version__ = "7.2.1"
 __api_subversion__ = "V20220125"

cognite/client/data_classes/capabilities.py

@@ -98,9 +98,6 @@ def as_tuples(self) -> set[tuple]:
             # Basic implementation for all simple Scopes (e.g. all or currentuser)
             return {(self._scope_name,)}
 
-        def is_within(self, other: Self) -> bool:
-            raise NotImplementedError
-
     @classmethod
     def from_tuple(cls, tpl: tuple) -> Self:
         acl_name, action, scope_name, *scope_params = tpl
@@ -113,9 +110,9 @@ def from_tuple(cls, tpl: tuple) -> Self:
             scope = scope_cls(scope_params)  # type: ignore [call-arg]
         elif len(scope_params) == 2 and scope_cls is TableScope:
             db, tbl = scope_params
-            scope = scope_cls({db: [tbl]})  # type: ignore [call-arg]
+            scope = scope_cls({db: [tbl] if tbl else []})  # type: ignore [call-arg]
         else:
-            raise ValueError(f"tuple not understood ({tpl})")
+            raise ValueError(f"tuple not understood as capability: {tpl}")
 
         return cast(Self, capability_cls(actions=[capability_cls.Action(action)], scope=scope))
 
@@ -162,13 +159,6 @@ def dump(self, camel_case: bool = True) -> dict[str, Any]:
         capability_name = self._capability_name
         return {to_camel_case(capability_name) if camel_case else to_snake_case(capability_name): data}
 
-    def has_capability(self, other: Capability) -> bool:
-        if not isinstance(self, type(other)):
-            return False
-        if not other.scope.is_within(self.scope):
-            return False
-        return not set(other.actions) - set(self.actions)
-
     def as_tuples(self) -> set[tuple]:
         return set(
             (acl, action, *scope_tpl)
@@ -262,17 +252,11 @@ def as_tuples(self, project: str | None = None) -> set[tuple]:
 class AllScope(Capability.Scope):
     _scope_name = "all"
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, AllScope)
-
 
 @dataclass(frozen=True)
 class CurrentUserScope(Capability.Scope):
     _scope_name = "currentuserscope"
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, (AllScope, CurrentUserScope))
-
 
 @dataclass(frozen=True)
 class IDScope(Capability.Scope):
@@ -285,9 +269,6 @@ def __post_init__(self) -> None:
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, i) for i in self.ids}
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, AllScope) or type(self) is type(other) and set(self.ids).issubset(other.ids)
-
 
 @dataclass(frozen=True)
 class IDScopeLowerCase(Capability.Scope):
@@ -302,9 +283,6 @@ def __post_init__(self) -> None:
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, i) for i in self.ids}
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, AllScope) or type(self) is type(other) and set(self.ids).issubset(other.ids)
-
 
 @dataclass(frozen=True)
 class ExtractionPipelineScope(Capability.Scope):
@@ -317,9 +295,6 @@ def __post_init__(self) -> None:
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, i) for i in self.ids}
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, AllScope) or type(self) is type(other) and set(self.ids).issubset(other.ids)
-
 
 @dataclass(frozen=True)
 class DataSetScope(Capability.Scope):
@@ -332,9 +307,6 @@ def __post_init__(self) -> None:
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, i) for i in self.ids}
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, AllScope) or type(self) is type(other) and set(self.ids).issubset(other.ids)
-
 
 @dataclass(frozen=True)
 class TableScope(Capability.Scope):
@@ -357,20 +329,9 @@ def dump(self, camel_case: bool = True) -> dict[str, Any]:
         return {self._scope_name: {key: {k: {"tables": v} for k, v in self.dbs_to_tables.items()}}}
 
     def as_tuples(self) -> set[tuple]:
-        return {(self._scope_name, db, tbl) for db, tables in self.dbs_to_tables.items() for tbl in tables}
-
-    def is_within(self, other: Self) -> bool:
-        if isinstance(other, AllScope):
-            return True
-        if not isinstance(other, TableScope):
-            return False
-
-        for db_name, tables in self.dbs_to_tables.items():
-            if (other_tables := other.dbs_to_tables.get(db_name)) is None:
-                return False
-            if not set(tables).issubset(other_tables):
-                return False
-        return True
+        # When the scope contains no tables, it means all tables... since database name must be at least 1
+        # character, we represent this internally with the empty string:
+        return {(self._scope_name, db, tbl) for db, tables in self.dbs_to_tables.items() for tbl in tables or [""]}
 
 
 @dataclass(frozen=True)
@@ -384,9 +345,6 @@ def __post_init__(self) -> None:
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, i) for i in self.root_ids}
 
-    def is_within(self, other: Self) -> bool:
-        return isinstance(other, AllScope) or type(self) is type(other) and set(self.root_ids).issubset(other.root_ids)
-
 
 @dataclass(frozen=True)
 class ExperimentsScope(Capability.Scope):
@@ -396,13 +354,6 @@ class ExperimentsScope(Capability.Scope):
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, s) for s in self.experiments}
 
-    def is_within(self, other: Self) -> bool:
-        return (
-            isinstance(other, AllScope)
-            or type(self) is type(other)
-            and set(self.experiments).issubset(other.experiments)
-        )
-
 
 @dataclass(frozen=True)
 class SpaceIDScope(Capability.Scope):
@@ -412,11 +363,6 @@ class SpaceIDScope(Capability.Scope):
     def as_tuples(self) -> set[tuple]:
         return {(self._scope_name, s) for s in self.space_ids}
 
-    def is_within(self, other: Self) -> bool:
-        return (
-            isinstance(other, AllScope) or type(self) is type(other) and set(self.space_ids).issubset(other.space_ids)
-        )
-
 
 @dataclass(frozen=True)
 class UnknownScope(Capability.Scope):
@@ -434,9 +380,6 @@ def __getitem__(self, item: str) -> Any:
     def as_tuples(self) -> set[tuple]:
         raise NotImplementedError("Unknown scope cannot be converted to tuples (needed for comparisons)")
 
-    def is_within(self, other: Self) -> bool:
-        raise NotImplementedError("Unknown scope cannot be compared")
-
 
 _SCOPE_CLASS_BY_NAME: MappingProxyType[str, type[Capability.Scope]] = MappingProxyType(
     {c._scope_name: c for c in Capability.Scope.__subclasses__() if not issubclass(c, UnknownScope)}

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "cognite-sdk"
 
-version = "7.2.0"
+version = "7.2.1"
 description = "Cognite Python SDK"
 readme = "README.md"
 documentation = "https://cognite-sdk-python.readthedocs-hosted.com"

tests/tests_unit/test_data_classes/test_capabilities.py

@@ -19,6 +19,7 @@
     ProjectCapabilityList,
     ProjectsAcl,
     RawAcl,
+    TableScope,
     UnknownAcl,
     UnknownScope,
 )
@@ -301,6 +302,8 @@ def test_project_scope_is_all_projects(self, proj_cap_allprojects_dct):
         loaded = ProjectCapabilityList.load([proj_cap_allprojects_dct])
         assert type(loaded[0].project_scope) is AllProjectsScope
 
+
+class TestIAMCompareCapabilities:
     @pytest.mark.parametrize(
         "capability",
         [
@@ -347,6 +350,40 @@ def test_partly_missing_capabilities(
         )
         assert missing_acls == [has_not]
 
+    def test_raw_acl_database_scope_only(self, cognite_client):
+        # Would fail with: 'ValueError: No capabilities given' prior to 7.2.1 due to a bug in 'as_tuples'.
+        has_all_scope = RawAcl([RawAcl.Action.Read], AllScope)
+        has_db_scope = RawAcl([RawAcl.Action.Read], TableScope(dbs_to_tables={"db1": []}))
+        assert not cognite_client.iam.compare_capabilities(has_all_scope, has_db_scope)
+
+    @pytest.mark.parametrize(
+        "extra_existing",
+        [
+            [],
+            [RawAcl(actions=[RawAcl.Action.Read], scope=AllScope)],
+            [RawAcl(actions=[RawAcl.Action.Read], scope=TableScope({"db1": []}))],
+            [RawAcl(actions=[RawAcl.Action.Read], scope=TableScope({"db1": {"tables": []}}))],
+        ],
+    )
+    def test_raw_acl_database_scope(self, cognite_client, extra_existing):
+        existing = [
+            RawAcl([RawAcl.Action.Read], RawAcl.Scope.Table({"db1": ["t1"]})),
+            RawAcl([RawAcl.Action.Read], RawAcl.Scope.Table({"db1": ["t1", "t2"]})),
+            RawAcl([RawAcl.Action.Read], RawAcl.Scope.Table({"db2": ["t1", "t2"]})),
+            *extra_existing,
+        ]
+        desired = [
+            RawAcl([RawAcl.Action.Read], RawAcl.Scope.Table({"db1": ["t1", "t2", "t3"]})),
+            RawAcl([RawAcl.Action.Write], RawAcl.Scope.Table({"db1": ["t1"]})),
+        ]
+        missing = cognite_client.iam.compare_capabilities(existing, desired)
+        if extra_existing:
+            assert missing == [RawAcl([RawAcl.Action.Write], RawAcl.Scope.Table({"db1": ["t1"]}))]
+        else:
+            assert len(missing) == 2
+            assert RawAcl([RawAcl.Action.Read], RawAcl.Scope.Table({"db1": ["t3"]})) in missing
+            assert RawAcl([RawAcl.Action.Write], RawAcl.Scope.Table({"db1": ["t1"]})) in missing
+
 
 @pytest.mark.parametrize(
     "dct",