Merge pull request #26753 from ericchiang/rbac-authorizer-tests

pkg/apis/rbac/validation/rulevalidation.go

@@ -17,11 +17,12 @@ limitations under the License.
 package validation
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/api/errors"
+	apierrors "k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 	"k8s.io/kubernetes/pkg/auth/user"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
@@ -66,7 +67,7 @@ func ConfirmNoEscalation(ctx api.Context, ruleResolver AuthorizationRuleResolver
 	ownerRightsCover, missingRights := Covers(ownerRules, rules)
 	if !ownerRightsCover {
 		user, _ := api.UserFrom(ctx)
-		return errors.NewUnauthorized(fmt.Sprintf("attempt to grant extra privileges: %v user=%v ownerrules=%v ruleResolutionErrors=%v", missingRights, user, ownerRules, ruleResolutionErrors))
+		return apierrors.NewUnauthorized(fmt.Sprintf("attempt to grant extra privileges: %v user=%v ownerrules=%v ruleResolutionErrors=%v", missingRights, user, ownerRules, ruleResolutionErrors))
 	}
 	return nil
 }
@@ -206,3 +207,79 @@ func appliesToUser(user user.Info, subject rbac.Subject) (bool, error) {
 		return false, fmt.Errorf("unknown subject kind: %s", subject.Kind)
 	}
 }
+
+// NewTestRuleResolver returns a rule resolver from lists of role objects.
+func NewTestRuleResolver(roles []rbac.Role, roleBindings []rbac.RoleBinding, clusterRoles []rbac.ClusterRole, clusterRoleBindings []rbac.ClusterRoleBinding) AuthorizationRuleResolver {
+	r := staticRoles{
+		roles:               roles,
+		roleBindings:        roleBindings,
+		clusterRoles:        clusterRoles,
+		clusterRoleBindings: clusterRoleBindings,
+	}
+	return newMockRuleResolver(&r)
+}
+
+func newMockRuleResolver(r *staticRoles) AuthorizationRuleResolver {
+	return NewDefaultRuleResolver(r, r, r, r)
+}
+
+type staticRoles struct {
+	roles               []rbac.Role
+	roleBindings        []rbac.RoleBinding
+	clusterRoles        []rbac.ClusterRole
+	clusterRoleBindings []rbac.ClusterRoleBinding
+}
+
+func (r *staticRoles) GetRole(ctx api.Context, id string) (*rbac.Role, error) {
+	namespace, ok := api.NamespaceFrom(ctx)
+	if !ok || namespace == "" {
+		return nil, errors.New("must provide namespace when getting role")
+	}
+	for _, role := range r.roles {
+		if role.Namespace == namespace && role.Name == id {
+			return &role, nil
+		}
+	}
+	return nil, errors.New("role not found")
+}
+
+func (r *staticRoles) GetClusterRole(ctx api.Context, id string) (*rbac.ClusterRole, error) {
+	namespace, ok := api.NamespaceFrom(ctx)
+	if ok && namespace != "" {
+		return nil, errors.New("cannot provide namespace when getting cluster role")
+	}
+	for _, clusterRole := range r.clusterRoles {
+		if clusterRole.Namespace == namespace && clusterRole.Name == id {
+			return &clusterRole, nil
+		}
+	}
+	return nil, errors.New("role not found")
+}
+
+func (r *staticRoles) ListRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.RoleBindingList, error) {
+	namespace, ok := api.NamespaceFrom(ctx)
+	if !ok || namespace == "" {
+		return nil, errors.New("must provide namespace when listing role bindings")
+	}
+
+	roleBindingList := new(rbac.RoleBindingList)
+	for _, roleBinding := range r.roleBindings {
+		if roleBinding.Namespace != namespace {
+			continue
+		}
+		// TODO(ericchiang): need to implement label selectors?
+		roleBindingList.Items = append(roleBindingList.Items, roleBinding)
+	}
+	return roleBindingList, nil
+}
+
+func (r *staticRoles) ListClusterRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.ClusterRoleBindingList, error) {
+	namespace, ok := api.NamespaceFrom(ctx)
+	if ok && namespace != "" {
+		return nil, errors.New("cannot list cluster role bindings from within a namespace")
+	}
+	clusterRoleBindings := new(rbac.ClusterRoleBindingList)
+	clusterRoleBindings.Items = make([]rbac.ClusterRoleBinding, len(r.clusterRoleBindings))
+	copy(clusterRoleBindings.Items, r.clusterRoleBindings)
+	return clusterRoleBindings, nil
+}

