diff --git a/changelog.d/20231204_142753_rra_DM_41998.md b/changelog.d/20231204_142753_rra_DM_41998.md new file mode 100644 index 000000000..acd3a8a7f --- /dev/null +++ b/changelog.d/20231204_142753_rra_DM_41998.md @@ -0,0 +1,3 @@ +### New features + +- An ingress may now be restricted to a specific user by setting the `username` attribute in the `config` section of a `GafaelfawrIngress`, or the corresponding `username` query parameter to the `/auth` route. Any other user will receive a 403 error. The scope requiremments must also still be met. diff --git a/crds/ingress.yaml b/crds/ingress.yaml index d81837e8f..89d7ddbe2 100644 --- a/crds/ingress.yaml +++ b/crds/ingress.yaml @@ -172,6 +172,13 @@ spec: - true required: - anonymous + username: + type: string + description: >- + Restrict access to this ingress to the given username. All + other users, regardless of their scopes, will receive 403 + errors. The user's token must still satisfy any scope + constraints. template: type: object description: "The template used to create the ingress." diff --git a/docs/user-guide/gafaelfawringress.rst b/docs/user-guide/gafaelfawringress.rst index 369b72317..e0b5c6538 100644 --- a/docs/user-guide/gafaelfawringress.rst +++ b/docs/user-guide/gafaelfawringress.rst @@ -196,6 +196,19 @@ The same token will also still be passed in the ``X-Auth-Request-Token`` header. If this configuration option is set, the incoming ``Authorization`` header will be entirely replaced by one containing only the delegated token, unlike Gafaelfawr's normal behavior of preserving any incoming ``Authorization`` header that doesn't include a Gafaelfawr token. +Per-user ingresses +================== + +Access to an ingress may be restricted to a specific user as follows: + +.. code-block:: yaml + + config: + username: + +Any user other than the one with the username ```` will then receive a 403 error. +The scope requirements must still be met to allow access. + .. _anonymous: Anonymous ingresses diff --git a/docs/user-guide/manual-ingress.rst b/docs/user-guide/manual-ingress.rst index 52b2f721c..e2b10ce72 100644 --- a/docs/user-guide/manual-ingress.rst +++ b/docs/user-guide/manual-ingress.rst @@ -203,4 +203,9 @@ Most but not all of these are discussed above. If set to a true value, replace the ``Authorization`` header with one containing the delegated token as a bearer token. This option only makes sense in combination with ``notebook`` or ``delegate_to``. +``username`` (optional) + If set, access to this ingress is restricted to the specified user. + Any other user will receive a 403 error. + ``scope`` must still be set and its requirements are still enforced. + These parameters must be URL-encoded as GET parameters to the ``/auth`` route. diff --git a/src/gafaelfawr/handlers/auth.py b/src/gafaelfawr/handlers/auth.py index 38ec38cca..170a587e5 100644 --- a/src/gafaelfawr/handlers/auth.py +++ b/src/gafaelfawr/handlers/auth.py @@ -62,6 +62,9 @@ class AuthConfig: use_authorization: bool """Whether to put any delegated token in the ``Authorization`` header.""" + username: str | None + """Restrict access to the ingress to only this username.""" + def auth_uri( x_original_uri: (str | None) = Header( @@ -152,6 +155,17 @@ def auth_config( ), examples=[True], ), + username: str | None = Query( + None, + title="Restrict to username", + description=( + "Only allow access to this ingress by the user with the given" + " username. All other users, regardless of scopes, will receive" + " 403 errors. The user must still meet the scope requirements" + " for the ingress." + ), + examples=["rra"], + ), auth_uri: str = Depends(auth_uri), context: RequestContext = Depends(context_dependency), ) -> AuthConfig: @@ -174,6 +188,8 @@ def auth_config( required_scopes=sorted(scopes), satisfy=satisfy.name.lower(), ) + if username: + context.rebind_logger(required_user=username) if delegate_scope: delegate_scopes = {s.strip() for s in delegate_scope.split(",")} @@ -193,6 +209,7 @@ def auth_config( delegate_scopes=delegate_scopes, minimum_lifetime=lifetime, use_authorization=use_authorization, + username=username, ) @@ -308,6 +325,16 @@ 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. + 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. context.logger.info("Token authorized") headers = await build_success_headers(context, auth_config, token_data) diff --git a/src/gafaelfawr/models/kubernetes.py b/src/gafaelfawr/models/kubernetes.py index cea23839f..f5482a1ae 100644 --- a/src/gafaelfawr/models/kubernetes.py +++ b/src/gafaelfawr/models/kubernetes.py @@ -273,6 +273,9 @@ class GafaelfawrIngressConfig(CamelCaseModel): ) """The scopes to require for access.""" + username: str | None = None + """Restrict access to the given user.""" + @model_validator(mode="after") def _validate_conflicts(self) -> Self: """Check for conflicts between settings. @@ -288,7 +291,13 @@ def _validate_conflicts(self) -> Self: raise ValueError(msg) if self.scopes and self.scopes.is_anonymous(): - fields = ("auth_type", "delegate", "login_redirect", "replace_403") + fields = ( + "auth_type", + "delegate", + "login_redirect", + "replace_403", + "username", + ) for snake_name in fields: if getattr(self, snake_name, None): camel_name = to_camel_case(snake_name) @@ -326,6 +335,8 @@ def to_auth_url(self) -> str: query.append(("use_authorization", "true")) if self.auth_type: query.append(("auth_type", self.auth_type.value)) + if self.username: + query.append(("username", self.username)) return f"{base_url}/auth?" + urlencode(query) diff --git a/tests/data/kubernetes/input/ingresses.yaml b/tests/data/kubernetes/input/ingresses.yaml index f080fdd48..bd202fcbe 100644 --- a/tests/data/kubernetes/input/ingresses.yaml +++ b/tests/data/kubernetes/input/ingresses.yaml @@ -155,3 +155,29 @@ template: name: something port: name: http +--- +apiVersion: gafaelfawr.lsst.io/v1alpha1 +kind: GafaelfawrIngress +metadata: + name: username-ingress + namespace: {namespace} +config: + baseUrl: "https://foo.example.com" + scopes: + all: ["read:all"] + username: some-user +template: + metadata: + name: username + spec: + rules: + - host: foo.example.com + http: + paths: + - path: /foo + pathType: Prefix + backend: + service: + name: something + port: + name: http diff --git a/tests/data/kubernetes/output/ingresses.yaml b/tests/data/kubernetes/output/ingresses.yaml index 071b83fc5..d4aa95f8b 100644 --- a/tests/data/kubernetes/output/ingresses.yaml +++ b/tests/data/kubernetes/output/ingresses.yaml @@ -195,3 +195,41 @@ spec: port: name: http status: {any} +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: username + 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-Token,X-Auth-Request-User" + nginx.ingress.kubernetes.io/auth-url: "https://foo.example.com/auth?scope=read%3Aall&username=some-user" + 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: username-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} diff --git a/tests/handlers/auth_test.py b/tests/handlers/auth_test.py index f8d23106c..86b1e721d 100644 --- a/tests/handlers/auth_test.py +++ b/tests/handlers/auth_test.py @@ -1037,3 +1037,29 @@ async def test_ldap_error( # We should not report any error message to Slack, however. If we did, we # would risk drowning the alert channel during an LDAP outage. assert mock_slack.messages == [] + + +@pytest.mark.asyncio +async def test_user(client: AsyncClient, factory: Factory) -> None: + token_data = await create_session_token( + factory, group_names=["admin"], scopes=["read:all"] + ) + + r = await client.get( + "/auth", + params={"scope": "read:all", "username": token_data.username}, + headers={"Authorization": f"Bearer {token_data.token}"}, + ) + assert r.status_code == 200 + assert r.headers["X-Auth-Request-User"] == token_data.username + + r = await client.get( + "/auth", + params={"scope": "read:all", "username": "other-user"}, + 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