Skip to content

Add system:cluster:... groups to effective users #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mjudeikis merged 3 commits into from
Sep 29, 2025
Merged

Add system:cluster:... groups to effective users #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
525f42a
UPSTREAM: <carry>: Add authentication.kcp.io/scopes to service accounts
ntnn Sep 10, 2025
3dfd955
UPSTREAM: <carry>: Add system:cluster:<cluster> group to effective users
ntnn Sep 10, 2025
74a7d5a
Update pkg/registry/rbac/validation/kcp_test.go
ntnn Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 67 additions & 11 deletions pkg/registry/rbac/validation/kcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package validation
import (
"context"
"fmt"
"slices"
"strings"

"github.com/kcp-dev/logicalcluster/v3"
Expand Down Expand Up @@ -32,6 +33,10 @@ const (
// are and'ed.
ScopeExtraKey = "authentication.kcp.io/scopes"

// ClusterExtraKey is the key used in a user's "extra" to specify
// the origin cluster of the user.
ClusterExtraKey = "authentication.kcp.io/cluster-name"

// ClusterPrefix is the prefix for cluster scopes.
clusterPrefix = "cluster:"
)
Expand Down Expand Up @@ -79,6 +84,35 @@ func withScopesAndWarrants(appliesToUser appliesToUserFunc) appliesToUserFuncCtx
}
}

// EffectiveScopes takes the scopes of a user and returns the
// intersection of all scopes.
func EffectiveScopes(in []string) []string {
if len(in) == 0 {
return in
}

effectiveScopes := strings.Split(in[0], ",")
if len(effectiveScopes) == 0 || len(in) == 1 {
return effectiveScopes
}

for _, further := range in[1:] {
furtherScopes := strings.Split(further, ",")
newEffective := make([]string, 0, len(effectiveScopes))
for _, es := range effectiveScopes {
if slices.Contains(furtherScopes, es) {
newEffective = append(newEffective, es)
}
}
effectiveScopes = newEffective
if len(effectiveScopes) == 0 {
return []string{}
}
}

return effectiveScopes
}

