Fix nonce credentials in TransformationsAPI (+minor fixes) (#1346)

CHANGELOG.md

@@ -17,10 +17,22 @@ Changes are grouped as follows
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
+## [6.25.1] - 2023-09-15
+### Fixed
+- Using nonce credentials now works as expected for `transformations.[create, update]`. Previously, the attempt to create
+  a session would always fail, leading to nonce credentials never being used (full credentials were passed to- and
+  stored in the transformations backend service).
+- Additionally, the automatic creation of a session no longer fails silently when an `CogniteAuthError` is encountered
+  (which happens when the credentials are invalid).
+- While processing source- and destination credentials in `client.transformations.[create, update]`, an `AttributeError`
+  can no longer be raised (by not specifying project).
+
+### Added
+- `TransformationList` now correctly inherits the two (missing) helper methods `as_ids()` and `as_external_ids()`.
+
 ## [6.25.0] - 2023-09-14
 ### Added
 - Support for `ignore_unknown_ids` in `client.functions.retrieve_multiple` method.
-- 
 ## [6.24.1] - 2023-09-13
 ### Fixed
 - Bugfix for `AssetsAPI.create_hierarchy` when running in upsert mode: It could skip certain updates above
@@ -51,7 +63,7 @@ data modeling query and receive updates through a provided callback.
 ### Added
 - Supporting pattern mode and extra configuration for diagram detect in beta.
 
-## [6.20.0] - 2023-09-05
+## [6.20.0] - 2023-09-06
 ### Fixed
 - When creating functions with `client.functions.create` using the `folder` argument, a trial-import is executed as part of
   the verification process. This could leave leftover modules still in scope, possibly affecting subsequent calls. This is

cognite/client/_version.py

@@ -1,4 +1,4 @@
 from __future__ import annotations
 
-__version__ = "6.25.0"
+__version__ = "6.25.1"
 __api_subversion__ = "V20220125"

cognite/client/data_classes/transformations/__init__.py

@@ -1,7 +1,9 @@
 from __future__ import annotations
 
+import logging
 from abc import abstractmethod
-from typing import TYPE_CHECKING, Any, Awaitable, Dict, cast
+from copy import deepcopy
+from typing import TYPE_CHECKING, Any, Awaitable, Dict, Literal, cast
 
 from cognite.client.data_classes._base import (
     CogniteFilter,
@@ -10,9 +12,9 @@
     CogniteResource,
     CogniteResourceList,
     CogniteUpdate,
+    IdTransformerMixin,
     PropertySpec,
 )
-from cognite.client.data_classes.iam import ClientCredentials
 from cognite.client.data_classes.shared import TimestampRange
 from cognite.client.data_classes.transformations.common import (
     NonceCredentials,
@@ -24,12 +26,16 @@
 from cognite.client.data_classes.transformations.jobs import TransformationJob, TransformationJobList
 from cognite.client.data_classes.transformations.schedules import TransformationSchedule
 from cognite.client.data_classes.transformations.schema import TransformationSchemaColumnList
+from cognite.client.exceptions import CogniteAPIError
 from cognite.client.utils._text import convert_all_keys_to_camel_case, convert_all_keys_to_snake_case
 
 if TYPE_CHECKING:
     from cognite.client import CogniteClient
 
 
+logger = logging.getLogger(__name__)
+
+
 class SessionDetails:
     """Details of a source session.
 
@@ -181,7 +187,7 @@ def copy(self) -> Transformation:
             self.blocked,
             self.schedule,
             self.data_set_id,
-            None,
+            None,  # skip cognite client
             self.source_nonce,
             self.destination_nonce,
             self.source_session,
@@ -195,45 +201,70 @@ def _process_credentials(
         if sessions_cache is None:
             sessions_cache = {}
 
-        def try_get_or_create_nonce(oidc_credentials: OidcCredentials | None, project: str) -> NonceCredentials | None:
-            if keep_none and oidc_credentials is None:
-                return None
-
-            # MyPy requires this to make sure it's not changed to None after inner declaration
-            assert sessions_cache is not None
-
-            key = (
-                f"{oidc_credentials.client_id}:{hash(oidc_credentials.client_secret)}:{project}"
-                if oidc_credentials
-                else "DEFAULT"
-            )
-
-            ret = sessions_cache.get(key)
-            if not ret:
-                if oidc_credentials and oidc_credentials.client_id and oidc_credentials.client_secret:
-                    credentials = ClientCredentials(oidc_credentials.client_id, oidc_credentials.client_secret)
-                else:
-                    credentials = None
-                try:
-                    session = self._cognite_client.iam.sessions.create(credentials)
-                    ret = NonceCredentials(session.id, session.nonce, project)
-                    sessions_cache[key] = ret
-                except Exception:
-                    ret = None
-            return ret
-
         if self.source_nonce is None and self.source_oidc_credentials:
-            project = self.source_oidc_credentials.cdf_project_name or self._cognite_client.config.project
-            self.source_nonce = try_get_or_create_nonce(self.source_oidc_credentials, project)
+            self.source_nonce = self._try_get_or_create_nonce(
+                self.source_oidc_credentials,
+                sessions_cache,
+                keep_none,
+                credentials_name="source",
+            )
             if self.source_nonce:
                 self.source_oidc_credentials = None
 
         if self.destination_nonce is None and self.destination_oidc_credentials:
-            project = self.destination_oidc_credentials.cdf_project_name or self._cognite_client.config.project
-            self.destination_nonce = try_get_or_create_nonce(self.destination_oidc_credentials, project)
+            self.destination_nonce = self._try_get_or_create_nonce(
+                self.destination_oidc_credentials,
+                sessions_cache,
+                keep_none,
+                credentials_name="destination",
+            )
             if self.destination_nonce:
                 self.destination_oidc_credentials = None
 
+    def _try_get_or_create_nonce(
+        self,
+        oidc_credentials: OidcCredentials,
+        sessions_cache: dict[str, NonceCredentials],
+        keep_none: bool,
+        credentials_name: Literal["source", "destination"],
+    ) -> NonceCredentials | None:
+        if keep_none and oidc_credentials is None:
+            return None
+
+        project = oidc_credentials.cdf_project_name or self._cognite_client.config.project
+
+        key = "DEFAULT"
+        if oidc_credentials:
+            key = f"{oidc_credentials.client_id}:{hash(oidc_credentials.client_secret)}:{project}"
+
+        if (ret := sessions_cache.get(key)) is None:
+            credentials = None
+            if oidc_credentials and oidc_credentials.client_id and oidc_credentials.client_secret:
+                credentials = oidc_credentials.as_client_credentials()
+
+            # We want to create a session using the supplied 'oidc_credentials' (either 'source_oidc_credentials'
+            # or 'destination_oidc_credentials') and send the nonce to the Transformations backend, to avoid sending
+            # (and it having to store) the full set of client credentials. However, there is no easy way to do this
+            # without instantiating a new 'CogniteClient' with the given credentials:
+            from cognite.client import CogniteClient
+
+            config = deepcopy(self._cognite_client.config)
+            config.project = project
+            config.credentials = oidc_credentials.as_credential_provider()
+            other_client = CogniteClient(config)
+            try:
+                session = other_client.iam.sessions.create(credentials)
+                ret = sessions_cache[key] = NonceCredentials(cast(int, session.id), cast(str, session.nonce), project)
+            except CogniteAPIError as err:
+                # This is fine, we might be missing SessionsACL. The OIDC credentials will then be passed
+                # directly to the backend service. We do however not catch CogniteAuthError as that would
+                # mean the credentials are invalid.
+                logger.debug(
+                    f"Unable to create a session and get a nonce towards {project=} using the provided "
+                    f"{credentials_name} credentials: {err!r}"
+                )
+        return ret
+
     def run(self, wait: bool = True, timeout: float | None = None) -> TransformationJob:
         return self._cognite_client.transformations.run(transformation_id=self.id, wait=wait, timeout=timeout)
 
@@ -413,7 +444,7 @@ def _get_update_properties(cls) -> list[PropertySpec]:
         ]
 
 
-class TransformationList(CogniteResourceList):
+class TransformationList(CogniteResourceList[Transformation], IdTransformerMixin):
     _RESOURCE = Transformation
 
 
@@ -490,12 +521,11 @@ def __init__(
 
     def dump(self, camel_case: bool = True) -> dict[str, Any]:
         obj = super().dump(camel_case=camel_case)
-        if obj.get("includePublic") is not None:
-            is_public = obj.pop("includePublic")
-            obj["isPublic"] = is_public
-        if obj.get("tags"):
-            tags = obj.pop("tags")
-            obj["tags"] = tags.dump(camel_case)
+        if (value := obj.pop("includePublic" if camel_case else "include_public", None)) is not None:
+            obj["isPublic" if camel_case else "is_public"] = value
+
+        if (value := obj.pop("tags", None)) is not None:
+            obj["tags"] = value.dump(camel_case)
         return obj
 
 

cognite/client/data_classes/transformations/common.py

@@ -2,6 +2,8 @@
 
 from typing import Any
 
+from cognite.client.credentials import OAuthClientCredentials
+from cognite.client.data_classes.iam import ClientCredentials
 from cognite.client.utils._auxiliary import basic_obj_dump
 from cognite.client.utils._text import convert_all_keys_to_snake_case, iterable_to_case
 
@@ -276,10 +278,10 @@ def _load(cls, resource: dict[str, Any]) -> Instances:
 class OidcCredentials:
     def __init__(
         self,
-        client_id: str | None = None,
-        client_secret: str | None = None,
-        scopes: str | None = None,
-        token_uri: str | None = None,
+        client_id: str,
+        client_secret: str,
+        scopes: str,
+        token_uri: str,
         audience: str | None = None,
         cdf_project_name: str | None = None,
     ) -> None:
@@ -290,6 +292,18 @@ def __init__(
         self.audience = audience
         self.cdf_project_name = cdf_project_name
 
+    def as_credential_provider(self) -> OAuthClientCredentials:
+        return OAuthClientCredentials(
+            token_url=self.token_uri,
+            client_id=self.client_id,
+            client_secret=self.client_secret,
+            scopes=self.scopes.split(","),
+            audience=self.audience,
+        )
+
+    def as_client_credentials(self) -> ClientCredentials:
+        return ClientCredentials(client_id=self.client_id, client_secret=self.client_secret)
+
     def dump(self, camel_case: bool = False) -> dict[str, Any]:
         """Dump the instance into a json serializable Python data type.
 

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "cognite-sdk"
 
-version = "6.25.0"
+version = "6.25.1"
 
 description = "Cognite Python SDK"
 readme = "README.md"

tests/tests_integration/test_api/test_transformations/test_transformations.py

@@ -23,10 +23,19 @@
     SequenceRows,
     ViewInfo,
 )
+from cognite.client.exceptions import CogniteAPIError
 from cognite.client.utils._text import random_string
+from cognite.client.utils._time import timestamp_to_ms
 
 
-@pytest.fixture
+@pytest.fixture(scope="session")
+def transform_cleanup(cognite_client):
+    transforms = cognite_client.transformations.list(created_time={"max": timestamp_to_ms("1d-ago")}, limit=None)
+    if transforms:
+        cognite_client.transformations.delete(id=transforms.as_ids())
+
+
+@pytest.fixture(scope="module")
 def new_datasets(cognite_client):
     ds_ext_id1 = "transformation-ds"
     ds_ext_id2 = "transformation-ds2"
@@ -42,11 +51,13 @@ def new_datasets(cognite_client):
 
 
 @pytest.fixture
-def new_transformation(cognite_client, new_datasets):
+def new_transformation(cognite_client, new_datasets, transform_cleanup):
     prefix = random_string(6, string.ascii_letters)
     creds = cognite_client.config.credentials
     if not isinstance(creds, (OAuthClientCredentials, OAuthClientCertificate)):
         pytest.skip("Only run in CI environment")
+    # TODO: Data Integration team, add:
+    pytest.skip("Need valid credentials for: 'source_oidc_credentials' and 'destination_oidc_credentials'...")
     transform = Transformation(
         name="any",
         query="select 1",
@@ -81,18 +92,14 @@ def new_transformation(cognite_client, new_datasets):
 
 class TestTransformationsAPI:
     def test_create_transformation_error(self, cognite_client):
-        prefix = random_string(6, string.ascii_letters)
-        transform_without_name = Transformation(
-            external_id=f"{prefix}-transformation", destination=TransformationDestination.assets()
-        )
-        try:
-            ts = cognite_client.transformations.create(transform_without_name)
-            failed = False
-            cognite_client.transformations.delete(id=ts.id)
-        except Exception as ex:
-            failed = True
-            str(ex)
-        assert failed
+        xid = f"{random_string(6, string.ascii_letters)}-transformation"
+        transform_without_name = Transformation(external_id=xid, destination=TransformationDestination.assets())
+
+        with pytest.raises(CogniteAPIError, match="^Invalid value for: body") as exc:
+            cognite_client.transformations.create(transform_without_name)
+
+        assert exc.value.code == 400
+        assert exc.value.failed == [transform_without_name]
 
     def test_create_asset_transformation(self, cognite_client):
         prefix = random_string(6, string.ascii_letters)
@@ -102,7 +109,7 @@ def test_create_asset_transformation(self, cognite_client):
         ts = cognite_client.transformations.create(transform)
         cognite_client.transformations.delete(id=ts.id)
 
-    @pytest.mark.skip(reason="Awaiting access to an additional CDF project")
+    @pytest.mark.skip(reason="Awaiting access to more than one CDF project for our credentials")
     def test_create_asset_with_source_destination_oidc_transformation(self, cognite_client):
         prefix = random_string(6, string.ascii_letters)
         transform = Transformation(
@@ -539,9 +546,14 @@ def test_filter_transformations_by_tags(self, cognite_client, new_transformation
         other_transformation.tags = ["hi", "kiki"]
         cognite_client.transformations.update([new_transformation, other_transformation])
         ts = cognite_client.transformations.list(tags=ContainsAny(["hello"]))
-        assert ts[0].id == new_transformation.id and ts[0].tags == ["hello"]
+
+        new_ts = ts.get(id=new_transformation.id)
+        assert new_ts is not None
+        assert new_ts.tags == ["hello"]
+
         ts3 = cognite_client.transformations.list(tags=ContainsAny(["hello", "kiki"]))
-        assert len(ts3) == 2 and {i.id for i in ts3} == {new_transformation.id, other_transformation.id}
+        assert len(ts3) == 2
+        assert {i.id for i in ts3} == {new_transformation.id, other_transformation.id}
 
     def test_transformation_dump_and_str(self, cognite_client, new_transformation, new_datasets):
         cognite_client.transformations.schedules.create(