Filter extra keys containing kcp.io on per-workspace authentication

Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>

Author: ntnn

docs/content/concepts/authentication/workspace.md

@@ -47,6 +47,7 @@ This feature has some small limitations that users should keep in mind:
 * Even when the feature is disabled on all shards and all front-proxies, the API (CRDs) are always available in kcp. Admins might uses RBAC or webhooks to prevent creating `WorkspaceAuthenticationConfiguration` objects if needed.
 * It is not possible to authenticate users with a username starting with with `system:` through per-workspace authentication.
 * It is not possible to assign groups starting with `system:` to users authenticated via per-workspace authentication, e.g. via claim mappings.
+* It is not possible to set keys containing `kcp.io` through the extra mappings in the authentication configuration.
 
 ## Example
 

pkg/authentication/extra.go

@@ -0,0 +1,77 @@
+/*
+Copyright 2025 The KCP Authors.
+
+Licensed 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.
+*/
+
+package authentication
+
+import (
+	"net/http"
+	"slices"
+	"strings"
+
+	"k8s.io/apiserver/pkg/authentication/authenticator"
+	"k8s.io/apiserver/pkg/authentication/user"
+)
+
+// ExtraFilter is a filter that filters out extra fields that are not
+// allowed.
+type ExtraFilter struct {
+	Authenticator authenticator.Request
+
+	// AllowExtraKeys is a list of exact keys to allow through.
+	// It takes precedence over DropExtraKeyContains.
+	AllowExtraKeys []string
+	// DropExtraKeyContains is a list of strings that will cause an extra
+	// key/value pair to be dropped if the key contains any of them.
+	DropExtraKeyContains []string
+}
+
+var _ authenticator.Request = &ExtraFilter{}
+
+func (a *ExtraFilter) AuthenticateRequest(req *http.Request) (*authenticator.Response, bool, error) {
+	resp, ok, err := a.Authenticator.AuthenticateRequest(req)
+	if resp == nil || resp.User == nil {
+		return resp, ok, err
+	}
+
+	info := user.DefaultInfo{
+		Name:   resp.User.GetName(),
+		UID:    resp.User.GetUID(),
+		Groups: resp.User.GetGroups(),
+		Extra:  map[string][]string{},
+	}
+	for k, v := range resp.User.GetExtra() {
+		if slices.Contains(a.AllowExtraKeys, k) {
+			info.Extra[k] = v
+			continue
+		}
+		if containsAny(k, a.DropExtraKeyContains...) {
+			continue
+		}
+		info.Extra[k] = v
+	}
+	resp.User = &info
+
+	return resp, ok, err
+}
+
+func containsAny(s string, substrs ...string) bool {
+	for _, substr := range substrs {
+		if strings.Contains(s, substr) {
+			return true
+		}
+	}
+	return false
+}

pkg/authentication/extra_test.go

@@ -0,0 +1,95 @@
+/*
+Copyright 2025 The KCP Authors.
+
+Licensed 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.
+*/
+
+package authentication
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestExtraFilter(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []struct {
+		name                 string
+		allowExtraKeys       []string
+		dropExtraKeyContains []string
+		requestedExtra       map[string][]string
+		wantExtra            map[string][]string
+	}{
+		{
+			name:      "no extra",
+			wantExtra: map[string][]string{},
+		},
+		{
+			name: "pass all",
+
+			requestedExtra: map[string][]string{
+				"foo":    {"bar"},
+				"foobar": {"baz"},
+				"barfoo": {"baz2"},
+			},
+			wantExtra: map[string][]string{
+				"foo":    {"bar"},
+				"foobar": {"baz"},
+				"barfoo": {"baz2"},
+			},
+		},
+		{
+			name: "drop contains",
+
+			dropExtraKeyContains: []string{"bar"},
+			requestedExtra: map[string][]string{
+				"foo":    {"bar"},
+				"foobar": {"baz"},
+				"barfoo": {"baz2"},
+			},
+			wantExtra: map[string][]string{
+				"foo": {"bar"},
+			},
+		},
+		{
+			name: "allow takes precedence over drop contains",
+
+			allowExtraKeys:       []string{"barfoo"},
+			dropExtraKeyContains: []string{"bar"},
+			requestedExtra: map[string][]string{
+				"foo":    {"bar"},
+				"foobar": {"baz"},
+				"barfoo": {"baz2"},
+			},
+			wantExtra: map[string][]string{
+				"foo":    {"bar"},
+				"barfoo": {"baz2"},
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			filter := &ExtraFilter{
+				Authenticator:        &requestAuthenticator{extra: tc.requestedExtra},
+				AllowExtraKeys:       tc.allowExtraKeys,
+				DropExtraKeyContains: tc.dropExtraKeyContains,
+			}
+			res, gotAuthenticated, err := filter.AuthenticateRequest(&http.Request{})
+			require.NoError(t, err)
+			require.True(t, gotAuthenticated)
+			require.Equal(t, tc.wantExtra, res.User.GetExtra())
+		})
+	}
+}

