Merge pull request #28852 from deads2k/use-user

Automatic merge from submit-queue

authorize based on user.Info

Update the `authorization.Attributes` to use the `user.Info` instead of discrete getters for each piece.

@kubernetes/sig-auth

pkg/auth/authorizer/abac/abac.go

@@ -133,12 +133,19 @@ func matches(p api.Policy, a authorizer.Attributes) bool {
 func subjectMatches(p api.Policy, a authorizer.Attributes) bool {
 	matched := false
 
+	username := ""
+	groups := []string{}
+	if user := a.GetUser(); user != nil {
+		username = user.GetName()
+		groups = user.GetGroups()
+	}
+
 	// If the policy specified a user, ensure it matches
 	if len(p.Spec.User) > 0 {
 		if p.Spec.User == "*" {
 			matched = true
 		} else {
-			matched = p.Spec.User == a.GetUserName()
+			matched = p.Spec.User == username
 			if !matched {
 				return false
 			}
@@ -151,7 +158,7 @@ func subjectMatches(p api.Policy, a authorizer.Attributes) bool {
 			matched = true
 		} else {
 			matched = false
-			for _, group := range a.GetGroups() {
+			for _, group := range groups {
 				if p.Spec.Group == group {
 					matched = true
 				}

pkg/auth/authorizer/interfaces.go

@@ -25,14 +25,8 @@ import (
 // Attributes is an interface used by an Authorizer to get information about a request
 // that is used to make an authorization decision.
 type Attributes interface {
-	// The user string which the request was authenticated as, or empty if
+	// GetUser returns the user.Info object to authorize
-	// no authentication occurred and the request was allowed to proceed.
+	GetUser() user.Info
-	GetUserName() string
-
-	// The list of group names the authenticated user is a member of. Can be
-	// empty if the authenticated user is not in any groups, or if no
-	// authentication occurred.
-	GetGroups() []string
 
 	// GetVerb returns the kube verb associated with API requests (this includes get, list, watch, create, update, patch, delete, and proxy),
 	// or the lowercased HTTP verb associated with non-API requests (this includes get, put, post, patch, and delete)
@@ -101,12 +95,8 @@ type AttributesRecord struct {
 	Path            string
 }
 
-func (a AttributesRecord) GetUserName() string {
+func (a AttributesRecord) GetUser() user.Info {
-	return a.User.GetName()
+	return a.User
-}
-
-func (a AttributesRecord) GetGroups() []string {
-	return a.User.GetGroups()
 }
 
 func (a AttributesRecord) GetVerb() string {

plugin/pkg/auth/authorizer/rbac/rbac.go

@@ -22,7 +22,6 @@ import (
 	"k8s.io/kubernetes/pkg/apis/rbac"
 	"k8s.io/kubernetes/pkg/apis/rbac/validation"
 	"k8s.io/kubernetes/pkg/auth/authorizer"
-	"k8s.io/kubernetes/pkg/auth/user"
 	"k8s.io/kubernetes/pkg/registry/clusterrole"
 	"k8s.io/kubernetes/pkg/registry/clusterrolebinding"
 	"k8s.io/kubernetes/pkg/registry/role"
@@ -36,16 +35,11 @@ type RBACAuthorizer struct {
 }
 
 func (r *RBACAuthorizer) Authorize(attr authorizer.Attributes) error {
-	if r.superUser != "" && attr.GetUserName() == r.superUser {
+	if r.superUser != "" && attr.GetUser() != nil && attr.GetUser().GetName() == r.superUser {
 		return nil
 	}
 
-	userInfo := &user.DefaultInfo{
+	ctx := api.WithNamespace(api.WithUser(api.NewContext(), attr.GetUser()), attr.GetNamespace())
-		Name:   attr.GetUserName(),
-		Groups: attr.GetGroups(),
-	}
-
-	ctx := api.WithNamespace(api.WithUser(api.NewContext(), userInfo), attr.GetNamespace())
 
 	// Frame the authorization request as a privilege escalation check.
 	var requestedRule rbac.PolicyRule

plugin/pkg/auth/authorizer/rbac/rbac_test.go

@@ -99,8 +99,9 @@ func (d *defaultAttributes) String() string {
 		d.user, strings.Split(d.groups, ","), d.verb, d.resource, d.namespace, d.apiGroup)
 }
 
-func (d *defaultAttributes) GetUserName() string     { return d.user }
+func (d *defaultAttributes) GetUser() user.Info {
-func (d *defaultAttributes) GetGroups() []string     { return strings.Split(d.groups, ",") }
+	return &user.DefaultInfo{Name: d.user, Groups: strings.Split(d.groups, ",")}
+}
 func (d *defaultAttributes) GetVerb() string         { return d.verb }
 func (d *defaultAttributes) IsReadOnly() bool        { return d.verb == "get" || d.verb == "watch" }
 func (d *defaultAttributes) GetNamespace() string    { return d.namespace }

plugin/pkg/auth/authorizer/webhook/webhook.go

@@ -129,12 +129,15 @@ func newWithBackoff(kubeConfigFile string, authorizedTTL, unauthorizedTTL, initi
 //     }
 //
 func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (err error) {
-	r := &v1beta1.SubjectAccessReview{
+	r := &v1beta1.SubjectAccessReview{}
-		Spec: v1beta1.SubjectAccessReviewSpec{
+	if user := attr.GetUser(); user != nil {
-			User:   attr.GetUserName(),
+		r.Spec = v1beta1.SubjectAccessReviewSpec{
-			Groups: attr.GetGroups(),
+			User:   user.GetName(),
-		},
+			Groups: user.GetGroups(),
+			Extra:  user.GetExtra(),
+		}
 	}
+
 	if attr.IsResourceRequest() {
 		r.Spec.ResourceAttributes = &v1beta1.ResourceAttributes{
 			Namespace:   attr.GetNamespace(),

test/integration/auth/auth_test.go

@@ -537,7 +537,7 @@ func TestAuthModeAlwaysDeny(t *testing.T) {
 type allowAliceAuthorizer struct{}
 
 func (allowAliceAuthorizer) Authorize(a authorizer.Attributes) error {
-	if a.GetUserName() == "alice" {
+	if a.GetUser() != nil && a.GetUser().GetName() == "alice" {
 		return nil
 	}
 	return errors.New("I can't allow that.  Go ask alice.")
@@ -705,18 +705,18 @@ type impersonateAuthorizer struct{}
 // alice can't act as anyone and bob can't do anything but act-as someone
 func (impersonateAuthorizer) Authorize(a authorizer.Attributes) error {
 	// alice can impersonate service accounts and do other actions
-	if a.GetUserName() == "alice" && a.GetVerb() == "impersonate" && a.GetResource() == "serviceaccounts" {
+	if a.GetUser() != nil && a.GetUser().GetName() == "alice" && a.GetVerb() == "impersonate" && a.GetResource() == "serviceaccounts" {
 		return nil
 	}
-	if a.GetUserName() == "alice" && a.GetVerb() != "impersonate" {
+	if a.GetUser() != nil && a.GetUser().GetName() == "alice" && a.GetVerb() != "impersonate" {
 		return nil
 	}
 	// bob can impersonate anyone, but that it
-	if a.GetUserName() == "bob" && a.GetVerb() == "impersonate" {
+	if a.GetUser() != nil && a.GetUser().GetName() == "bob" && a.GetVerb() == "impersonate" {
 		return nil
 	}
 	// service accounts can do everything
-	if strings.HasPrefix(a.GetUserName(), serviceaccount.ServiceAccountUsernamePrefix) {
+	if a.GetUser() != nil && strings.HasPrefix(a.GetUser().GetName(), serviceaccount.ServiceAccountUsernamePrefix) {
 		return nil
 	}
 

test/integration/serviceaccount/service_account_test.go

@@ -370,7 +370,10 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie
 	// 2. ServiceAccounts named "ro" are allowed read-only operations in their namespace
 	// 3. ServiceAccounts named "rw" are allowed any operation in their namespace
 	authorizer := authorizer.AuthorizerFunc(func(attrs authorizer.Attributes) error {
-		username := attrs.GetUserName()
+		username := ""
+		if user := attrs.GetUser(); user != nil {
+			username = user.GetName()
+		}
 		ns := attrs.GetNamespace()
 
 		// If the user is "root"...