diff --git a/pkg/authorization/api/register.go b/pkg/authorization/api/register.go index a6fa1888aa6e..405999e8d8b7 100644 --- a/pkg/authorization/api/register.go +++ b/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() {} diff --git a/pkg/authorization/api/types.go b/pkg/authorization/api/types.go index 6f3333a50b65..83a4d8afae56 100644 --- a/pkg/authorization/api/types.go +++ b/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 diff --git a/pkg/authorization/api/v1beta1/register.go b/pkg/authorization/api/v1beta1/register.go index b9af654b041e..090493eda408 100644 --- a/pkg/authorization/api/v1beta1/register.go +++ b/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() {} diff --git a/pkg/authorization/api/v1beta1/types.go b/pkg/authorization/api/v1beta1/types.go index aae3b70865ce..dd45b7da41dc 100644 --- a/pkg/authorization/api/v1beta1/types.go +++ b/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"` diff --git a/pkg/authorization/authorizer/attributes.go b/pkg/authorization/authorizer/attributes.go index cf25caff5519..4d7b311d44dc 100644 --- a/pkg/authorization/authorizer/attributes.go +++ b/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 } } diff --git a/pkg/authorization/authorizer/attributes_builder.go b/pkg/authorization/authorizer/attributes_builder.go index a6eff957eb96..b1fd94b1fa11 100644 --- a/pkg/authorization/authorizer/attributes_builder.go +++ b/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 diff --git a/pkg/authorization/authorizer/bootstrap_policy_test.go b/pkg/authorization/authorizer/bootstrap_policy_test.go index d266c5153131..2a4b3ff29798 100644 --- a/pkg/authorization/authorizer/bootstrap_policy_test.go +++ b/pkg/authorization/authorizer/bootstrap_policy_test.go @@ -5,6 +5,7 @@ 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" @@ -12,6 +13,40 @@ import ( "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{ diff --git a/pkg/authorization/authorizer/personal_subjectaccessreview.go b/pkg/authorization/authorizer/personal_subjectaccessreview.go new file mode 100644 index 000000000000..1f2c95028032 --- /dev/null +++ b/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 +} diff --git a/pkg/cmd/server/bootstrappolicy/policy.go b/pkg/cmd/server/bootstrappolicy/policy.go index 4cbcde82c857..9989643d4847 100644 --- a/pkg/cmd/server/bootstrappolicy/policy.go +++ b/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{}}}, }, }, { diff --git a/test/integration/bootstrap_policy_test.go b/test/integration/bootstrap_policy_test.go index cd3706ed270b..4ec25d41a1ab 100644 --- a/test/integration/bootstrap_policy_test.go +++ b/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) + +}