diff --git a/changelog.d/20241211_105613_rra_DM_47986.md b/changelog.d/20241211_105613_rra_DM_47986.md new file mode 100644 index 00000000..19042522 --- /dev/null +++ b/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. diff --git a/src/gafaelfawr/handlers/ingress.py b/src/gafaelfawr/handlers/ingress.py index 6a10bf71..38bdc46f 100644 --- a/src/gafaelfawr/handlers/ingress.py +++ b/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, diff --git a/tests/data/kubernetes/input/ingresses.yaml b/tests/data/kubernetes/input/ingresses.yaml index cd69a67a..719e8063 100644 --- a/tests/data/kubernetes/input/ingresses.yaml +++ b/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: diff --git a/tests/data/kubernetes/output/ingresses.yaml b/tests/data/kubernetes/output/ingresses.yaml index 0e4a9b90..7a111133 100644 --- a/tests/data/kubernetes/output/ingresses.yaml +++ b/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: diff --git a/tests/handlers/ingress_test.py b/tests/handlers/ingress_test.py index 9ba01e93..48193e5f 100644 --- a/tests/handlers/ingress_test.py +++ b/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(