Apply review feedback

Author: BaptisteCentreon

docs/admin/guides/auth/workload_identity.md

@@ -26,20 +26,25 @@ A token that matches no rule is rejected with a 401.
 
 ## Enabling
 
-Add the authentication class to `DEFAULT_AUTHENTICATION_CLASSES`,
-then populate `WORKLOAD_IDENTITY`:
+Add the authentication class to `DEFAULT_AUTHENTICATION_CLASSES`, add the backend to
+`AUTHENTICATION_BACKENDS`, then populate `WORKLOAD_IDENTITY`:
 
 ```python title="settings.py"
 REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"] = [
     "pulpcore.app.workload_identity.authentication.WorkloadIdentityAuthentication",
     "pulpcore.app.authentication.BasicAuthentication",
     "rest_framework.authentication.SessionAuthentication",
 ]
+
+AUTHENTICATION_BACKENDS = [
+    "django.contrib.auth.backends.ModelBackend",
+    "pulpcore.backends.ObjectRolePermissionBackend",
+    "pulpcore.app.workload_identity.backend.WorkloadIdentityBackend",
+]
 ```
 
-No change to `AUTHENTICATION_BACKENDS` is needed.
-The feature stays off while `WORKLOAD_IDENTITY` is empty,
-so adding the class alone changes nothing.
+The backend answers the permission checks for a workload identity request, so it must be present
+for the feature to grant anything. Nothing is active until all three pieces are configured.
 
 With the example below,
 a push from the `main` branch of `my-org/app` is granted the `file.filerepository_owner` role on the repository named `prod`,

pulpcore/app/access_policy.py

@@ -1,6 +1,7 @@
 from copy import deepcopy
 
 from django.conf import settings
+from django.contrib.auth import get_user_model
 from rest_access_policy import AccessPolicy
 from rest_framework.exceptions import APIException
 
