Merge pull request #44076 from liggitt/impersonation-groupify

Automatic merge from submit-queue

Include system:authenticated group when impersonating

Fixes #43227

An authorized impersonation request solely for a specific username previously resulted in a `user.Info` that did not include either the `system:authenticated` or `system:unauthenticated` groups. That meant that permissions intended to be granted to all users, like discovery, would be denied the impersonated user.

This allows `kubectl get pods --as=<username>` to work as expected

```release-note
API requests using impersonation now include the `system:authenticated` group in the impersonated user automatically.
```

staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go

@@ -60,7 +60,7 @@ func WithImpersonation(handler http.Handler, requestContextMapper request.Reques
 		}
 
 		// if groups are not specified, then we need to look them up differently depending on the type of user
-		// if they are specified, then they are the authority
+		// if they are specified, then they are the authority (including the inclusion of system:authenticated/system:unauthenticated groups)
 		groupsSpecified := len(req.Header[authenticationapi.ImpersonateGroupHeader]) > 0
 
 		// make sure we're allowed to impersonate each thing we're requesting.  While we're iterating through, start building username
@@ -116,6 +116,22 @@ func WithImpersonation(handler http.Handler, requestContextMapper request.Reques
 			}
 		}
 
+		if !groupsSpecified && username != user.Anonymous {
+			// When impersonating a non-anonymous user, if no groups were specified
+			// if neither the system:authenticated nor system:unauthenticated groups are explicitly included,
+			// include the system:authenticated group in the impersonated user info
+			found := false
+			for _, group := range groups {
+				if group == user.AllAuthenticated || group == user.AllUnauthenticated {
+					found = true
+					break
+				}
+			}
+			if !found {
+				groups = append(groups, user.AllAuthenticated)
+			}
+		}
+
 		newUser := &user.DefaultInfo{
 			Name:   username,
 			Groups: groups,

staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation_test.go

@@ -215,7 +215,7 @@ func TestImpersonationFilter(t *testing.T) {
 			impersonationUserExtras: map[string][]string{"scopes": {"scope-a", "scope-b"}},
 			expectedUser: &user.DefaultInfo{
 				Name:   "system:admin",
-				Groups: []string{},
+				Groups: []string{"system:authenticated"},
 				Extra:  map[string][]string{"scopes": {"scope-a", "scope-b"}},
 			},
 			expectedCode: http.StatusOK,
@@ -229,7 +229,7 @@ func TestImpersonationFilter(t *testing.T) {
 			impersonationUser: "tester",
 			expectedUser: &user.DefaultInfo{
 				Name:   "tester",
-				Groups: []string{},
+				Groups: []string{"system:authenticated"},
 				Extra:  map[string][]string{},
 			},
 			expectedCode: http.StatusOK,
@@ -257,7 +257,48 @@ func TestImpersonationFilter(t *testing.T) {
 			impersonationUser: "system:serviceaccount:foo:default",
 			expectedUser: &user.DefaultInfo{
 				Name:   "system:serviceaccount:foo:default",
-				Groups: []string{"system:serviceaccounts", "system:serviceaccounts:foo"},
+				Groups: []string{"system:serviceaccounts", "system:serviceaccounts:foo", "system:authenticated"},
+				Extra:  map[string][]string{},
+			},
+			expectedCode: http.StatusOK,
+		},
+		{
+			name: "anonymous-username-prevents-adding-authenticated-group",
+			user: &user.DefaultInfo{
+				Name: "system:admin",
+			},
+			impersonationUser: "system:anonymous",
+			expectedUser: &user.DefaultInfo{
+				Name:   "system:anonymous",
+				Groups: []string{},
+				Extra:  map[string][]string{},
+			},
+			expectedCode: http.StatusOK,
+		},
+		{
+			name: "unauthenticated-group-prevents-adding-authenticated-group",
+			user: &user.DefaultInfo{
+				Name: "system:admin",
+			},
+			impersonationUser:   "unknown",
+			impersonationGroups: []string{"system:unauthenticated"},
+			expectedUser: &user.DefaultInfo{
+				Name:   "unknown",
+				Groups: []string{"system:unauthenticated"},
+				Extra:  map[string][]string{},
+			},
+			expectedCode: http.StatusOK,
+		},
+		{
+			name: "unauthenticated-group-prevents-double-adding-authenticated-group",
+			user: &user.DefaultInfo{
+				Name: "system:admin",
+			},
+			impersonationUser:   "unknown",
+			impersonationGroups: []string{"system:authenticated"},
+			expectedUser: &user.DefaultInfo{
+				Name:   "unknown",
+				Groups: []string{"system:authenticated"},
 				Extra:  map[string][]string{},
 			},
 			expectedCode: http.StatusOK,