diff --git a/pkg/authorization/api/types.go b/pkg/authorization/api/types.go index f45fa5022258..353ae353c49c 100644 --- a/pkg/authorization/api/types.go +++ b/pkg/authorization/api/types.go @@ -15,9 +15,10 @@ import ( const ( // Policy is a singleton and this is its name - PolicyName = "default" - ResourceAll = "*" - VerbAll = "*" + PolicyName = "default" + ResourceAll = "*" + VerbAll = "*" + NonResourceAll = "*" ) const ( @@ -77,6 +78,9 @@ type PolicyRule struct { Resources []string // ResourceNames is an optional white list of names that the rule applies to. An empty set means that everything is allowed. ResourceNames kutil.StringSet + // NonResourceURLs is a set of partial urls that a user should have access to. *s are allowed, but only as the full, final step in the path + // If an action is not a resource API request, then the URL is split on '/' and is checked against the NonResourceURLs to look for a match. + NonResourceURLs kutil.StringSet } // Role is a logical grouping of PolicyRules that can be referenced as a unit by RoleBindings. diff --git a/pkg/authorization/api/v1beta1/conversion.go b/pkg/authorization/api/v1beta1/conversion.go index 295e76a115e7..194200efc806 100644 --- a/pkg/authorization/api/v1beta1/conversion.go +++ b/pkg/authorization/api/v1beta1/conversion.go @@ -3,10 +3,10 @@ package v1beta1 import ( "sort" - "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + newer "github.com/openshift/origin/pkg/authorization/api" ) @@ -26,6 +26,8 @@ func init() { out.ResourceNames = util.NewStringSet(in.ResourceNames...) + out.NonResourceURLs = util.NewStringSet(in.NonResourceURLsSlice...) + return nil }, func(in *newer.PolicyRule, out *PolicyRule, s conversion.Scope) error { @@ -41,6 +43,8 @@ func init() { out.ResourceNames = in.ResourceNames.List() + out.NonResourceURLsSlice = in.NonResourceURLs.List() + return nil }, func(in *Policy, out *newer.Policy, s conversion.Scope) error { diff --git a/pkg/authorization/api/v1beta1/types.go b/pkg/authorization/api/v1beta1/types.go index 4557f33ad89d..a9e249aa8d85 100644 --- a/pkg/authorization/api/v1beta1/types.go +++ b/pkg/authorization/api/v1beta1/types.go @@ -28,6 +28,9 @@ type PolicyRule struct { Resources []string `json:"resources"` // ResourceNames is an optional white list of names that the rule applies to. An empty set means that everything is allowed. ResourceNames []string `json:"resourceNames,omitempty"` + // NonResourceURLsSlice is a set of partial urls that a user should have access to. *s are allowed, but only as the full, final step in the path + // This name is intentionally different than the internal type so that the DefaultConvert works nicely and because the ordering may be different. + NonResourceURLsSlice []string `json:"nonResourceURLs,omitempty"` } // Role is a logical grouping of PolicyRules that can be referenced as a unit by RoleBindings. diff --git a/pkg/authorization/authorizer/authorizer.go b/pkg/authorization/authorizer/authorizer.go index 7b834da6cec2..e0710b36493d 100644 --- a/pkg/authorization/authorizer/authorizer.go +++ b/pkg/authorization/authorizer/authorizer.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net/http" + "path" "strings" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -28,13 +29,22 @@ type AuthorizationAttributeBuilder interface { } type AuthorizationAttributes interface { + // GetUserInfo returns the user requesting the action GetUserInfo() user.Info + // GetVerb returns the verb associated with the action. For resource requests, this verb is the logical kube verb. For non-resource requests it is the http method tolowered. GetVerb() string + // GetResource returns the resource type. If IsNonResourceURL() is true, then GetResource() is "". GetResource() string + // GetNamespace returns the namespace of a resource request. If IsNonResourceURL() is true, then GetNamespace() is "". GetNamespace() string + // GetResourceName returns the name of the resource being acted upon. Not all resource actions have one (list as a for instance). If IsNonResourceURL() is true, then GetResourceName() is "". GetResourceName() string - // GetRequestAttributes is of type interface{} because different verbs and different Authorizer/AuthorizationAttributeBuilder pairs may have different contract requirements + // GetRequestAttributes is of type interface{} because different verbs and different Authorizer/AuthorizationAttributeBuilder pairs may have different contract requirements. GetRequestAttributes() interface{} + // IsNonResourceURL returns true if this is not an action performed against the resource API + IsNonResourceURL() bool + // GetURL returns the URL split on '/'s + GetURL() string } type openshiftAuthorizer struct { @@ -54,6 +64,8 @@ type DefaultAuthorizationAttributes struct { ResourceName string Namespace string RequestAttributes interface{} + NonResourceURL bool + URL string } type openshiftAuthorizationAttributeBuilder struct { @@ -289,6 +301,8 @@ func coerceToDefaultAuthorizationAttributes(passedAttributes AuthorizationAttrib Resource: passedAttributes.GetResource(), ResourceName: passedAttributes.GetResourceName(), User: passedAttributes.GetUserInfo(), + NonResourceURL: passedAttributes.IsNonResourceURL(), + URL: passedAttributes.GetURL(), } } @@ -296,9 +310,19 @@ func coerceToDefaultAuthorizationAttributes(passedAttributes AuthorizationAttrib } func (a DefaultAuthorizationAttributes) RuleMatches(rule authorizationapi.PolicyRule) (bool, error) { - allowedResourceTypes := resolveResources(rule) + if a.IsNonResourceURL() { + if a.nonResourceMatches(rule) { + if a.verbMatches(util.NewStringSet(rule.Verbs...)) { + return true, nil + } + } + + return false, nil + } if a.verbMatches(util.NewStringSet(rule.Verbs...)) { + allowedResourceTypes := resolveResources(rule) + if a.resourceMatches(allowedResourceTypes) { if a.nameMatches(rule.ResourceNames) { return true, nil @@ -360,6 +384,35 @@ func (a DefaultAuthorizationAttributes) GetUserInfo() user.Info { func (a DefaultAuthorizationAttributes) GetVerb() string { return a.Verb } + +// nonResourceMatches take the remainer of a URL and attempts to match it against a series of explicitly allowed steps that can end in a wildcard +func (a DefaultAuthorizationAttributes) nonResourceMatches(rule authorizationapi.PolicyRule) bool { + for allowedNonResourcePath := range rule.NonResourceURLs { + // if the allowed resource path ends in a wildcard, check to see if the URL starts with it + if strings.HasSuffix(allowedNonResourcePath, "*") { + if strings.HasPrefix(a.GetURL(), allowedNonResourcePath[0:len(allowedNonResourcePath)-1]) { + return true + } + } + + // if we have an exact match, return true + if a.GetURL() == allowedNonResourcePath { + return true + } + } + + return false +} + +// splitPath returns the segments for a URL path. +func splitPath(thePath string) []string { + thePath = strings.Trim(path.Clean(thePath), "/") + if thePath == "" { + return []string{} + } + return strings.Split(thePath, "/") +} + func (a DefaultAuthorizationAttributes) GetResource() string { return a.Resource } @@ -375,16 +428,15 @@ func (a DefaultAuthorizationAttributes) GetRequestAttributes() interface{} { return a.RequestAttributes } -func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request) (AuthorizationAttributes, error) { - requestInfo, err := a.infoResolver.GetAPIRequestInfo(req) - if err != nil { - return nil, err - } +func (a DefaultAuthorizationAttributes) IsNonResourceURL() bool { + return a.NonResourceURL +} - if (requestInfo.Resource == "projects") && (len(requestInfo.Name) > 0) { - requestInfo.Namespace = requestInfo.Name - } +func (a DefaultAuthorizationAttributes) GetURL() string { + return a.URL +} +func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request) (AuthorizationAttributes, error) { ctx, ok := a.contextMapper.Get(req) if !ok { return nil, errors.New("could not get request context") @@ -394,6 +446,31 @@ func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request return nil, errors.New("could not get user") } + // any url that starts with an API prefix and is more than one step long is considered to be a resource URL. + // That means that /api is non-resource, /api/v1beta1 is resource, /healthz is non-resource, and /swagger/anything is non-resource + urlSegments := splitPath(req.URL.Path) + isResourceURL := (len(urlSegments) > 1) && a.infoResolver.APIPrefixes.Has(urlSegments[0]) + + if !isResourceURL { + return DefaultAuthorizationAttributes{ + User: userInfo, + Verb: strings.ToLower(req.Method), + NonResourceURL: true, + URL: req.URL.Path, + }, nil + } + + requestInfo, err := a.infoResolver.GetAPIRequestInfo(req) + if err != nil { + return nil, err + } + + // TODO reconsider special casing this. Having the special case hereallow us to fully share the kube + // APIRequestInfoResolver without any modification or customization. + if (requestInfo.Resource == "projects") && (len(requestInfo.Name) > 0) { + requestInfo.Namespace = requestInfo.Name + } + return DefaultAuthorizationAttributes{ User: userInfo, Verb: requestInfo.Verb, @@ -401,10 +478,11 @@ func (a *openshiftAuthorizationAttributeBuilder) GetAttributes(req *http.Request ResourceName: requestInfo.Name, Namespace: requestInfo.Namespace, RequestAttributes: nil, + NonResourceURL: false, + URL: req.URL.Path, }, nil } -// TODO enumerate all resources and verbs instead of using * func GetBootstrapPolicy(masterNamespace string) *authorizationapi.Policy { return &authorizationapi.Policy{ ObjectMeta: kapi.ObjectMeta{ @@ -424,6 +502,10 @@ func GetBootstrapPolicy(masterNamespace string) *authorizationapi.Policy { Verbs: []string{authorizationapi.VerbAll}, Resources: []string{authorizationapi.ResourceAll}, }, + { + Verbs: []string{authorizationapi.VerbAll}, + NonResourceURLs: util.NewStringSet(authorizationapi.NonResourceAll), + }, }, }, "admin": { @@ -480,6 +562,18 @@ func GetBootstrapPolicy(masterNamespace string) *authorizationapi.Policy { {Verbs: []string{"list"}, Resources: []string{"projects"}}, }, }, + "cluster-status": { + ObjectMeta: kapi.ObjectMeta{ + Name: "cluster-status", + Namespace: masterNamespace, + }, + Rules: []authorizationapi.PolicyRule{ + { + Verbs: []string{"get"}, + NonResourceURLs: util.NewStringSet("/healthz", "/version", "/api", "/osapi"), + }, + }, + }, "system:deployer": { ObjectMeta: kapi.ObjectMeta{ Name: "system:deployer", @@ -597,6 +691,17 @@ func GetBootstrapPolicyBinding(masterNamespace string) *authorizationapi.PolicyB }, GroupNames: []string{"system:authenticated", "system:unauthenticated"}, }, + "cluster-status-binding": { + ObjectMeta: kapi.ObjectMeta{ + Name: "cluster-status-binding", + Namespace: masterNamespace, + }, + RoleRef: kapi.ObjectReference{ + Name: "cluster-status", + Namespace: masterNamespace, + }, + GroupNames: []string{"system:authenticated", "system:unauthenticated"}, + }, }, } } diff --git a/pkg/authorization/authorizer/authorizer_test.go b/pkg/authorization/authorizer/authorizer_test.go index 7fa23d96c7c8..b6edb7f9abc9 100644 --- a/pkg/authorization/authorizer/authorizer_test.go +++ b/pkg/authorization/authorizer/authorizer_test.go @@ -124,7 +124,73 @@ func TestAllowedWithMissingBinding(t *testing.T) { UserNames: []string{"Anna"}, } test.namespacedPolicy, test.namespacedPolicyBinding = newAdzePolicy() + test.test(t) +} +func TestHealthAllow(t *testing.T) { + test := &authorizeTest{ + attributes: &DefaultAuthorizationAttributes{ + User: &user.DefaultInfo{ + Name: "no-one", + Groups: []string{"system:unauthenticated"}, + }, + Verb: "get", + NonResourceURL: true, + URL: "/healthz", + }, + expectedAllowed: true, + expectedReason: "allowed by rule in master", + } + test.globalPolicy, test.globalPolicyBinding = newDefaultGlobalPolicy() + test.test(t) +} + +func TestNonResourceAllow(t *testing.T) { + test := &authorizeTest{ + attributes: &DefaultAuthorizationAttributes{ + User: &user.DefaultInfo{ + Name: "ClusterAdmin", + }, + Verb: "get", + URL: "not-specified", + }, + expectedAllowed: true, + expectedReason: "allowed by rule in master", + } + test.globalPolicy, test.globalPolicyBinding = newDefaultGlobalPolicy() + test.test(t) +} + +func TestNonResourceDeny(t *testing.T) { + test := &authorizeTest{ + attributes: &DefaultAuthorizationAttributes{ + User: &user.DefaultInfo{ + Name: "no-one", + }, + Verb: "get", + URL: "not-allowed", + }, + expectedAllowed: false, + expectedReason: "denied by default", + } + test.globalPolicy, test.globalPolicyBinding = newDefaultGlobalPolicy() + test.test(t) +} + +func TestHealthDeny(t *testing.T) { + test := &authorizeTest{ + attributes: &DefaultAuthorizationAttributes{ + User: &user.DefaultInfo{ + Name: "no-one", + }, + Verb: "get", + NonResourceURL: true, + URL: "/healthz", + }, + expectedAllowed: false, + expectedReason: "denied by default", + } + test.globalPolicy, test.globalPolicyBinding = newDefaultGlobalPolicy() test.test(t) } @@ -425,12 +491,13 @@ func newDefaultGlobalPolicy() ([]authorizationapi.Policy, []authorizationapi.Pol }, }, }, + *GetBootstrapPolicyBinding(testMasterNamespace), } } func newAdzePolicy() ([]authorizationapi.Policy, []authorizationapi.PolicyBinding) { - return append(make([]authorizationapi.Policy, 0, 0), - authorizationapi.Policy{ + return []authorizationapi.Policy{ + { ObjectMeta: kapi.ObjectMeta{ Name: authorizationapi.PolicyName, Namespace: "adze", @@ -448,9 +515,9 @@ func newAdzePolicy() ([]authorizationapi.Policy, []authorizationapi.PolicyBindin }), }, }, - }), - append(make([]authorizationapi.PolicyBinding, 0, 0), - authorizationapi.PolicyBinding{ + }}, + []authorizationapi.PolicyBinding{ + { ObjectMeta: kapi.ObjectMeta{ Name: testMasterNamespace, Namespace: "adze", @@ -491,7 +558,7 @@ func newAdzePolicy() ([]authorizationapi.Policy, []authorizationapi.PolicyBindin }, }, }, - authorizationapi.PolicyBinding{ + { ObjectMeta: kapi.ObjectMeta{ Name: "adze", Namespace: "adze", @@ -510,5 +577,5 @@ func newAdzePolicy() ([]authorizationapi.Policy, []authorizationapi.PolicyBindin }, }, }, - ) + } } diff --git a/pkg/authorization/authorizer/non_resource_match_test.go b/pkg/authorization/authorizer/non_resource_match_test.go new file mode 100644 index 000000000000..4d7622840287 --- /dev/null +++ b/pkg/authorization/authorizer/non_resource_match_test.go @@ -0,0 +1,76 @@ +package authorizer + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + + authorizationapi "github.com/openshift/origin/pkg/authorization/api" +) + +type nonResourceMatchTest struct { + url string + matcher string + expectedResult bool +} + +func TestNonResourceMatchStar(t *testing.T) { + test := &nonResourceMatchTest{ + url: "first/second", + matcher: "first/*", + expectedResult: true, + } + test.run(t) +} + +func TestNonResourceMatchExact(t *testing.T) { + test := &nonResourceMatchTest{ + url: "first/second", + matcher: "first/second", + expectedResult: true, + } + test.run(t) +} + +func TestNonResourceMatchMatcherEndsShort(t *testing.T) { + test := &nonResourceMatchTest{ + url: "first/second", + matcher: "first", + expectedResult: false, + } + test.run(t) +} + +func TestNonResourceMatchURLEndsShort(t *testing.T) { + test := &nonResourceMatchTest{ + url: "first", + matcher: "first/second", + expectedResult: false, + } + test.run(t) +} + +func TestNonResourceMatchNoSimilarity(t *testing.T) { + test := &nonResourceMatchTest{ + url: "first/second", + matcher: "foo", + expectedResult: false, + } + test.run(t) +} + +func (test *nonResourceMatchTest) run(t *testing.T) { + attributes := &DefaultAuthorizationAttributes{ + NonResourceURL: true, + URL: test.url, + } + + rule := authorizationapi.PolicyRule{NonResourceURLs: util.NewStringSet(test.matcher)} + + result := attributes.nonResourceMatches(rule) + + if result != test.expectedResult { + t.Errorf("Expected %v, got %v", test.expectedResult, result) + } + +} diff --git a/pkg/authorization/authorizer/subjects_test.go b/pkg/authorization/authorizer/subjects_test.go index b3aa5de8113b..f3d2871d1ae8 100644 --- a/pkg/authorization/authorizer/subjects_test.go +++ b/pkg/authorization/authorizer/subjects_test.go @@ -27,8 +27,8 @@ func TestSubjects(t *testing.T) { Resource: "pods", Namespace: "adze", }, - expectedUsers: []string{"Anna", "ClusterAdmin", "Ellen", "Valerie"}, - expectedGroups: []string{"RootUsers"}, + expectedUsers: []string{"Anna", "ClusterAdmin", "Ellen", "Valerie", "system:admin", "system:kube-client", "system:openshift-client", "system:openshift-deployer"}, + expectedGroups: []string{"RootUsers", "system:authenticated", "system:unauthenticated"}, } globalPolicy, globalPolicyBinding := newDefaultGlobalPolicy() namespacedPolicy, namespacedPolicyBinding := newAdzePolicy()