From 59c8105ebc6a89ea74c8d5c7102dfa90f10925be Mon Sep 17 00:00:00 2001 From: miki3421 <42601846+miki3421@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:05:19 +0200 Subject: [PATCH] Require immutable OIDC identity binding before issuing tenant credentials --- docs/OIDC.md | 22 ++++ openserverless/impl/auth/auth_service.py | 47 ++++++-- tests/test_auth_service_oidc.py | 24 +++- tests/test_sso_identity_binding.py | 141 +++++++++++++++++++++++ 4 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 tests/test_sso_identity_binding.py diff --git a/docs/OIDC.md b/docs/OIDC.md index 9a883d1..82c7b13 100644 --- a/docs/OIDC.md +++ b/docs/OIDC.md @@ -65,6 +65,28 @@ precedence: If a namespace cannot be provisioned automatically, an existing `WhiskUser` can be bound to an SSO identity manually by an admin, or auto-provisioning can be enabled (see below). +Namespace mapping does not grant ownership. Before returning credentials, admin-api requires +the `WhiskUser` annotations `openserverless.apache.org/sso-issuer` and +`openserverless.apache.org/sso-sub` to match the validated token's nonempty `iss` and `sub` +exactly. An unbound legacy namespace, a different subject, or a disabled binding receives +`403`, even when the external username matches the namespace. Password login is unaffected. + +To bind an existing namespace, a cluster administrator must verify its owner and obtain the +stable subject identifier from the trusted identity provider, then explicitly set: + +```sh +kubectl -n openserverless annotate whisksusers/ \ + openserverless.apache.org/sso-issuer='' \ + openserverless.apache.org/sso-sub='' --overwrite +``` + +This operation must not be delegated to an unauthenticated caller or derived from a matching +username alone. If the binding was deliberately disabled, re-enabling it requires a separate +administrator decision to remove or change `openserverless.apache.org/sso-disabled`. +Auto-provisioning records the binding on a new namespace and never takes over an unbound +existing namespace or one belonging to another subject. Binding is checked again after +provisioning. Orphan metadata and a `WhiskUser` replaced during the login are rejected. + ## Auto-provisioning By default, an OIDC login only succeeds if a `WhiskUser` for the resolved namespace already diff --git a/openserverless/impl/auth/auth_service.py b/openserverless/impl/auth/auth_service.py index 9897a4f..fe95eea 100644 --- a/openserverless/impl/auth/auth_service.py +++ b/openserverless/impl/auth/auth_service.py @@ -127,6 +127,11 @@ def login_oidc(self, access_token, expected_namespace=None): logging.warning(f"OIDC token validation failed: {exc}") return res_builder.build_error_message("Invalid OIDC token", 401) + # A username is a routing hint, not a stable external identity. + if any(not isinstance(claims.get(key), str) or not claims[key].strip() + for key in ("iss", "sub")): + return res_builder.build_error_message("Invalid OIDC identity", 401) + username_claim = self._environ.get("OIDC_USERNAME_CLAIM", "preferred_username") external_username = claims[username_claim] try: @@ -145,12 +150,17 @@ def login_oidc(self, access_token, expected_namespace=None): 403, ) - if self.is_sso_login_disabled(login): - logging.warning(f"OIDC user {external_username} is unbound from namespace {login}") + whisk_user = self.kube_client.get_whisk_user(login) + if whisk_user and not self.is_sso_identity_bound(whisk_user, claims): + logging.warning(f"OIDC identity is not bound to namespace {login}") return res_builder.build_error_message("Forbidden", 403) user_data = self.fetch_user_data(login) + if user_data and not whisk_user: + # Orphan metadata must not be claimed through a concurrent create. + return res_builder.build_error_message("Forbidden", 403) + if not user_data: user_data = self.provision_oidc_user_if_enabled(login, external_username, claims) @@ -158,6 +168,17 @@ def login_oidc(self, access_token, expected_namespace=None): logging.warning(f"OIDC user {external_username} has no provisioned namespace") return res_builder.build_error_message("Namespace not provisioned", 404) + # Re-read after provisioning/waiting: an existing resource, including + # one found after a create conflict, must belong to this exact identity. + current_whisk_user = self.kube_client.get_whisk_user(login) + if not self.is_sso_identity_bound(current_whisk_user, claims): + return res_builder.build_error_message("Forbidden", 403) + if whisk_user and ( + (whisk_user.get("metadata") or {}).get("uid") + != (current_whisk_user.get("metadata") or {}).get("uid") + ): + return res_builder.build_error_message("Forbidden", 403) + login_data = self.map_login_data(user_data) if "AUTH" not in login_data: logging.error(f"OIDC user {login} metadata is missing AUTH") @@ -184,8 +205,7 @@ def provision_oidc_user_if_enabled(self, login, external_username, claims): def ensure_whisk_user(self, login, external_username, email, claims): existing_whisk_user = self.kube_client.get_whisk_user(login) if existing_whisk_user: - logging.info(f"WhiskUser for OIDC user {login} already exists") - return True + return self.is_sso_identity_bound(existing_whisk_user, claims) whisk_user = self.build_sso_whisk_user(login, external_username, email, claims) if self.kube_client.create_whisk_user(whisk_user): @@ -193,9 +213,9 @@ def ensure_whisk_user(self, login, external_username, email, claims): return True # A concurrent login may have created the CR between get and create. - if self.kube_client.get_whisk_user(login): - logging.info(f"WhiskUser for OIDC user {login} found after create conflict") - return True + existing_whisk_user = self.kube_client.get_whisk_user(login) + if existing_whisk_user: + return self.is_sso_identity_bound(existing_whisk_user, claims) logging.error(f"failed to create WhiskUser for OIDC user {login}") return False @@ -299,13 +319,18 @@ def _float_env(self, name, default): except (TypeError, ValueError): return default - def is_sso_login_disabled(self, login): - if not hasattr(self.kube_client, "get_whisk_user"): + def is_sso_identity_bound(self, whisk_user, claims): + if not whisk_user: return False - whisk_user = self.kube_client.get_whisk_user(login) annotations = ((whisk_user or {}).get("metadata") or {}).get("annotations") or {} - return self._is_truthy(annotations.get(SSO_DISABLED_ANNOTATION)) + return ( + not self._is_truthy(annotations.get(SSO_DISABLED_ANNOTATION)) + and isinstance(claims.get("iss"), str) and bool(claims["iss"].strip()) + and isinstance(claims.get("sub"), str) and bool(claims["sub"].strip()) + and annotations.get(SSO_ISSUER_ANNOTATION) == claims["iss"] + and annotations.get(SSO_SUB_ANNOTATION) == claims["sub"] + ) def map_login_data(self, user_data): """ diff --git a/tests/test_auth_service_oidc.py b/tests/test_auth_service_oidc.py index b139f26..808e0f3 100644 --- a/tests/test_auth_service_oidc.py +++ b/tests/test_auth_service_oidc.py @@ -55,6 +55,8 @@ def get_whisk_user(self, username): def create_whisk_user(self, whisk_user): self.created.append(whisk_user) + if self.create_result: + self.existing = whisk_user return self.create_result @@ -69,12 +71,22 @@ def service(self, docs, environ=None, kube_client=None): return AuthService( environ=env, couch_db=FakeCouchDB(docs), - kube_client=kube_client if kube_client is not None else object(), + kube_client=kube_client if kube_client is not None else FakeKubeClient( + existing={ + "metadata": {"annotations": { + "openserverless.apache.org/sso-issuer": "http://issuer.test", + "openserverless.apache.org/sso-sub": "owner-subject", + }}, + "spec": {"auth": "uuid:key"}, + } + ), ) @patch("openserverless.impl.auth.auth_service.OidcTokenValidator") def test_oidc_login_returns_ops_login_data(self, validator_class): validator_class.return_value.validate.return_value = { + "iss": "http://issuer.test", + "sub": "owner-subject", "preferred_username": "devel", } service = self.service( @@ -109,6 +121,8 @@ def test_oidc_login_returns_ops_login_data(self, validator_class): @patch("openserverless.impl.auth.auth_service.OidcTokenValidator") def test_oidc_login_returns_404_when_namespace_is_missing(self, validator_class): validator_class.return_value.validate.return_value = { + "iss": "http://issuer.test", + "sub": "owner-subject", "preferred_username": "missing.lab", } service = self.service([]) @@ -122,6 +136,8 @@ def test_oidc_login_returns_404_when_namespace_is_missing(self, validator_class) @patch("openserverless.impl.auth.auth_service.OidcTokenValidator") def test_oidc_login_returns_403_when_sso_binding_is_disabled(self, validator_class): validator_class.return_value.validate.return_value = { + "iss": "http://issuer.test", + "sub": "owner-subject", "preferred_username": "devel", } service = self.service( @@ -183,7 +199,7 @@ def test_oidc_login_autoprovisions_whisk_user_when_enabled(self, validator_class kube_client=kube_client, ) - with app.app_context(): + with app.app_context(), patch.object(service, "_random_auth", return_value="uuid:key"): response = service.login_oidc("token") self.assertEqual(200, response.status_code) @@ -268,7 +284,7 @@ def test_oidc_login_autoprovisions_normalized_namespace(self, validator_class): kube_client=kube_client, ) - with app.app_context(): + with app.app_context(), patch.object(service, "_random_auth", return_value="uuid:key"): response = service.login_oidc("token") self.assertEqual(200, response.status_code) @@ -286,6 +302,8 @@ def test_oidc_login_autoprovisions_normalized_namespace(self, validator_class): @patch("openserverless.impl.auth.auth_service.OidcTokenValidator") def test_oidc_login_returns_500_when_auth_is_missing(self, validator_class): validator_class.return_value.validate.return_value = { + "iss": "http://issuer.test", + "sub": "owner-subject", "preferred_username": "developer.lab", } service = self.service( diff --git a/tests/test_sso_identity_binding.py b/tests/test_sso_identity_binding.py new file mode 100644 index 0000000..9b6e879 --- /dev/null +++ b/tests/test_sso_identity_binding.py @@ -0,0 +1,141 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import copy +import unittest +from unittest.mock import Mock, patch + +from openserverless import app +from openserverless.impl.auth.auth_service import AuthService + + +class SsoIdentityBindingTest(unittest.TestCase): + def setUp(self): + self.claims = {"iss": "https://issuer.test/realm", "sub": "owner-id", + "preferred_username": "tenantuser", "email": "user@example.test"} + self.resource = {"metadata": {"name": "tenantuser", "uid": "tenant-uid", "annotations": { + "openserverless.apache.org/sso-issuer": self.claims["iss"], + "openserverless.apache.org/sso-sub": self.claims["sub"], + }}, "spec": {"namespace": "tenantuser", "auth": "uuid:key"}} + self.metadata = {"login": "tenantuser", "email": "user@example.test", + "env": [{"key": "AUTH", "value": "uuid:key"}]} + self.couch = Mock() + self.couch.find_doc.return_value = {"docs": [self.metadata]} + self.kube = Mock() + self.kube.get_whisk_user.return_value = self.resource + self.service = AuthService(environ={}, couch_db=self.couch, kube_client=self.kube) + + def login(self, claims=None, **kwargs): + with patch("openserverless.impl.auth.auth_service.OidcTokenValidator") as validator: + validator.return_value.validate.return_value = claims if claims is not None else self.claims + with app.app_context(): + return self.service.login_oidc("signed-token", **kwargs) + + def denied(self, response, status=403): + self.assertEqual(status, response.status_code) + self.assertNotIn("AUTH", response.json) + self.kube.update_whisk_user.assert_not_called() + + def test_matching_immutable_identity_receives_own_credentials(self): + response = self.login() + self.assertEqual(200, response.status_code) + self.assertEqual("uuid:key", response.json["AUTH"]) + + def test_legacy_namespace_without_binding_is_denied(self): + self.resource["metadata"]["annotations"] = {} + self.denied(self.login()) + self.couch.find_doc.assert_not_called() + self.kube.create_whisk_user.assert_not_called() + + def test_different_identity_same_username_is_denied(self): + self.denied(self.login({**self.claims, "sub": "different-owner"})) + + def test_different_bound_issuer_is_denied(self): + self.resource["metadata"]["annotations"]["openserverless.apache.org/sso-issuer"] = "https://other.test" + self.denied(self.login()) + + def test_missing_and_non_string_identity_claims_are_denied(self): + for key in ("iss", "sub"): + for value in (None, "", " ", 123, ["owner-id"]): + with self.subTest(key=key, value=value): + self.denied(self.login({**self.claims, key: value}), 401) + + def test_partial_binding_is_denied(self): + for key in ("openserverless.apache.org/sso-issuer", "openserverless.apache.org/sso-sub"): + resource = copy.deepcopy(self.resource) + del resource["metadata"]["annotations"][key] + self.kube.get_whisk_user.return_value = resource + with self.subTest(key=key): + self.denied(self.login()) + + def test_disabled_matching_binding_is_denied(self): + self.resource["metadata"]["annotations"]["openserverless.apache.org/sso-disabled"] = "true" + self.denied(self.login()) + + def test_explicit_namespace_claim_does_not_bypass_binding(self): + self.service._environ = {"OIDC_NAMESPACE_CLAIM": "workspace"} + self.denied(self.login({**self.claims, "preferred_username": "attackeruser", + "workspace": "tenantuser", "sub": "attacker-id"})) + + def test_requested_namespace_does_not_bypass_binding(self): + self.denied(self.login({**self.claims, "sub": "attacker-id"}, expected_namespace="tenantuser")) + + def test_missing_whisk_user_does_not_expose_orphan_metadata(self): + self.kube.get_whisk_user.return_value = None + self.denied(self.login()) + + def test_revoked_binding_after_metadata_lookup_is_denied(self): + revoked = copy.deepcopy(self.resource) + revoked["metadata"]["annotations"]["openserverless.apache.org/sso-disabled"] = "true" + self.kube.get_whisk_user.side_effect = [self.resource, revoked] + self.denied(self.login()) + + def test_stale_credentials_after_recreation_are_not_returned(self): + recreated = copy.deepcopy(self.resource) + recreated["metadata"]["uid"] = "recreated-tenant-uid" + self.kube.get_whisk_user.side_effect = [self.resource, recreated] + self.denied(self.login()) + + def test_existing_metadata_credentials_remain_authoritative(self): + self.resource["spec"]["auth"] = "old-spec:old-key" + response = self.login() + self.assertEqual(200, response.status_code) + self.assertEqual("uuid:key", response.json["AUTH"]) + + def test_existing_resource_during_provisioning_must_match_identity(self): + self.resource["metadata"]["annotations"]["openserverless.apache.org/sso-sub"] = "other-id" + self.assertFalse(self.service.ensure_whisk_user("tenantuser", "tenantuser", "u@example.test", self.claims)) + self.kube.create_whisk_user.assert_not_called() + + def test_create_conflict_rejects_other_owner_and_legacy_and_disabled(self): + variants = [copy.deepcopy(self.resource) for _ in range(3)] + variants[0]["metadata"]["annotations"]["openserverless.apache.org/sso-sub"] = "other-id" + variants[1]["metadata"]["annotations"] = {} + variants[2]["metadata"]["annotations"]["openserverless.apache.org/sso-disabled"] = "true" + for resource in variants: + with self.subTest(resource=resource): + self.kube.get_whisk_user.side_effect = [None, resource] + self.kube.create_whisk_user.return_value = False + self.assertFalse(self.service.ensure_whisk_user("tenantuser", "tenantuser", "u@example.test", self.claims)) + + def test_create_conflict_accepts_same_bound_owner(self): + self.kube.get_whisk_user.side_effect = [None, self.resource] + self.kube.create_whisk_user.return_value = False + self.assertTrue(self.service.ensure_whisk_user("tenantuser", "tenantuser", "u@example.test", self.claims)) + + +if __name__ == "__main__": + unittest.main()