Merge pull request #1117 from lsst-sqre/tickets/DM-46399

DM-46399: Allow restricting ingresses to services

changelog.d/20240925_144604_rra_DM_46399.md

@@ -0,0 +1,7 @@
+### New features
+
+- Add `config.onlyServices` to `GafaelfawrIngress`, which restricts the ingress to tokens issued to one of the listed services in addition to the other constraints.
+
+### Bug fixes
+
+- Stop including the required scopes in 403 errors when the request was rejected by a username restriction rather than a scope restriction, since the client cannot fix this problem by obtaining different scopes.

crds/ingress.yaml

@@ -131,6 +131,15 @@ spec:
                   description: >-
                     Whether to redirect to the login flow if the user is
                     not currently authenticated.
+                onlyServices:
+                  type: array
+                  description: >-
+                    If set, access is restricted to tokens issued to one of
+                    the listed services, in addition to any other access
+                    constraints. Users will not be able to access the ingress
+                    directly with their own tokens.
+                  items:
+                    type: string
                 replace403:
                   type: boolean
                   description: >-

docs/user-guide/gafaelfawringress.rst

@@ -27,6 +27,7 @@ Here is a simple example, which requires ``read:all`` scope to access a service,
      scopes:
        all:
          - "read:all"
+     service: service
    template:
      metadata:
        name: service-ingress
@@ -48,12 +49,24 @@ The ``apiVersion`` and ``kind`` keys must always have those values.
 The ``metadata`` section is standard Kubernetes resource metadata.
 You can add labels and annotations here if you wish, but they will have no effect on the generated ``Ingress`` resource.
 
+``config`` section
+------------------
+
 The ``config`` section configures Gafaelfawr.
 Here, only the two mandatory parameters are shown.
+Many other settings are possible and are discussed below.
 
-``config.scopes`` specifies the scopes required to access this service.
+``config.scopes`` specifies the scopes required to access this service and is required.
 The scopes can be listed under either ``all``, meaning that all of those scopes are required, or ``any``, meaning that any one of those scopes is required.
 Only one of ``all`` or ``any`` may be given.
+Alternately, the ingress can be anonymous; see :ref:`anonymous` for details on that.
+
+``config.service`` names the service underlying this ingress.
+This is used for logging, metrics, and token delegation (see :ref:`delegated-tokens`).
+This setting is not technically required currently, but will become mandatory in the future and should always be provided.
+
+``template`` section
+--------------------
 
 The ``template`` section is a template for the ``Ingress`` resource.
 It uses a subset of the ``Ingress`` schema.
@@ -119,20 +132,22 @@ Services may request an internal token from Gafaelfawr using the ``config.delega
    config:
      delegate:
        internal:
-         service: "service-name"
          scopes:
            - "read:image"
            - "read:tap"
 
-``config.delegate.internal.service`` should be an identifier for the service (generally the service name).
-It will be added to the metadata of the generated internal token and, from there, to log messages, so that it's possible to track which service is using a delegated token.
+The resulting token will be marked as delegated to the service of the ingress as configured in ``config.service``.
+This information will be used in logging and metrics, and can be used to restrict access to only specific services (see :ref:`ingress-service-only`).
 
 ``config.delegate.internal.scopes`` is a list of scopes requested for the internal token.
 The delegated token will have these scopes if the token used by the user to authenticate to the service had these scopes.
 
 The scopes listed here are not mandatory; if the user's authentication token didn't have them, the Gafaelfawr authorization check will still succeed, the internal delegated token will be provided, but it will not have the missing scopes.
 If the scopes must always be present, also list them in ``config.scopes.all`` as required to access this service.
 
+``config.delegate.internal.service`` overrides ``config.service`` when determining the service associated with the delegated token, and is mandatory if ``config.service`` isn't set.
+This setting is deprecated; set ``config.service`` instead.
+
 The delegated token will be included in the request to the protected service in the ``X-Auth-Request-Token`` HTTP header.
 This token may be used in an ``Authorization`` header with type ``bearer`` to make requests to other protected services.
 It can also be verified and used to obtain information about a user by presenting it in an ``Authorization`` header with type ``bearer`` to either of the ``/auth/v1/api/token-info`` or ``/auth/v1/api/user-info`` Gafaelfawr routes.
