add rule to allow self-subject access reviews

pkg/authorization/api/register.go

@@ -18,6 +18,8 @@ func init() {
 		&PolicyBindingList{},
 		&RoleBindingList{},
 		&RoleList{},
+
+		&IsPersonalSubjectAccessReview{},
 	)
 }
 
@@ -33,3 +35,5 @@ func (*PolicyList) IsAnAPIObject()                   {}
 func (*PolicyBindingList) IsAnAPIObject()            {}
 func (*RoleBindingList) IsAnAPIObject()              {}
 func (*RoleList) IsAnAPIObject()                     {}
+
+func (*IsPersonalSubjectAccessReview) IsAnAPIObject() {}

pkg/authorization/api/types.go

@@ -83,6 +83,11 @@ type PolicyRule struct {
 	NonResourceURLs kutil.StringSet
 }
 
+// IsPersonalSubjectAccessReview is a marker for PolicyRule.AttributeRestrictions that denotes that subjectaccessreviews on self should be allowed
+type IsPersonalSubjectAccessReview struct {
+	kapi.TypeMeta
+}
+
 // Role is a logical grouping of PolicyRules that can be referenced as a unit by RoleBindings.
 type Role struct {
 	kapi.TypeMeta

pkg/authorization/api/v1beta1/register.go

@@ -18,6 +18,8 @@ func init() {
 		&PolicyBindingList{},
 		&RoleBindingList{},
 		&RoleList{},
+
+		&IsPersonalSubjectAccessReview{},
 	)
 }
 
@@ -33,3 +35,5 @@ func (*PolicyList) IsAnAPIObject()                   {}
 func (*PolicyBindingList) IsAnAPIObject()            {}
 func (*RoleBindingList) IsAnAPIObject()              {}
 func (*RoleList) IsAnAPIObject()                     {}
+
+func (*IsPersonalSubjectAccessReview) IsAnAPIObject() {}

pkg/authorization/api/v1beta1/types.go