var (
authenticated = &user.DefaultInfo{Name: user.Anonymous, Groups: []string{user.AllAuthenticated}}
unauthenticated = &user.DefaultInfo{Name: user.Anonymous, Groups: []string{user.AllUnauthenticated}}
Expand All @@ -96,6 +130,10 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info {
recursive = func(u user.Info) {
if IsInScope(u, clusterName) {
ret = append(ret, u)
// TODO(ntnn): Add system:cluster:<cluster> group?
// Though the user is in scope so it shouldn't need the
// group added but it might be better for consistency?
// Leaving it out for now but could be revisited.
} else {
found := false
for _, g := range u.GetGroups() {
Expand All @@ -119,17 +157,29 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info {
if g == user.AllAuthenticated {
rewritten.Groups = append(rewritten.Groups, user.AllAuthenticated)
}
// system:cluster:* groups are used to allow
// controlling binding APIExports between workspaces
// via RBAC
if strings.HasPrefix(g, "system:cluster:") {
rewritten.Groups = append(rewritten.Groups, g)
}
// Add the system:cluster:<cluster> group if the SA has
// an effective scope for the cluster the effective
// users are calculated for.
for _, g := range EffectiveScopes(u.GetExtra()[ScopeExtraKey]) {
if strings.HasPrefix(g, clusterPrefix) {
rewritten.Groups = append(rewritten.Groups, "system:"+g)
}
}
ret = append(ret, rewritten)
}
}

if len(u.GetExtra()[WarrantExtraKey]) > 0 && len(u.GetExtra()[ClusterExtraKey]) > 0 && !IsServiceAccount(u) {
// Users that are not SAs and have a cluster extra key
// cannot have warrants, as they would be coming from
// per-workspace authentication.
// If this happens it is either a misconfiguration from the
// admin or a malicious actor that tries to escalate
// privileges through warrants.
return
}

for _, v := range u.GetExtra()[WarrantExtraKey] {
var w Warrant
if err := json.Unmarshal([]byte(v), &w); err != nil {
Expand All @@ -143,9 +193,17 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info {
Extra: w.Extra,
}
if IsServiceAccount(wu) && len(w.Extra[authserviceaccount.ClusterNameKey]) == 0 {
// warrants must be scoped to a cluster
// For SAs warrants must be scoped to a cluster.
continue
}
// Add the system:cluster:<cluster> group if the warrant has
// an effective scope for the cluster the effective
// users are calculated for.
for _, g := range EffectiveScopes(wu.GetExtra()[ScopeExtraKey]) {
if strings.HasPrefix(g, clusterPrefix) {
wu.Groups = append(wu.Groups, "system:"+g)
}
}
recursive(wu)
}
}
Expand All @@ -156,11 +214,9 @@ func EffectiveUsers(clusterName logicalcluster.Name, u user.Info) []user.Info {
Name: user.Anonymous,
Groups: []string{user.AllAuthenticated},
}
for _, g := range u.GetGroups() {
// system:cluster:* groups are used to allow controlling
// binding APIExports between workspaces via RBAC
if strings.HasPrefix(g, "system:cluster:") {
authed.Groups = append(authed.Groups, g)
for _, g := range EffectiveScopes(u.GetExtra()[ScopeExtraKey]) {
if strings.HasPrefix(g, clusterPrefix) {
authed.Groups = append(authed.Groups, "system:"+g)
}
}
ret = append(ret, authed)
Expand Down
117 changes: 109 additions & 8 deletions pkg/registry/rbac/validation/kcp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,30 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) {
sub: rbacv1.Subject{Kind: "User", Name: "user-a"},
want: true,
},
{
name: "simple matching user with warrants and from this cluster",
user: &user.DefaultInfo{Name: "user-a", Extra: map[string][]string{
WarrantExtraKey: {`{"user":"user-b"}`},
ClusterExtraKey: {"this"},
}},
sub: rbacv1.Subject{Kind: "User", Name: "user-a"},
want: true, // user is subject
},
{
name: "simple non-matching user with matching warrants",
user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-a"}`}}},
sub: rbacv1.Subject{Kind: "User", Name: "user-a"},
want: true,
},
{
name: "simple non-matching user with matching warrants but with cluster-name",
user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{
WarrantExtraKey: {`{"user":"user-a"}`},
ClusterExtraKey: {"this"},
}},
sub: rbacv1.Subject{Kind: "User", Name: "user-a"},
want: false, // Warrants are ineffective on users with cluster
},
{
name: "simple non-matching user with non-matching warrants",
user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b"}`}}},
Expand All @@ -190,6 +208,15 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) {
sub: rbacv1.Subject{Kind: "User", Name: "user-a"},
want: true,
},
{
name: "simple non-matching user with multiple warrants and cluster-name",
user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{
WarrantExtraKey: {`{"user":"user-b"}`, `{"user":"user-a"}`, `{"user":"user-c"}`},
ClusterExtraKey: {"this"},
}},
sub: rbacv1.Subject{Kind: "User", Name: "user-a"},
want: false, // Warrants are ineffective on users with cluster
},
{
name: "simple non-matching user with nested warrants",
user: &user.DefaultInfo{Name: "user-b", Extra: map[string][]string{WarrantExtraKey: {`{"user":"user-b","extra":{"authorization.kcp.io/warrant":["{\"user\":\"user-a\"}"]}}`}}},
Expand All @@ -206,13 +233,13 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) {
},
{
name: "non-cluster-aware service account with this scope",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:this"}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ScopeExtraKey: {"cluster:this"}}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: true,
},
{
name: "non-cluster-aware service account with other scope",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/scopes": {"cluster:other"}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ScopeExtraKey: {"cluster:other"}}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: false,
},
Expand All @@ -226,37 +253,46 @@ func TestAppliesToUserWithWarrantsAndScopes(t *testing.T) {
// service accounts with cluster
{
name: "local service account",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"this"}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ClusterExtraKey: {"this"}}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: true,
},
{
name: "foreign service account",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"other"}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ClusterExtraKey: {"other"}}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: false,
},
{
name: "foreign service account with local warrant",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"other"}, WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["this"]}}`}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{
ClusterExtraKey: {"other"},
WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["this"]}}`},
}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: true,
},
{
name: "foreign service account with foreign warrant",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"other"}, WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["other"]}}`}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{
ClusterExtraKey: {"other"},
WarrantExtraKey: {`{"user":"system:serviceaccount:ns:sa","extra":{"authentication.kcp.io/cluster-name":["other"]}}`},
}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: false,
},
{
name: "local service account with multiple clusters",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"this", "this"}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{ClusterExtraKey: {"this", "this"}}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: false,
},
{
name: "out-of-scope local service account",
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{"authentication.kcp.io/cluster-name": {"this"}, "authentication.kcp.io/scopes": {"cluster:other"}}},
user: &user.DefaultInfo{Name: "system:serviceaccount:ns:sa", Extra: map[string][]string{
ClusterExtraKey: {"this"},
ScopeExtraKey: {"cluster:other"},
}},
sub: rbacv1.Subject{Kind: "ServiceAccount", Namespace: "ns", Name: "sa"},
want: false,
},
Expand Down Expand Up @@ -487,3 +523,68 @@ func TestPrefixUser(t *testing.T) {
})
}
}

func TestEffectiveUsers(t *testing.T) {
tests := map[string]struct {
in []string
want []string
}{
"empty": {
in: []string{},
want: []string{},
},
"one scope entry, one cluster": {
in: []string{"cluster:this"},
want: []string{"cluster:this"},
},
"one scope entry, multiple clusters": {
in: []string{"cluster:this,cluster:that"},
want: []string{"cluster:this", "cluster:that"},
},
"multiple scope entries, multiple clusters, empty result": {
in: []string{
"cluster:this,cluster:that",
"cluster:other",
},
want: []string{},
},
"multiple scope entries, multiple clusters, non-empty result": {
in: []string{
"cluster:this,cluster:that",
"cluster:other,cluster:this",
},
want: []string{"cluster:this"},
},
"multiple scopes entries, multiple clusters, multiple others": {
in: []string{
"cluster:this,foo:bar",
"cluster:this,cluster:other,foo:bar",
"cluster:third,foo:bar,foo:baz",
},
want: []string{
"foo:bar",
},
},
"multiple equal scopes entries": {
in: []string{
"cluster:this,cluster:other,foo:bar",
"cluster:this,cluster:other,foo:bar",
"cluster:this,cluster:other,foo:bar",
},
want: []string{
"cluster:this",
"cluster:other",
"foo:bar",
},
},
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()
got := EffectiveScopes(tt.in)
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("EffectiveScopes() mismatch (-want +got):\n%s", diff)
}
})
}
}
3 changes: 3 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const (
NodeUIDKey = "authentication.kubernetes.io/node-uid"
// ClusterNameKey is the logical cluster name this service-account comes from.
ClusterNameKey = "authentication.kcp.io/cluster-name"
// ClusterScopeKey additionally sets the scope for the service-account to the cluster it belongs to.
ClusterScopeKey = "authentication.kcp.io/scopes"
)

// MakeUsername generates a username from the given namespace and ServiceAccount name.
Expand Down Expand Up @@ -170,6 +172,7 @@ func (sa *ServiceAccountInfo) UserInfo() user.Info {
info.Extra = map[string][]string{}
}
info.Extra[ClusterNameKey] = []string{sa.ClusterName.String()}
info.Extra[ClusterScopeKey] = []string{"cluster:" + sa.ClusterName.String()}
return info
}

Expand Down