pkg/authentication/groups_test.go

@@ -29,18 +29,21 @@ import (
 
 type requestAuthenticator struct {
 	groups []string
+	extra  map[string][]string
 }
 
 func (a *requestAuthenticator) AuthenticateRequest(*http.Request) (*authenticator.Response, bool, error) {
 	return &authenticator.Response{
 		User: &user.DefaultInfo{
 			Name:   "system:unsecured",
 			Groups: a.groups,
+			Extra:  a.extra,
 		},
 	}, true, nil
 }
 
 func TestGroupFilter(t *testing.T) {
+	t.Parallel()
 	for _, tc := range []struct {
 		name                                     string
 		passOnGroups, dropGroups                 sets.Set[string]
@@ -114,6 +117,7 @@ func TestGroupFilter(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
 			filter := &GroupFilter{
 				Authenticator:       &requestAuthenticator{groups: tc.requestedGroups},
 				PassOnGroups:        tc.passOnGroups,

pkg/authentication/index.go

@@ -239,10 +239,14 @@ func (c *state) Lookup(wsType logicalcluster.Path) (authenticator.Request, bool)
 
 	// ensure that per-workspace auth cannot be used to become a system: user/group
 	authenticator = ForbidSystemUsernames(authenticator)
-	filtered := &GroupFilter{
+	groupFiltered := &GroupFilter{
 		Authenticator:     authenticator,
 		DropGroupPrefixes: []string{"system:"},
 	}
+	extraFiltered := &ExtraFilter{
+		Authenticator:        groupFiltered,
+		DropExtraKeyContains: []string{"kcp.io"},
+	}
 
-	return filtered, true
+	return extraFiltered, true
 }

pkg/authentication/index_test.go

@@ -34,6 +34,8 @@ import (
 )
 
 func TestCrossShardWorkspaceType(t *testing.T) {
+	t.Parallel()
+
 	const (
 		shardName   = "shard-1"
 		teamCluster = "logicalteamcluster"

test/e2e/authentication/workspace_test.go

@@ -65,8 +65,8 @@ func TestWorkspaceOIDC(t *testing.T) {
 
 	// setup a new workspace auth config that uses mockoidc's server, one for
 	// each of our mockoidc servers
-	authConfigA := authfixtures.CreateWorkspaceOIDCAuthentication(t, ctx, kcpClusterClient, baseWsPath, mockA, ca)
-	authConfigB := authfixtures.CreateWorkspaceOIDCAuthentication(t, ctx, kcpClusterClient, baseWsPath, mockB, ca)
+	authConfigA := authfixtures.CreateWorkspaceOIDCAuthentication(t, ctx, kcpClusterClient, baseWsPath, mockA, ca, nil)
+	authConfigB := authfixtures.CreateWorkspaceOIDCAuthentication(t, ctx, kcpClusterClient, baseWsPath, mockB, ca, nil)
 
 	// use these configs in new WorkspaceTypes and create one extra workspace type that allows
 	// both mockoidc issuers
@@ -268,7 +268,22 @@ func TestUserScope(t *testing.T) {
 	require.NoError(t, err)
 
 	mock, ca := authfixtures.StartMockOIDC(t, server)
-	authConfig := authfixtures.CreateWorkspaceOIDCAuthentication(t, ctx, kcpClusterClient, baseWsPath, mock, ca)
+	authConfig := authfixtures.CreateWorkspaceOIDCAuthentication(t, ctx, kcpClusterClient, baseWsPath, mock, ca,
+		[]tenancyv1alpha1.ExtraMapping{
+			{
+				Key:             "authentication.kcp.io/scopes",
+				ValueExpression: "['cluster:my-test-cluster']",
+			},
+			{
+				Key:             "kcp.io/test",
+				ValueExpression: "'test-value'",
+			},
+			{
+				Key:             "other.example.com/test",
+				ValueExpression: "'pass-value'",
+			},
+		},
+	)
 	wsType := authfixtures.CreateWorkspaceType(t, ctx, kcpClusterClient, baseWsPath, "with-oidc", authConfig)
 
 	// create a new workspace with our new type
@@ -280,6 +295,14 @@ func TestUserScope(t *testing.T) {
 		userEmail      = "[email protected]"
 		userGroups     = []string{"developers", "admins"}
 		expectedGroups = []string{"system:authenticated"}
+		expectedExtras = map[string]authenticationv1.ExtraValue{
+			// authentication.kcp.io/scopes from the extra mapping has
+			// been scrubbed and only the expected cluster:<id> is set
+			"authentication.kcp.io/scopes":       {"cluster:" + teamWs.Spec.Cluster},
+			"authentication.kcp.io/cluster-name": {teamWs.Spec.Cluster},
+			// kcp.io/test from the extra mapping should be scrubbed
+			"other.example.com/test": {"pass-value"},
+		}
 	)
 
 	for _, group := range userGroups {
@@ -314,8 +337,7 @@ func TestUserScope(t *testing.T) {
 	user := review.Status.UserInfo
 	require.Equal(t, "oidc:"+userEmail, user.Username)
 	require.Subset(t, user.Groups, expectedGroups)
-	require.Equal(t, user.Extra["authentication.kcp.io/scopes"], authenticationv1.ExtraValue{"cluster:" + teamWs.Spec.Cluster})
-	require.Equal(t, user.Extra["authentication.kcp.io/cluster-name"], authenticationv1.ExtraValue{teamWs.Spec.Cluster})
+	require.Subset(t, user.Extra, expectedExtras)
 }
 
 func TestForbiddenSystemAccess(t *testing.T) {

test/e2e/fixtures/authfixtures/mockoidc.go

@@ -167,18 +167,21 @@ func CreateOIDCToken(t *testing.T, mock *mockoidc.MockOIDC, subject, email strin
 	return token
 }
 
-func CreateWorkspaceOIDCAuthentication(t *testing.T, ctx context.Context, client kcpclientset.ClusterInterface, workspace logicalcluster.Path, mock *mockoidc.MockOIDC, ca *crypto.CA) string {
+func CreateWorkspaceOIDCAuthentication(t *testing.T, ctx context.Context, client kcpclientset.ClusterInterface, workspace logicalcluster.Path, mock *mockoidc.MockOIDC, ca *crypto.CA, extraMapping []tenancyv1alpha1.ExtraMapping) string {
 	name := fmt.Sprintf("mockoidc-%d", rand.Int())
 
+	jwtAuth := MockJWTAuthenticator(t, mock, ca, "oidc:", "oidc:")
+	if len(extraMapping) > 0 {
+		jwtAuth.ClaimMappings.Extra = extraMapping
+	}
+
 	// setup a new workspace auth config that uses mockoidc's server
 	authConfig := &tenancyv1alpha1.WorkspaceAuthenticationConfiguration{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: name,
 		},
 		Spec: tenancyv1alpha1.WorkspaceAuthenticationConfigurationSpec{
-			JWT: []tenancyv1alpha1.JWTAuthenticator{
-				MockJWTAuthenticator(t, mock, ca, "oidc:", "oidc:"),
-			},
+			JWT: []tenancyv1alpha1.JWTAuthenticator{jwtAuth},
 		},
 	}