@@ -20,7 +20,7 @@ type PolicyRule struct {
 	Verbs []string `json:"verbs"`
 	// AttributeRestrictions will vary depending on what the Authorizer/AuthorizationAttributeBuilder pair supports.
 	// If the Authorizer does not recognize how to handle the AttributeRestrictions, the Authorizer should report an error.
-	AttributeRestrictions kruntime.RawExtension `json:"attributeRestrictions"`
+	AttributeRestrictions kruntime.RawExtension `json:"attributeRestrictions,omitempty"`
 	// ResourceKinds is a list of resources this rule applies to.  ResourceAll represents all resources.
 	// DEPRECATED
 	ResourceKinds []string `json:"resourceKinds,omitempty"`
@@ -33,6 +33,11 @@ type PolicyRule struct {
 	NonResourceURLsSlice []string `json:"nonResourceURLs,omitempty"`
 }
 
+// IsPersonalSubjectAccessReview is a marker for PolicyRule.AttributeRestrictions that denotes that subjectaccessreviews on self should be allowed
+type IsPersonalSubjectAccessReview struct {
+	kapi.TypeMeta `json:",inline"`
+}
+
 // Role is a logical grouping of PolicyRules that can be referenced as a unit by RoleBindings.
 type Role struct {
 	kapi.TypeMeta   `json:",inline"`

pkg/authorization/authorizer/attributes.go

@@ -1,6 +1,7 @@
 package authorizer
 
 import (
+	"fmt"
 	"path"
 	"strings"
 
@@ -34,6 +35,16 @@ func (a DefaultAuthorizationAttributes) RuleMatches(rule authorizationapi.Policy
 
 		if a.resourceMatches(allowedResourceTypes) {
 			if a.nameMatches(rule.ResourceNames) {
+				// this rule matches the request, so we should check the additional restrictions to be sure that it's allowed
+				if rule.AttributeRestrictions.Object != nil {
+					switch rule.AttributeRestrictions.Object.(type) {
+					case (*authorizationapi.IsPersonalSubjectAccessReview):
+						return IsPersonalAccessReview(a)
+					default:
+						return false, fmt.Errorf("unable to interpret: %#v", rule.AttributeRestrictions.Object)
+					}
+				}
+
 				return true, nil
 			}
 		}

pkg/authorization/authorizer/attributes_builder.go

@@ -40,7 +40,7 @@ func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request
 		Verb:              requestInfo.Verb,
 		Resource:          requestInfo.Resource,
 		ResourceName:      requestInfo.Name,
-		RequestAttributes: nil,
+		RequestAttributes: req,
 		NonResourceURL:    false,
 		URL:               req.URL.Path,
 	}, nil

pkg/authorization/authorizer/bootstrap_policy_test.go

@@ -5,13 +5,48 @@ import (
 
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
 
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
 )
 
+func TestInvalidRole(t *testing.T) {
+	test := &authorizeTest{
+		context: kapi.WithUser(kapi.WithNamespace(kapi.NewContext(), "mallet"), &user.DefaultInfo{Name: "Brad"}),
+		attributes: &DefaultAuthorizationAttributes{
+			Verb:     "get",
+			Resource: "buildConfigs",
+		},
+		expectedAllowed: false,
+		expectedError:   "unable to interpret:",
+	}
+	test.policies = newDefaultGlobalPolicies()
+	test.policies = append(test.policies, newInvalidExtensionPolicies()...)
+	test.bindings = newDefaultGlobalBinding()
+	test.bindings = append(test.bindings, newInvalidExtensionBindings()...)
+
+	test.test(t)
+}
+func TestInvalidRoleButRuleNotUsed(t *testing.T) {
+	test := &authorizeTest{
+		context: kapi.WithUser(kapi.WithNamespace(kapi.NewContext(), "mallet"), &user.DefaultInfo{Name: "Brad"}),
+		attributes: &DefaultAuthorizationAttributes{
+			Verb:     "update",
+			Resource: "buildConfigs",
+		},
+		expectedAllowed: true,
+		expectedReason:  "allowed by rule in mallet",
+	}
+	test.policies = newDefaultGlobalPolicies()
+	test.policies = append(test.policies, newInvalidExtensionPolicies()...)
+	test.bindings = newDefaultGlobalBinding()
+	test.bindings = append(test.bindings, newInvalidExtensionBindings()...)
+
+	test.test(t)
+}
 func TestViewerGetAllowedKindInMallet(t *testing.T) {
 	test := &authorizeTest{
 		context: kapi.WithUser(kapi.WithNamespace(kapi.NewContext(), "mallet"), &user.DefaultInfo{Name: "Victor"}),
@@ -418,6 +453,57 @@ func newMalletBindings() []authorizationapi.PolicyBinding {
 		},
 	}
 }
+func newInvalidExtensionPolicies() []authorizationapi.Policy {
+	return []authorizationapi.Policy{
+		{
+			ObjectMeta: kapi.ObjectMeta{
+				Name:      authorizationapi.PolicyName,
+				Namespace: "mallet",
+			},
+			Roles: map[string]authorizationapi.Role{
+				"badExtension": {
+					ObjectMeta: kapi.ObjectMeta{
+						Name:      "failure",
+						Namespace: "mallet",
+					},
+					Rules: []authorizationapi.PolicyRule{
+						{
+							Verbs:                 util.NewStringSet("watch", "list", "get"),
+							Resources:             util.NewStringSet("buildConfigs"),
+							AttributeRestrictions: runtime.EmbeddedObject{&authorizationapi.Role{}},
+						},
+						{
+							Verbs:     util.NewStringSet("update"),
+							Resources: util.NewStringSet("buildConfigs"),
+						},
+					},
+				},
+			},
+		}}
+}
+func newInvalidExtensionBindings() []authorizationapi.PolicyBinding {
+	return []authorizationapi.PolicyBinding{
+		{
+			ObjectMeta: kapi.ObjectMeta{
+				Name:      "mallet",
+				Namespace: "mallet",
+			},
+			RoleBindings: map[string]authorizationapi.RoleBinding{
+				"borked": {
+					ObjectMeta: kapi.ObjectMeta{
+						Name:      "borked",
+						Namespace: "mallet",
+					},
+					RoleRef: kapi.ObjectReference{
+						Name:      "badExtension",
+						Namespace: "mallet",
+					},
+					Users: util.NewStringSet("Brad"),
+				},
+			},
+		},
+	}
+}
 
 func GetBootstrapPolicy(masterNamespace string) *authorizationapi.Policy {
 	policy := &authorizationapi.Policy{

pkg/authorization/authorizer/personal_subjectaccessreview.go

@@ -0,0 +1,37 @@
+package authorizer
+
+import (
+	"bytes"
+	"errors"
+	"io/ioutil"
+	"net/http"
+
+	"github.com/openshift/origin/pkg/api/latest"
+	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
+)
+
+func IsPersonalAccessReview(a AuthorizationAttributes) (bool, error) {
+	req, ok := a.GetRequestAttributes().(*http.Request)
+	if !ok {
+		return false, errors.New("expected request, but did not get one")
+	}
+
+	// TODO once we're integrated with the api installer, we should have direct access to the deserialized content
+	// for now, this only happens on subjectaccessreviews with a personal check, pay the double retrieve and decode cost
+	body, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		return false, err
+	}
+	req.Body = ioutil.NopCloser(bytes.NewBuffer(body))
+
+	subjectAccessReview := &authorizationapi.SubjectAccessReview{}
+	if err := latest.Codec.DecodeInto(body, subjectAccessReview); err != nil {
+		return false, err
+	}
+
+	if (len(subjectAccessReview.User) == 0) && (len(subjectAccessReview.Groups) == 0) {
+		return true, nil
+	}
+
+	return false, nil
+}

pkg/cmd/server/bootstrappolicy/policy.go

@@ -2,6 +2,7 @@ package bootstrappolicy
 
 import (
 	kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
@@ -102,6 +103,7 @@ func GetBootstrapMasterRoles(masterNamespace string) []authorizationapi.Role {
 			Rules: []authorizationapi.PolicyRule{
 				{Verbs: util.NewStringSet("get"), Resources: util.NewStringSet("users"), ResourceNames: util.NewStringSet("~")},
 				{Verbs: util.NewStringSet("list"), Resources: util.NewStringSet("projects")},
+				{Verbs: util.NewStringSet("create"), Resources: util.NewStringSet("subjectaccessreviews"), AttributeRestrictions: runtime.EmbeddedObject{&authorizationapi.IsPersonalSubjectAccessReview{}}},
 			},
 		},
 		{

test/integration/bootstrap_policy_test.go

@@ -10,6 +10,7 @@ import (
 	kapierror "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
 	"github.com/openshift/origin/pkg/client"
@@ -106,3 +107,56 @@ func TestOverwritePolicyCommand(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}
 }
+
+func TestSelfSubjectAccessReviews(t *testing.T) {
+	_, clusterAdminKubeConfig, err := testutil.StartTestMaster()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	valerieClientConfig := *clusterAdminClientConfig
+	valerieClientConfig.Username = ""
+	valerieClientConfig.Password = ""
+	valerieClientConfig.BearerToken = ""
+	valerieClientConfig.CertFile = ""
+	valerieClientConfig.KeyFile = ""
+	valerieClientConfig.CertData = nil
+	valerieClientConfig.KeyData = nil
+
+	accessToken, err := tokencmd.RequestToken(&valerieClientConfig, nil, "valerie", "security!")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	valerieClientConfig.BearerToken = accessToken
+	valerieOpenshiftClient, err := client.New(&valerieClientConfig)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// can I get a subjectaccessreview on myself even if I have no rights to do it generally
+	askCanICreatePolicyBindings := &authorizationapi.SubjectAccessReview{Verb: "create", Resource: "policybindings"}
+	subjectAccessReviewTest{
+		clientInterface: valerieOpenshiftClient.SubjectAccessReviews("openshift"),
+		review:          askCanICreatePolicyBindings,
+		response: authorizationapi.SubjectAccessReviewResponse{
+			Allowed:   false,
+			Reason:    "denied by default",
+			Namespace: "openshift",
+		},
+	}.run(t)
+
+	// I shouldn't be allowed to ask whether someone else can perform an action
+	askCanClusterAdminsCreateProject := &authorizationapi.SubjectAccessReview{Groups: util.NewStringSet("system:cluster-admins"), Verb: "create", Resource: "projects"}
+	subjectAccessReviewTest{
+		clientInterface: valerieOpenshiftClient.SubjectAccessReviews("openshift"),
+		review:          askCanClusterAdminsCreateProject,
+		err:             "Forbidden",
+	}.run(t)
+
+}