@@ -22,8 +23,6 @@ def get_user_group_values(self, user):
         which does not work for a stateless principal. Any non-database user (a workload-identity
         principal, or another one added later) supplies its groups via a `group_names` attribute.
         """
-        from django.contrib.auth import get_user_model
-
         if isinstance(user, get_user_model()):
             return super().get_user_group_values(user)
         return list(getattr(user, "group_names", []))

pulpcore/app/checks.py

@@ -125,8 +125,14 @@ def check_artifact_checksums(app_configs, **kwargs):
     return messages
 
 
+_WORKLOAD_IDENTITY_BACKEND = "pulpcore.app.workload_identity.backend.WorkloadIdentityBackend"
+
+
 @register(deploy=True)
 def workload_identity_reserved_username(app_configs, **kwargs):
+    if _WORKLOAD_IDENTITY_BACKEND not in settings.AUTHENTICATION_BACKENDS:
+        return []
+
     from pulpcore.app.workload_identity import config
 
     messages = []
@@ -157,6 +163,9 @@ def workload_identity_reserved_username(app_configs, **kwargs):
 
 @register(deploy=True)
 def workload_identity_domain_scopes(app_configs, **kwargs):
+    if _WORKLOAD_IDENTITY_BACKEND not in settings.AUTHENTICATION_BACKENDS:
+        return []
+
     from pulpcore.app.workload_identity import config
 
     messages = []
@@ -183,6 +192,9 @@ def workload_identity_domain_scopes(app_configs, **kwargs):
 
 @register(deploy=True)
 def workload_identity_unqualified_name_scopes(app_configs, **kwargs):
+    if _WORKLOAD_IDENTITY_BACKEND not in settings.AUTHENTICATION_BACKENDS:
+        return []
+
     from pulpcore.app.workload_identity import config
 
     messages = []

pulpcore/app/role_util.py

@@ -173,23 +173,27 @@ def get_objects_for_user(
     accept_domain_perms=True,
     accept_global_perms=True,
 ):
-    from pulpcore.app.workload_identity.principal import WorkloadIdentityPrincipal
+    if (
+        "pulpcore.app.workload_identity.backend.WorkloadIdentityBackend"
+        in settings.AUTHENTICATION_BACKENDS
+    ):
+        from pulpcore.app.workload_identity.principal import WorkloadIdentityPrincipal
 
-    if isinstance(user, WorkloadIdentityPrincipal):
-        from pulpcore.app.workload_identity.authz import grants_queryset
+        if isinstance(user, WorkloadIdentityPrincipal):
+            from pulpcore.app.workload_identity.authz import grants_queryset
 
-        grants = user.grants
-        if isinstance(perms, str):
-            return grants_queryset(grants, perms, qs)
-        if any_perm:
-            result = qs.none()
+            grants = user.grants
+            if isinstance(perms, str):
+                return grants_queryset(grants, perms, qs)
+            if any_perm:
+                result = qs.none()
+                for permission_name in perms:
+                    result |= grants_queryset(grants, permission_name, qs)
+                return result
+            result = qs.all()
             for permission_name in perms:
-                result |= grants_queryset(grants, permission_name, qs)
+                result &= grants_queryset(grants, permission_name, qs)
             return result
-        result = qs.all()
-        for permission_name in perms:
-            result &= grants_queryset(grants, permission_name, qs)
-        return result
 
     new_qs = qs.none()
     replace = False

pulpcore/app/settings.py

@@ -155,7 +155,6 @@
 AUTHENTICATION_BACKENDS = [
     "django.contrib.auth.backends.ModelBackend",
     "pulpcore.backends.ObjectRolePermissionBackend",
-    "pulpcore.app.workload_identity.backend.WorkloadIdentityBackend",
 ]
 
 ROOT_URLCONF = "pulpcore.app.urls"

pulpcore/tests/unit/workload_identity/test_workload_identity.py

@@ -32,6 +32,19 @@
 from pulpcore.app.workload_identity.principal import WorkloadIdentityPrincipal
 from pulpcore.app.workload_identity.rules import grants_for
 
+WI_BACKENDS = [
+    "django.contrib.auth.backends.ModelBackend",
+    "pulpcore.backends.ObjectRolePermissionBackend",
+    "pulpcore.app.workload_identity.backend.WorkloadIdentityBackend",
+]
+
+
+@pytest.fixture(autouse=True)
+def _wi_backend_enabled(settings):
+    # The backend is an opt-in addon, so enable it for the tests that exercise it.
+    settings.AUTHENTICATION_BACKENDS = WI_BACKENDS
+
+
 PROVIDER = {
     "issuer": "https://issuer",
     "rules": [
@@ -622,3 +635,22 @@ def test_authenticate_not_a_jwt_returns_none():
 @override_settings(WORKLOAD_IDENTITY=WI_AUTH)
 def test_authenticate_no_token_returns_none():
     assert WorkloadIdentityAuthentication().authenticate(_FakeRequest("")) is None
+
+
+# --- checks are silent when the backend is not configured ---
+
+
+@override_settings(
+    AUTHENTICATION_BACKENDS=["django.contrib.auth.backends.ModelBackend"],
+    DOMAIN_ENABLED=True,
+    WORKLOAD_IDENTITY={
+        "basic_auth_username": "ci-bot",
+        "providers": {
+            "p": {"rules": [{"grants": [{"role": "r", "scope": {"type": "object", "name": "x"}}]}]}
+        },
+    },
+)
+def test_checks_silent_when_backend_not_configured():
+    assert workload_identity_reserved_username(None) == []
+    assert workload_identity_domain_scopes(None) == []
+    assert workload_identity_unqualified_name_scopes(None) == []

pyproject.toml

@@ -31,7 +31,7 @@ dependencies = [
   "backoff>=2.1.2,<2.3",  # Looks like only bugfixes in z-Stream.
   "click>=8.1.0,<8.5",  # Uses milestone.feature.fix https://palletsprojects.com/versions .
   "cryptography>=49.0.0,<51.0",  # SemVer compatible https://cryptography.io/en/latest/api-stability/#versioning .
-  "pyjwt>=2.4,<3",  # >=2.4 fixes CVE-2022-29217, capped below the next major. https://pyjwt.readthedocs.io/en/stable/changelog.html
+  "pyjwt>=2.4,<3",  # SemVer compatible https://pyjwt.readthedocs.io/en/stable/changelog.html .
   "Django>=5.2.0,<5.3",  # LTS version, we aim at supporting one or two at a time.
   "django-filter>=24.3,<=26.1",  # Uses CalVer, not released often https://github.com/carltongibson/django-filter
   "django-guid>=3.4.0,<3.7",  # Looks like only bugfixes in z-Stream.