@@ -213,6 +228,41 @@ The value must be an `NGINX time interval <https://nginx.org/en/docs/syntax.html
 
 The cache is automatically invalidated if the ``Cookie`` or ``Authorization`` HTTP headers change.
 
+.. _ingress-service-only:
+
+Service-only ingresses
+======================
+
+Sometimes it is useful to restrict an ingress to only allow access from other services acting on behalf of users.
+For example, in a microservice architecture, a user-facing service may call out to other internal services to perform part of its work, but users should not be able to access the internal services directly.
+
+Gafaelfawr supports this use case with ingresses that can only be accessed by tokens issued to other services.
+Normally this is an internal token delegated to a service via its ingress.
+
+To restrict an ingress to only access by a list of other services, use the ``onlyServices`` setting:
+
+.. code-block:: yaml
+
+   config:
+     onlyServices:
+       - portal
+       - vo-cutouts
+
+The value is a list of service names that should be allowed access.
+All other services, and all direct access by users, will be denied.
+
+The names of the services listed here must match the service name in issued tokens that should be permitted access.
+This is configured in ``config.service`` in the ingress for the calling service.
+
+For example, suppose there are two services, user-service and backend-service.
+The user will make direct requests to user-service.
+backend-service wants to only allow requests from user-service, but not directly from users.
+In this case, the ingress for user-service should set ``config.service`` to ``user-service`` and request delegated internal tokens.
+The ingress for backend-service should then set ``config.onlyServices`` to ``["user-service"]``.
+
+All other access restrictions are still applied in addition to the service restrictions.
+So, for example, if the internal token from a listed service doesn't have a required scope, Gafaelfawr will still reject the request.
+
 Per-user ingresses
 ==================
 

src/gafaelfawr/handlers/ingress.py

@@ -62,6 +62,9 @@ class AuthConfig:
     notebook: bool
     """Whether to generate a notebook token."""
 
+    only_services: set[str] | None
+    """Restrict access to tokens issued to one of the listed services."""
+
     satisfy: Satisfy
     """The authorization strategy if multiple scopes are required."""
 
@@ -156,6 +159,20 @@ def auth_config(
             examples=[True],
         ),
     ] = False,
+    only_service: Annotated[
+        list[str] | None,
+        Query(
+            title="Restrict to service",
+            description=(
+                "Restrict access to only tokens issued to the named service,"
+                " in addition to any other constraints. This will prevent"
+                " users from accessing the service directly, but allow the"
+                " named service to access it on their behalf. May be given"
+                " multiple times to allow multiple services."
+            ),
+            examples=["portal", "vo-cutouts"],
+        ),
+    ] = None,
     satisfy: Annotated[
         Satisfy,
         Query(
@@ -238,6 +255,8 @@ def auth_config(
         required_scopes=sorted(scopes),
         satisfy=satisfy.name.lower(),
     )
+    if only_service:
+        context.rebind_logger(only_services=only_service)
     if username:
         context.rebind_logger(required_user=username)
 
@@ -255,6 +274,7 @@ def auth_config(
         delegate_scopes=delegate_scopes,
         delegate_to=delegate_to,
         minimum_lifetime=lifetime,
+        only_services=set(only_service) if only_service else None,
         notebook=notebook,
         satisfy=satisfy,
         scopes=scopes,
@@ -316,14 +336,21 @@ async def get_auth(
             auth_config.scopes,
         )
 
-    # Check a user constraint. InsufficientScopeError is not really correct,
-    # but none of the RFC 6750 error codes are correct and it's the closest.
+    # Check a user or service constraint. InsufficientScopeError is not really
+    # correct, but none of the RFC 6750 error codes are correct and it's the
+    # closest.
+    if auth_config.only_services:
+        if token_data.service not in auth_config.only_services:
+            raise generate_challenge(
+                context,
+                auth_config.auth_type,
+                InsufficientScopeError("Access not allowed for this user"),
+            )
     if auth_config.username and token_data.username != auth_config.username:
         raise generate_challenge(
             context,
             auth_config.auth_type,
             InsufficientScopeError("Access not allowed for this user"),
-            auth_config.scopes,
         )
 
     # Log and return the results.

src/gafaelfawr/models/kubernetes.py

@@ -280,6 +280,9 @@ class GafaelfawrIngressConfig(BaseModel):
     login_redirect: bool = False
     """Whether to redirect unauthenticated users to the login flow."""
 
+    only_services: list[str] | None = None
+    """If non-empty, restrict to tokens issued by one of the services."""
+
     replace_403: bool = False
     """Whether to generate a custom error response for 403 errors."""
 
@@ -318,6 +321,7 @@ def _validate_conflicts(self) -> Self:
                 "auth_type",
                 "delegate",
                 "login_redirect",
+                "only_services",
                 "replace_403",
                 "username",
             )
@@ -349,6 +353,7 @@ def to_auth_query(self) -> list[tuple[str, str]]:
             configuration to pass to the Gafaelfawr ``/ingress/auth`` route.
         """
         query = [("scope", s) for s in self.scopes.scopes]
+        query.extend(("only_service", s) for s in self.only_services or [])
         if self.service:
             query.append(("service", self.service))
         if self.scopes.satisfy != Satisfy.ALL:

tests/data/kubernetes/input/ingresses.yaml

@@ -183,3 +183,31 @@ template:
                   name: something
                   port:
                     name: http
+---
+apiVersion: gafaelfawr.lsst.io/v1alpha1
+kind: GafaelfawrIngress
+metadata:
+  name: service-ingress
+  namespace: {namespace}
+config:
+  scopes:
+    all: ["read:all"]
+  onlyServices:
+    - portal
+    - vo-cutouts
+  service: uws
+template:
+  metadata:
+    name: service
+  spec:
+    rules:
+      - host: foo.example.com
+        http:
+          paths:
+            - path: /foo
+              pathType: Prefix
+              backend:
+                service:
+                  name: something
+                  port:
+                    name: http

tests/data/kubernetes/output/ingresses.yaml

@@ -235,3 +235,41 @@ spec:
                 port:
                   name: http
 status: {any}
+---
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: service
+  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?scope=read%3Aall&only_service=portal&only_service=vo-cutouts&service=uws"
+    nginx.ingress.kubernetes.io/configuration-snippet: |
+      {snippet}
+  creationTimestamp: {any}
+  generation: {any}
+  managedFields: {any}
+  ownerReferences:
+    - apiVersion: gafaelfawr.lsst.io/v1alpha1
+      blockOwnerDeletion: true
+      controller: true
+      kind: GafaelfawrIngress
+      name: service-ingress
+      uid: {any}
+  resourceVersion: {any}
+  uid: {any}
+spec:
+  ingressClassName: nginx
+  rules:
+    - host: foo.example.com
+      http:
+        paths:
+          - path: /foo
+            pathType: Prefix
+            backend:
+              service:
+                name: something
+                port:
+                  name: http
+status: {any}

tests/handlers/ingress_test.py

@@ -1078,3 +1078,65 @@ async def test_user(client: AsyncClient, factory: Factory) -> None:
     assert isinstance(authenticate, AuthErrorChallenge)
     assert authenticate.auth_type == AuthType.Bearer
     assert authenticate.error == AuthError.insufficient_scope
+
+
+@pytest.mark.asyncio
+async def test_only_service(client: AsyncClient, factory: Factory) -> None:
+    token_data = await create_session_token(
+        factory, group_names=["admin"], scopes=["read:all"]
+    )
+
+    # Directly authenticating to an ingress restricted to specific services
+    # will not work.
+    r = await client.get(
+        "/ingress/auth",
+        params=(
+            ("scope", "read:all"),
+            ("only_service", "tap"),
+            ("only_service", "vo-cutouts"),
+        ),
+        headers={"Authorization": f"Bearer {token_data.token}"},
+    )
+    assert r.status_code == 403
+    authenticate = parse_www_authenticate(r.headers["WWW-Authenticate"])
+    assert isinstance(authenticate, AuthErrorChallenge)
+    assert authenticate.auth_type == AuthType.Bearer
+    assert authenticate.error == AuthError.insufficient_scope
+
+    # Getting an internal token and then using that will work.
+    r = await client.get(
+        "/ingress/auth",
+        params={
+            "scope": "read:all",
+            "delegate_to": "tap",
+            "delegate_scope": "read:all",
+        },
+        headers={"Authorization": f"Bearer {token_data.token}"},
+    )
+    assert r.status_code == 200
+    internal_token = r.headers["X-Auth-Request-Token"]
+    r = await client.get(
+        "/ingress/auth",
+        params=(
+            ("scope", "read:all"),
+            ("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(
+        "/ingress/auth",
+        params={"scope": "read:all", "only_service": "vo-cutouts"},
+        headers={"Authorization": f"Bearer {internal_token}"},
+    )
+    assert r.status_code == 403
+    authenticate = parse_www_authenticate(r.headers["WWW-Authenticate"])
+    assert isinstance(authenticate, AuthErrorChallenge)
+    assert authenticate.auth_type == AuthType.Bearer
+    assert authenticate.error == AuthError.insufficient_scope