pkg/apis/rbac/validation/rulevalidation_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package validation
 
 import (
-	"errors"
 	"hash/fnv"
 	"io"
 	"reflect"
@@ -30,71 +29,6 @@ import (
 	"k8s.io/kubernetes/pkg/util/diff"
 )
 
-func newMockRuleResolver(r *staticRoles) AuthorizationRuleResolver {
-	return NewDefaultRuleResolver(r, r, r, r)
-}
-
-type staticRoles struct {
-	roles               []rbac.Role
-	roleBindings        []rbac.RoleBinding
-	clusterRoles        []rbac.ClusterRole
-	clusterRoleBindings []rbac.ClusterRoleBinding
-}
-
-func (r *staticRoles) GetRole(ctx api.Context, id string) (*rbac.Role, error) {
-	namespace, ok := api.NamespaceFrom(ctx)
-	if !ok || namespace == "" {
-		return nil, errors.New("must provide namespace when getting role")
-	}
-	for _, role := range r.roles {
-		if role.Namespace == namespace && role.Name == id {
-			return &role, nil
-		}
-	}
-	return nil, errors.New("role not found")
-}
-
-func (r *staticRoles) GetClusterRole(ctx api.Context, id string) (*rbac.ClusterRole, error) {
-	namespace, ok := api.NamespaceFrom(ctx)
-	if ok && namespace != "" {
-		return nil, errors.New("cannot provide namespace when getting cluster role")
-	}
-	for _, clusterRole := range r.clusterRoles {
-		if clusterRole.Namespace == namespace && clusterRole.Name == id {
-			return &clusterRole, nil
-		}
-	}
-	return nil, errors.New("role not found")
-}
-
-func (r *staticRoles) ListRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.RoleBindingList, error) {
-	namespace, ok := api.NamespaceFrom(ctx)
-	if !ok || namespace == "" {
-		return nil, errors.New("must provide namespace when listing role bindings")
-	}
-
-	roleBindingList := new(rbac.RoleBindingList)
-	for _, roleBinding := range r.roleBindings {
-		if roleBinding.Namespace != namespace {
-			continue
-		}
-		// TODO(ericchiang): need to implement label selectors?
-		roleBindingList.Items = append(roleBindingList.Items, roleBinding)
-	}
-	return roleBindingList, nil
-}
-
-func (r *staticRoles) ListClusterRoleBindings(ctx api.Context, options *api.ListOptions) (*rbac.ClusterRoleBindingList, error) {
-	namespace, ok := api.NamespaceFrom(ctx)
-	if ok && namespace != "" {
-		return nil, errors.New("cannot list cluster role bindings from within a namespace")
-	}
-	clusterRoleBindings := new(rbac.ClusterRoleBindingList)
-	clusterRoleBindings.Items = make([]rbac.ClusterRoleBinding, len(r.clusterRoleBindings))
-	copy(clusterRoleBindings.Items, r.clusterRoleBindings)
-	return clusterRoleBindings, nil
-}
-
 // compute a hash of a policy rule so we can sort in a deterministic order
 func hashOf(p rbac.PolicyRule) string {
 	hash := fnv.New32()

pkg/apiserver/authz.go

@@ -143,16 +143,13 @@ func NewAuthorizerFromAuthorizationConfig(authorizationModes []string, config Au
 			}
 			authorizers = append(authorizers, webhookAuthorizer)
 		case ModeRBAC:
-			rbacAuthorizer, err := rbac.New(
+			rbacAuthorizer := rbac.New(
 				config.RBACRoleRegistry,
 				config.RBACRoleBindingRegistry,
 				config.RBACClusterRoleRegistry,
 				config.RBACClusterRoleBindingRegistry,
 				config.RBACSuperUser,
 			)
-			if err != nil {
-				return nil, err
-			}
 			authorizers = append(authorizers, rbacAuthorizer)
 		default:
 			return nil, fmt.Errorf("Unknown authorization mode %s specified", authorizationMode)

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

@@ -65,7 +65,7 @@ func (r *RBACAuthorizer) Authorize(attr authorizer.Attributes) error {
 	return validation.ConfirmNoEscalation(ctx, r.authorizationRuleResolver, []rbac.PolicyRule{requestedRule})
 }
 
-func New(roleRegistry role.Registry, roleBindingRegistry rolebinding.Registry, clusterRoleRegistry clusterrole.Registry, clusterRoleBindingRegistry clusterrolebinding.Registry, superUser string) (*RBACAuthorizer, error) {
+func New(roleRegistry role.Registry, roleBindingRegistry rolebinding.Registry, clusterRoleRegistry clusterrole.Registry, clusterRoleBindingRegistry clusterrolebinding.Registry, superUser string) *RBACAuthorizer {
 	authorizer := &RBACAuthorizer{
 		superUser: superUser,
 		authorizationRuleResolver: validation.NewDefaultRuleResolver(
@@ -75,5 +75,5 @@ func New(roleRegistry role.Registry, roleBindingRegistry rolebinding.Registry, c
 			clusterRoleBindingRegistry,
 		),
 	}
-	return authorizer, nil
+	return authorizer
 }

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

@@ -17,43 +17,129 @@ limitations under the License.
 package rbac
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
-	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/rbac"
-	"k8s.io/kubernetes/pkg/registry/clusterrole"
-	clusterroleetcd "k8s.io/kubernetes/pkg/registry/clusterrole/etcd"
-	"k8s.io/kubernetes/pkg/registry/clusterrolebinding"
-	clusterrolebindingetcd "k8s.io/kubernetes/pkg/registry/clusterrolebinding/etcd"
-	"k8s.io/kubernetes/pkg/registry/generic"
-	"k8s.io/kubernetes/pkg/registry/role"
-	roleetcd "k8s.io/kubernetes/pkg/registry/role/etcd"
-	"k8s.io/kubernetes/pkg/registry/rolebinding"
-	rolebindingetcd "k8s.io/kubernetes/pkg/registry/rolebinding/etcd"
-	"k8s.io/kubernetes/pkg/storage/etcd"
-	"k8s.io/kubernetes/pkg/storage/etcd/etcdtest"
-	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
+	"k8s.io/kubernetes/pkg/apis/rbac/validation"
+	"k8s.io/kubernetes/pkg/auth/authorizer"
 )
 
-func TestNew(t *testing.T) {
-	// NOTE(ericchiang): Can't get this strategy to do reads. Get cryptic "client: etcd cluster is unavailable or misconfigured"
-	// Writes work fine, so use to test storing initial data.
-	server := etcdtesting.NewEtcdTestClientServer(t)
-	defer server.Terminate(t)
-
-	codec := testapi.Groups[rbac.GroupName].StorageCodec()
-	getRESTOptions := func(resource string) generic.RESTOptions {
-		cacheSize := etcdtest.DeserializationCacheSize
-		storage := etcd.NewEtcdStorage(server.Client, codec, resource, false, cacheSize)
-		return generic.RESTOptions{Storage: storage, Decorator: generic.UndecoratedStorage}
+func newRule(verbs, apiGroups, resources string) rbac.PolicyRule {
+	return rbac.PolicyRule{
+		Verbs:     strings.Split(verbs, ","),
+		APIGroups: strings.Split(apiGroups, ","),
+		Resources: strings.Split(resources, ","),
 	}
+}
+
+func newRole(name, namespace string, rules ...rbac.PolicyRule) rbac.Role {
+	return rbac.Role{ObjectMeta: api.ObjectMeta{Namespace: namespace, Name: name}, Rules: rules}
+}
+
+func newClusterRole(name string, rules ...rbac.PolicyRule) rbac.ClusterRole {
+	return rbac.ClusterRole{ObjectMeta: api.ObjectMeta{Name: name}, Rules: rules}
+}
+
+const (
+	bindToRole        uint16 = 0x0
+	bindToClusterRole uint16 = 0x1
+)
+
+func newRoleBinding(namespace, roleName string, bindType uint16, subjects ...string) rbac.RoleBinding {
+	r := rbac.RoleBinding{ObjectMeta: api.ObjectMeta{Namespace: namespace}}
+
+	switch bindType {
+	case bindToRole:
+		r.RoleRef = api.ObjectReference{Kind: "Role", Namespace: namespace, Name: roleName}
+	case bindToClusterRole:
+		r.RoleRef = api.ObjectReference{Kind: "ClusterRole", Name: roleName}
+	}
+
+	r.Subjects = make([]rbac.Subject, len(subjects))
+	for i, subject := range subjects {
+		split := strings.SplitN(subject, ":", 2)
+		r.Subjects[i].Kind, r.Subjects[i].Name = split[0], split[1]
+	}
+	return r
+}
+
+type defaultAttributes struct {
+	user      string
+	groups    string
+	verb      string
+	resource  string
+	namespace string
+	apiGroup  string
+}
+
+func (d *defaultAttributes) String() string {
+	return fmt.Sprintf("user=(%s), groups=(%s), verb=(%s), resource=(%s), namespace=(%s), apiGroup=(%s)",
+		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) GetGroups() []string     { return 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 }
+func (d *defaultAttributes) GetResource() string     { return d.resource }
+func (d *defaultAttributes) GetSubresource() string  { return "" }
+func (d *defaultAttributes) GetName() string         { return "" }
+func (d *defaultAttributes) GetAPIGroup() string     { return d.apiGroup }
+func (d *defaultAttributes) GetAPIVersion() string   { return "" }
+func (d *defaultAttributes) IsResourceRequest() bool { return true }
+func (d *defaultAttributes) GetPath() string         { return "" }
+
+func TestAuthorizer(t *testing.T) {
+	tests := []struct {
+		roles               []rbac.Role
+		roleBindings        []rbac.RoleBinding
+		clusterRoles        []rbac.ClusterRole
+		clusterRoleBindings []rbac.ClusterRoleBinding
+
+		superUser string
+
+		shouldPass []authorizer.Attributes
+		shouldFail []authorizer.Attributes
+	}{
+		{
+			clusterRoles: []rbac.ClusterRole{
+				newClusterRole("admin", newRule("*", "*", "*")),
+			},
+			roleBindings: []rbac.RoleBinding{
+				newRoleBinding("ns1", "admin", bindToClusterRole, "User:admin", "Group:admins"),
+			},
+			shouldPass: []authorizer.Attributes{
+				&defaultAttributes{"admin", "", "get", "Pods", "ns1", ""},
+				&defaultAttributes{"admin", "", "watch", "Pods", "ns1", ""},
+				&defaultAttributes{"admin", "group1", "watch", "Foobar", "ns1", ""},
+				&defaultAttributes{"joe", "admins", "watch", "Foobar", "ns1", ""},
+				&defaultAttributes{"joe", "group1,admins", "watch", "Foobar", "ns1", ""},
+			},
+			shouldFail: []authorizer.Attributes{
+				&defaultAttributes{"admin", "", "GET", "Pods", "ns2", ""},
+				&defaultAttributes{"admin", "", "GET", "Nodes", "", ""},
+				&defaultAttributes{"admin", "admins", "GET", "Pods", "ns2", ""},
+				&defaultAttributes{"admin", "admins", "GET", "Nodes", "", ""},
+			},
+		},
+	}
+	for i, tt := range tests {
+		ruleResolver := validation.NewTestRuleResolver(tt.roles, tt.roleBindings, tt.clusterRoles, tt.clusterRoleBindings)
+		a := RBACAuthorizer{tt.superUser, ruleResolver}
+		for _, attr := range tt.shouldPass {
+			if err := a.Authorize(attr); err != nil {
+				t.Errorf("case %d: incorrectly restricted %s: %T %v", i, attr, err, err)
+			}
+		}
 
-	roleRegistry := role.NewRegistry(roleetcd.NewREST(getRESTOptions("roles")))
-	roleBindingRegistry := rolebinding.NewRegistry(rolebindingetcd.NewREST(getRESTOptions("rolebindings")))
-	clusterRoleRegistry := clusterrole.NewRegistry(clusterroleetcd.NewREST(getRESTOptions("clusterroles")))
-	clusterRoleBindingRegistry := clusterrolebinding.NewRegistry(clusterrolebindingetcd.NewREST(getRESTOptions("clusterrolebindings")))
-	_, err := New(roleRegistry, roleBindingRegistry, clusterRoleRegistry, clusterRoleBindingRegistry, "")
-	if err != nil {
-		t.Fatalf("failed to create authorizer: %v", err)
+		for _, attr := range tt.shouldFail {
+			if err := a.Authorize(attr); err == nil {
+				t.Errorf("case %d: incorrectly passed %s", i, attr)
+			}
+		}
 	}
 }

test/integration/framework/etcd_utils.go

@@ -76,6 +76,13 @@ func NewExtensionsEtcdStorage(client etcd.Client) storage.Interface {
 	return etcdstorage.NewEtcdStorage(client, testapi.Extensions.Codec(), etcdtest.PathPrefix(), false, etcdtest.DeserializationCacheSize)
 }
 
+func NewRbacEtcdStorage(client etcd.Client) storage.Interface {
+	if client == nil {
+		client = NewEtcdClient()
+	}
+	return etcdstorage.NewEtcdStorage(client, testapi.Rbac.Codec(), etcdtest.PathPrefix(), false, etcdtest.DeserializationCacheSize)
+}
+
 func RequireEtcd() {
 	if _, err := etcd.NewKeysAPI(NewEtcdClient()).Get(context.TODO(), "/", nil); err != nil {
 		glog.Fatalf("unable to connect to etcd for testing: %v", err)