DM-48088: Allow an empty list of required scopes

changelog.d/20241211_105613_rra_DM_47986.md

@@ -0,0 +1,3 @@
+### New features
+
+- Allow an authenticated `GafaelfawrIngress` with no required scopes. This is useful for an `onlyService` case where the token may have any scope but must be delegated to one of the listed services.

src/gafaelfawr/handlers/ingress.py

@@ -188,7 +188,7 @@ def auth_config(
         ),
     ] = Satisfy.ALL,
     scope: Annotated[
-        list[str],
+        list[str] | None,
         Query(
             title="Required scopes",
             description=(
@@ -197,7 +197,7 @@ def auth_config(
             ),
             examples=["read:all"],
         ),
-    ],
+    ] = None,
     service: Annotated[
         str | None,
         Query(
@@ -252,7 +252,7 @@ def auth_config(
     if service and delegate_to and service != delegate_to:
         msg = "service must be the same as delegate_to"
         raise InvalidServiceError(msg)
-    scopes = set(scope)
+    scopes = set(scope) if scope else set()
     context.rebind_logger(
         auth_uri=auth_uri,
         required_scopes=sorted(scopes),
@@ -322,12 +322,13 @@ async def get_auth(
     response: Response,
 ) -> dict[str, str]:
     check_lifetime(context, auth_config, token_data)
+    token_scopes = set(token_data.scopes)
 
     # Determine whether the request is authorized.
     if auth_config.satisfy == Satisfy.ANY:
-        authorized = any(s in token_data.scopes for s in auth_config.scopes)
+        authorized = not token_scopes.isdisjoint(auth_config.scopes)
     else:
-        authorized = all(s in token_data.scopes for s in auth_config.scopes)
+        authorized = token_scopes.issuperset(auth_config.scopes)
     if not authorized:
         raise generate_challenge(
             context,

tests/data/kubernetes/input/ingresses.yaml

@@ -124,7 +124,7 @@ template:
       - host: foo.example.com
         http:
           paths:
-            - path: /foo
+            - path: /foo/bar
               pathType: Prefix
               backend:
                 service:
@@ -151,7 +151,7 @@ template:
       - host: foo.example.com
         http:
           paths:
-            - path: /foo
+            - path: /foo/baz
               pathType: Prefix
               backend:
                 service:
@@ -177,7 +177,7 @@ template:
       - host: foo.example.com
         http:
           paths:
-            - path: /foo
+            - path: /username
               pathType: Prefix
               backend:
                 service:
@@ -205,7 +205,34 @@ template:
       - host: foo.example.com
         http:
           paths:
-            - path: /foo
+            - path: /service
+              pathType: Prefix
+              backend:
+                service:
+                  name: something
+                  port:
+                    name: http
+---
+apiVersion: gafaelfawr.lsst.io/v1alpha1
+kind: GafaelfawrIngress
+metadata:
+  name: service-any-ingress
+  namespace: {namespace}
+config:
+  scopes:
+    all: []
+  onlyServices:
+    - vo-cutouts
+  service: uws
+template:
+  metadata:
+    name: service-any
+  spec:
+    rules:
+      - host: foo.example.com
+        http:
+          paths:
+            - path: /service/any
               pathType: Prefix
               backend:
                 service:

tests/data/kubernetes/output/ingresses.yaml

@@ -159,7 +159,7 @@ spec:
     - host: foo.example.com
       http:
         paths:
-          - path: /foo
+          - path: /foo/bar
             pathType: Prefix
             backend:
               service:
@@ -198,7 +198,7 @@ spec:
     - host: foo.example.com
       http:
         paths:
-          - path: /foo
+          - path: /foo/baz
             pathType: Prefix
             backend:
               service:
@@ -238,7 +238,7 @@ spec:
     - host: foo.example.com
       http:
         paths:
-          - path: /foo
+          - path: /username
             pathType: Prefix
             backend:
               service:
@@ -278,7 +278,47 @@ spec:
     - host: foo.example.com
       http:
         paths:
-          - path: /foo
+          - path: /service
+            pathType: Prefix
+            backend:
+              service:
+                name: something
+                port:
+                  name: http
+status: {any}
+---
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: service-any
+  namespace: {namespace}
+  annotations:
+    nginx.ingress.kubernetes.io/auth-method: GET
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User"
+    nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?only_service=vo-cutouts&service=uws"
+    nginx.ingress.kubernetes.io/configuration-snippet: |
+      {snippet}
+  creationTimestamp: {any}
+  generation: {any}
+  labels:
+    app.kubernetes.io/managed-by: Gafaelfawr
+  managedFields: {any}
+  ownerReferences:
+    - apiVersion: gafaelfawr.lsst.io/v1alpha1
+      blockOwnerDeletion: true
+      controller: true
+      kind: GafaelfawrIngress
+      name: service-any-ingress
+      uid: {any}
+  resourceVersion: {any}
+  uid: {any}
+spec:
+  ingressClassName: nginx
+  rules:
+    - host: foo.example.com
+      http:
+        paths:
+          - path: /service/any
             pathType: Prefix
             backend:
               service:

tests/handlers/ingress_test.py

@@ -60,12 +60,6 @@ async def test_invalid(
 ) -> None:
     token = await create_session_token(factory)
 
-    r = await client.get(
-        "/ingress/auth", headers={"Authorization": f"bearer {token.token}"}
-    )
-    assert r.status_code == 422
-    assert r.json()["detail"][0]["type"] == "missing"
-
     r = await client.get(
         "/ingress/auth",
         headers={"Authorization": f"bearer {token.token}"},
@@ -222,6 +216,14 @@ async def test_success(client: AsyncClient, factory: Factory) -> None:
     assert r.headers["X-Auth-Request-Email"] == token_data.email
     assert "X-Auth-Request-Service" not in r.headers
 
+    # Request with no required scopes is always valid.
+    r = await client.get(
+        "/ingress/auth",
+        headers={"Authorization": f"bearer {token_data.token}"},
+    )
+    assert r.status_code == 200
+    assert r.headers["X-Auth-Request-User"] == token_data.username
+
 
 @pytest.mark.asyncio
 async def test_success_minimal(client: AsyncClient, factory: Factory) -> None:
@@ -1123,6 +1125,16 @@ async def test_only_service(client: AsyncClient, factory: Factory) -> None:
     assert r.headers["X-Auth-Request-User"] == token_data.username
     assert r.headers["X-Auth-Request-Service"] == "tap"
 
+    # It still works if no scope restrictions are present.
+    r = await client.get(
+        "/ingress/auth",
+        params=(("only_service", "tap"), ("only_service", "vo-cutouts")),
+        headers={"Authorization": f"Bearer {internal_token}"},
+    )
+    assert r.status_code == 200
+    assert r.headers["X-Auth-Request-User"] == token_data.username
+    assert r.headers["X-Auth-Request-Service"] == "tap"
+
     # But an internal token delegated to a service that isn't one of the valid
     # ones will not work.
     r = await client.get(