@@ -1,6 +1,8 @@

 package authorizer

 import (

+	"errors"

+

 	kapi "k8s.io/kubernetes/pkg/api"

 	"k8s.io/kubernetes/pkg/auth/user"

 	kerrors "k8s.io/kubernetes/pkg/util/errors"

@@ -21,36 +23,20 @@ func NewAuthorizer(ruleResolver rulevalidation.AuthorizationRuleResolver, forbid

 func (a *openshiftAuthorizer) Authorize(ctx kapi.Context, passedAttributes Action) (bool, string, error) {

 	attributes := CoerceToDefaultAuthorizationAttributes(passedAttributes)

-	// keep track of errors in case we are unable to authorize the action.

-	// It is entirely possible to get an error and be able to continue determine authorization status in spite of it.

-	// This is most common when a bound role is missing, but enough roles are still present and bound to authorize the request.

-	errs := []error{}

-

-	masterContext := kapi.WithNamespace(ctx, kapi.NamespaceNone)

-	globalAllowed, globalReason, err := a.authorizeWithNamespaceRules(masterContext, attributes)

-	if globalAllowed {

-		return true, globalReason, nil

-	}

-	if err != nil {

-		errs = append(errs, err)

+	user, ok := kapi.UserFrom(ctx)

+	if !ok {

+		return false, "", errors.New("no user available on context")

 	}

-

 	namespace, _ := kapi.NamespaceFrom(ctx)

-	if len(namespace) != 0 {

-		namespaceAllowed, namespaceReason, err := a.authorizeWithNamespaceRules(ctx, attributes)

-		if namespaceAllowed {

-			return true, namespaceReason, nil

-		}

-		if err != nil {

-			errs = append(errs, err)

-		}

+	allowed, reason, err := a.authorizeWithNamespaceRules(user, namespace, attributes)

+	if allowed {

+		return true, reason, nil

 	}

-

-	if len(errs) > 0 {

-		return false, "", kerrors.NewAggregate(errs)

+	// errors are allowed to occur

+	if err != nil {

+		return false, "", err

 	}

-	user, _ := kapi.UserFrom(ctx)

 	denyReason, err := a.forbiddenMessageMaker.MakeMessage(MessageContext{user, namespace, attributes})

 	if err != nil {

 		denyReason = err.Error()

@@ -64,37 +50,18 @@ func (a *openshiftAuthorizer) Authorize(ctx kapi.Context, passedAttributes Actio

 // This is done because policy rules are purely additive and policy determinations

 // can be made on the basis of those rules that are found.

 func (a *openshiftAuthorizer) GetAllowedSubjects(ctx kapi.Context, attributes Action) (sets.String, sets.String, error) {

-	errs := []error{}

-

-	masterContext := kapi.WithNamespace(ctx, kapi.NamespaceNone)

-	globalUsers, globalGroups, err := a.getAllowedSubjectsFromNamespaceBindings(masterContext, attributes)

-	if err != nil {

-		errs = append(errs, err)

-	}

-	localUsers, localGroups, err := a.getAllowedSubjectsFromNamespaceBindings(ctx, attributes)

-	if err != nil {

-		errs = append(errs, err)

-	}

-

-	users := sets.String{}

-	users.Insert(globalUsers.List()...)

-	users.Insert(localUsers.List()...)

-

-	groups := sets.String{}

-	groups.Insert(globalGroups.List()...)

-	groups.Insert(localGroups.List()...)

-

-	return users, groups, kerrors.NewAggregate(errs)

+	namespace, _ := kapi.NamespaceFrom(ctx)

+	return a.getAllowedSubjectsFromNamespaceBindings(namespace, attributes)

 }

-func (a *openshiftAuthorizer) getAllowedSubjectsFromNamespaceBindings(ctx kapi.Context, passedAttributes Action) (sets.String, sets.String, error) {

+func (a *openshiftAuthorizer) getAllowedSubjectsFromNamespaceBindings(namespace string, passedAttributes Action) (sets.String, sets.String, error) {

 	attributes := CoerceToDefaultAuthorizationAttributes(passedAttributes)

-	errs := []error{}

+	var errs []error

-	roleBindings, err := a.ruleResolver.GetRoleBindings(ctx)

+	roleBindings, err := a.ruleResolver.GetRoleBindings(namespace)

 	if err != nil {

-		return nil, nil, err

+		errs = append(errs, err)

 	}

 	users := sets.String{}

@@ -129,26 +96,34 @@ func (a *openshiftAuthorizer) getAllowedSubjectsFromNamespaceBindings(ctx kapi.C

 // authorizeWithNamespaceRules returns isAllowed, reason, and error.  If an error is returned, isAllowed and reason are still valid.  This seems strange

 // but errors are not always fatal to the authorization process.  It is entirely possible to get an error and be able to continue determine authorization

 // status in spite of it.  This is most common when a bound role is missing, but enough roles are still present and bound to authorize the request.

-func (a *openshiftAuthorizer) authorizeWithNamespaceRules(ctx kapi.Context, passedAttributes Action) (bool, string, error) {

+func (a *openshiftAuthorizer) authorizeWithNamespaceRules(user user.Info, namespace string, passedAttributes Action) (bool, string, error) {

 	attributes := CoerceToDefaultAuthorizationAttributes(passedAttributes)

-	allRules, ruleRetrievalError := a.ruleResolver.GetEffectivePolicyRules(ctx)

+	allRules, ruleRetrievalError := a.ruleResolver.RulesFor(user, namespace)

+	var errs []error

 	for _, rule := range allRules {

 		matches, err := attributes.RuleMatches(rule)

 		if err != nil {

-			return false, "", err

+			errs = append(errs, err)

+			continue

 		}

 		if matches {

-			namespace := kapi.NamespaceValue(ctx)

 			if len(namespace) == 0 {

 				return true, "allowed by cluster rule", nil

 			}

+			// not 100% accurate, because the rule may have been provided by a cluster rule. we no longer have

+			// this distinction upstream in practice.

 			return true, "allowed by rule in " + namespace, nil

 		}

 	}

-

-	return false, "", ruleRetrievalError

+	if len(errs) == 0 {

+		return false, "", ruleRetrievalError

+	}

+	if ruleRetrievalError != nil {

+		errs = append(errs, ruleRetrievalError)

+	}

+	return false, "", kerrors.NewAggregate(errs)

 }

 // TODO this may or may not be the behavior we want for managing rules.  As a for instance, a verb might be specified

@@ -414,7 +414,7 @@ func TestGlobalPolicyOutranksLocalPolicy(t *testing.T) {

 			Resource: "roles",

 		},

 		expectedAllowed: true,

-		expectedReason:  "allowed by cluster rule",

+		expectedReason:  "allowed by rule in adze",

 	}

 	test.clusterPolicies = newDefaultClusterPolicies()

 	test.policies = append(test.policies, newAdzePolicies()...)

@@ -23,7 +23,7 @@ func TestClusterAdminUseGroup(t *testing.T) {

 			Resource: "jobs",

 		},

 		expectedAllowed: true,

-		expectedReason:  "allowed by cluster rule",

+		expectedReason:  "allowed by rule in mallet",

 	}

 	test.clusterPolicies = newDefaultClusterPolicies()

 	test.clusterBindings = newDefaultClusterPolicyBindings()

@@ -40,7 +40,7 @@ func TestClusterReaderUseGroup(t *testing.T) {

 			Resource: "jobs",

 		},

 		expectedAllowed: true,

-		expectedReason:  "allowed by cluster rule",

+		expectedReason:  "allowed by rule in mallet",

 	}

 	test.clusterPolicies = newDefaultClusterPolicies()

 	test.clusterBindings = newDefaultClusterPolicyBindings()

@@ -74,26 +74,15 @@ func GetEffectivePolicyRules(ctx kapi.Context, ruleResolver rulevalidation.Autho

 		return nil, []error{kapierrors.NewBadRequest(fmt.Sprintf("user missing from context"))}

 	}

-	errors := []error{}

-	rules := []authorizationapi.PolicyRule{}

-

-	namespaceRules, err := ruleResolver.GetEffectivePolicyRules(ctx)

+	var errors []error

+	var rules []authorizationapi.PolicyRule

+	namespaceRules, err := ruleResolver.RulesFor(user, namespace)

 	if err != nil {

 		errors = append(errors, err)

 	}

 	for _, rule := range namespaceRules {

 		rules = append(rules, rulevalidation.BreakdownRule(rule)...)

 	}

-	if len(namespace) != 0 {

-		masterContext := kapi.WithNamespace(ctx, kapi.NamespaceNone)

-		clusterRules, err := ruleResolver.GetEffectivePolicyRules(masterContext)

-		if err != nil {

-			errors = append(errors, err)

-		}

-		for _, rule := range clusterRules {

-			rules = append(rules, rulevalidation.BreakdownRule(rule)...)

-		}

-	}

 	if scopes := user.GetExtra()[authorizationapi.ScopesKey]; len(scopes) > 0 {

 		rules, err = filterRulesByScopes(rules, scopes, namespace, clusterPolicyGetter)

@@ -1,8 +1,6 @@

 package rulevalidation

 import (

-	"errors"

-

 	kapi "k8s.io/kubernetes/pkg/api"

 	kapierror "k8s.io/kubernetes/pkg/api/errors"

 	"k8s.io/kubernetes/pkg/auth/user"

@@ -27,48 +25,57 @@ func NewDefaultRuleResolver(policyGetter client.PoliciesListerNamespacer, bindin

 }

 type AuthorizationRuleResolver interface {

-	GetRoleBindings(ctx kapi.Context) ([]authorizationinterfaces.RoleBinding, error)

+	GetRoleBindings(namespace string) ([]authorizationinterfaces.RoleBinding, error)

 	GetRole(roleBinding authorizationinterfaces.RoleBinding) (authorizationinterfaces.Role, error)

-	// GetEffectivePolicyRules returns the list of rules that apply to a given user in a given namespace and error.  If an error is returned, the slice of

+	// RulesFor returns the list of rules that apply to a given user in a given namespace and error.  If an error is returned, the slice of

 	// PolicyRules may not be complete, but it contains all retrievable rules.  This is done because policy rules are purely additive and policy determinations

 	// can be made on the basis of those rules that are found.

-	GetEffectivePolicyRules(ctx kapi.Context) ([]authorizationapi.PolicyRule, error)

+	RulesFor(info user.Info, namespace string) ([]authorizationapi.PolicyRule, error)

 }

-func (a *DefaultRuleResolver) GetRoleBindings(ctx kapi.Context) ([]authorizationinterfaces.RoleBinding, error) {

-	namespace := kapi.NamespaceValue(ctx)

+func (a *DefaultRuleResolver) GetRoleBindings(namespace string) ([]authorizationinterfaces.RoleBinding, error) {

+	clusterBindings, clusterErr := a.clusterBindingLister.List(kapi.ListOptions{})

-	if len(namespace) == 0 {

-		policyBindingList, err := a.clusterBindingLister.List(kapi.ListOptions{})

-		if err != nil {

-			return nil, err

-		}

+	var namespaceBindings *authorizationapi.PolicyBindingList

+	var namespaceErr error

+	if a.bindingLister != nil && len(namespace) > 0 {

+		namespaceBindings, namespaceErr = a.bindingLister.PolicyBindings(namespace).List(kapi.ListOptions{})

+	}

-		ret := make([]authorizationinterfaces.RoleBinding, 0, len(policyBindingList.Items))

-		for _, policyBinding := range policyBindingList.Items {

+	// return all loaded bindings

+	expect := 0

+	if clusterBindings != nil {

+		expect += len(clusterBindings.Items)

+	}

+	if namespaceBindings != nil {

+		expect += len(namespaceBindings.Items)

+	}

+	bindings := make([]authorizationinterfaces.RoleBinding, 0, expect)

+	if clusterBindings != nil {

+		for _, policyBinding := range clusterBindings.Items {

 			for _, value := range policyBinding.RoleBindings {

-				ret = append(ret, authorizationinterfaces.NewClusterRoleBindingAdapter(value))

+				bindings = append(bindings, authorizationinterfaces.NewClusterRoleBindingAdapter(value))

 			}

 		}

-		return ret, nil

 	}

-

-	if a.bindingLister == nil {

-		return nil, nil

+	if namespaceBindings != nil {

+		for _, policyBinding := range namespaceBindings.Items {

+			for _, value := range policyBinding.RoleBindings {

+				bindings = append(bindings, authorizationinterfaces.NewLocalRoleBindingAdapter(value))

+			}

+		}

 	}

-	policyBindingList, err := a.bindingLister.PolicyBindings(namespace).List(kapi.ListOptions{})

-	if err != nil {

-		return nil, err

+	// return all errors

+	var errs []error

+	if clusterErr != nil {

+		errs = append(errs, clusterErr)

 	}

-

-	ret := make([]authorizationinterfaces.RoleBinding, 0, len(policyBindingList.Items))

-	for _, policyBinding := range policyBindingList.Items {

-		for _, value := range policyBinding.RoleBindings {

-			ret = append(ret, authorizationinterfaces.NewLocalRoleBindingAdapter(value))

-		}

+	if namespaceErr != nil {

+		errs = append(errs, namespaceErr)

 	}

-	return ret, nil

+

+	return bindings, kerrors.NewAggregate(errs)

 }

 func (a *DefaultRuleResolver) GetRole(roleBinding authorizationinterfaces.RoleBinding) (authorizationinterfaces.Role, error) {

@@ -113,20 +120,17 @@ func (a *DefaultRuleResolver) GetRole(roleBinding authorizationinterfaces.RoleBi

 }

-// GetEffectivePolicyRules returns the list of rules that apply to a given user in a given namespace and error.  If an error is returned, the slice of

+// RulesFor returns the list of rules that apply to a given user in a given namespace and error.  If an error is returned, the slice of

 // PolicyRules may not be complete, but it contains all retrievable rules.  This is done because policy rules are purely additive and policy determinations

 // can be made on the basis of those rules that are found.

-func (a *DefaultRuleResolver) GetEffectivePolicyRules(ctx kapi.Context) ([]authorizationapi.PolicyRule, error) {

-	roleBindings, err := a.GetRoleBindings(ctx)

+func (a *DefaultRuleResolver) RulesFor(user user.Info, namespace string) ([]authorizationapi.PolicyRule, error) {

+	var errs []error

+

+	roleBindings, err := a.GetRoleBindings(namespace)

 	if err != nil {

-		return nil, err

-	}

-	user, exists := kapi.UserFrom(ctx)

-	if !exists {

-		return nil, errors.New("user missing from context")

+		errs = append(errs, err)

 	}

-	errs := []error{}

 	rules := make([]authorizationapi.PolicyRule, 0, len(roleBindings))

 	for _, roleBinding := range roleBindings {

 		if !roleBinding.AppliesToUser(user) {

@@ -146,6 +150,7 @@ func (a *DefaultRuleResolver) GetEffectivePolicyRules(ctx kapi.Context) ([]autho

 	return rules, kerrors.NewAggregate(errs)

 }

+

 func appliesToUser(ruleUsers, ruleGroups sets.String, user user.Info) bool {

 	if ruleUsers.Has(user.GetName()) {

 		return true

@@ -11,53 +11,46 @@ import (

 	kapierrors "k8s.io/kubernetes/pkg/api/errors"

 	"k8s.io/kubernetes/pkg/api/unversioned"

-	authorizationapi "github.com/openshift/origin/pkg/authorization/api"

 	authorizationinterfaces "github.com/openshift/origin/pkg/authorization/interfaces"

 )

 func ConfirmNoEscalation(ctx kapi.Context, resource unversioned.GroupResource, name string, ruleResolver AuthorizationRuleResolver, role authorizationinterfaces.Role) error {

-	ruleResolutionErrors := []error{}

+	var ruleResolutionErrors []error

-	ownerLocalRules, err := ruleResolver.GetEffectivePolicyRules(ctx)

-	if err != nil {

-		// do not fail in this case.  Rules are purely additive, so we can continue with a coverage check based on the rules we have

-		user, _ := kapi.UserFrom(ctx)

-		glog.V(1).Infof("non-fatal error getting local rules for %v: %v", user, err)

-		ruleResolutionErrors = append(ruleResolutionErrors, err)

+	user, ok := kapi.UserFrom(ctx)

+	if !ok {

+		return kapierrors.NewForbidden(resource, name, fmt.Errorf("no user provided in context"))

 	}

-	masterContext := kapi.WithNamespace(ctx, "")

-	ownerGlobalRules, err := ruleResolver.GetEffectivePolicyRules(masterContext)

+	namespace, _ := kapi.NamespaceFrom(ctx)

+

+	ownerRules, err := ruleResolver.RulesFor(user, namespace)

 	if err != nil {

 		// do not fail in this case.  Rules are purely additive, so we can continue with a coverage check based on the rules we have

-		user, _ := kapi.UserFrom(ctx)

-		glog.V(1).Infof("non-fatal error getting global rules for %v: %v", user, err)

+		glog.V(1).Infof("non-fatal error getting rules for %v: %v", user, err)

 		ruleResolutionErrors = append(ruleResolutionErrors, err)

 	}

-	ownerRules := make([]authorizationapi.PolicyRule, 0, len(ownerGlobalRules)+len(ownerLocalRules))

-	ownerRules = append(ownerRules, ownerLocalRules...)

-	ownerRules = append(ownerRules, ownerGlobalRules...)

-

 	ownerRightsCover, missingRights := Covers(ownerRules, role.Rules())

-	if !ownerRightsCover {

-		if compactedMissingRights, err := CompactRules(missingRights); err == nil {

-			missingRights = compactedMissingRights

-		}

-

-		missingRightsStrings := make([]string, 0, len(missingRights))

-		for _, missingRight := range missingRights {

-			missingRightsStrings = append(missingRightsStrings, missingRight.CompactString())

-		}

-		sort.Strings(missingRightsStrings)

-		user, _ := kapi.UserFrom(ctx)

-		var internalErr error

-		if len(ruleResolutionErrors) > 0 {

-			internalErr = fmt.Errorf("user %q cannot grant extra privileges:\n%v\nrule resolution errors: %v)", user.GetName(), strings.Join(missingRightsStrings, "\n"), ruleResolutionErrors)

-		} else {

-			internalErr = fmt.Errorf("user %q cannot grant extra privileges:\n%v", user.GetName(), strings.Join(missingRightsStrings, "\n"))

-		}

-		return kapierrors.NewForbidden(resource, name, internalErr)

+	if ownerRightsCover {

+		return nil

 	}

-	return nil

+	// determine what resources the user is missing

+	if compactedMissingRights, err := CompactRules(missingRights); err == nil {

+		missingRights = compactedMissingRights

+	}

+

+	missingRightsStrings := make([]string, 0, len(missingRights))

+	for _, missingRight := range missingRights {

+		missingRightsStrings = append(missingRightsStrings, missingRight.CompactString())

+	}

+	sort.Strings(missingRightsStrings)

+

+	var internalErr error

+	if len(ruleResolutionErrors) > 0 {

+		internalErr = fmt.Errorf("user %q cannot grant extra privileges:\n%v\nrule resolution errors: %v)", user.GetName(), strings.Join(missingRightsStrings, "\n"), ruleResolutionErrors)

+	} else {

+		internalErr = fmt.Errorf("user %q cannot grant extra privileges:\n%v", user.GetName(), strings.Join(missingRightsStrings, "\n"))

+	}

+	return kapierrors.NewForbidden(resource, name, internalErr)

 }

@@ -730,7 +730,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) {

 		},

 		response: authorizationapi.SubjectAccessReviewResponse{

 			Allowed:   true,

-			Reason:    "allowed by cluster rule",

+			Reason:    "allowed by rule in any-project",

 			Namespace: "any-project",

 		},

 	}.run(t)

@@ -742,7 +742,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) {

 		},

 		response: authorizationapi.SubjectAccessReviewResponse{

 			Allowed:   true,

-			Reason:    "allowed by cluster rule",

+			Reason:    "allowed by rule in any-project",

 			Namespace: "any-project",

 		},

 	}.run(t)

@@ -754,7 +754,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) {

 		},

 		response: authorizationapi.SubjectAccessReviewResponse{

 			Allowed:   true,

-			Reason:    "allowed by cluster rule",

+			Reason:    "allowed by rule in any-project",

 			Namespace: "any-project",

 		},

 	}.run(t)

@@ -766,7 +766,7 @@ func TestAuthorizationSubjectAccessReviewAPIGroup(t *testing.T) {

 		},

 		response: authorizationapi.SubjectAccessReviewResponse{

 			Allowed:   true,

-			Reason:    "allowed by cluster rule",

+			Reason:    "allowed by rule in any-project",

 			Namespace: "any-project",

 		},

 	}.run(t)