Require OIDC identity binding before issuing tenant credentials

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/<namespace> \
+  openserverless.apache.org/sso-issuer='<exact configured issuer>' \
+  openserverless.apache.org/sso-sub='<verified owner subject>' --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

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,19 +150,35 @@ 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)
 
         if not user_data:
             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,18 +205,17 @@ 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):
             logging.info(f"WhiskUser for OIDC user {login} created")
             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):
         """

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(

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()