authorization changes

pkg/authorization/authorizer/authorizer.go

@@ -27,17 +27,17 @@ func NewSubjectLocator(delegate authorizerrbac.SubjectLocator) SubjectLocator {
 	return &openshiftSubjectLocator{delegate: delegate}
 }
 
-func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (bool, string, error) {
+func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) {
 	if attributes.GetUser() == nil {
-		return false, "", errors.New("no user available on context")
+		return authorizer.DecisionDeny, "", errors.New("no user available on context")
 	}
-	allowed, delegateReason, err := a.delegate.Authorize(attributes)
-	if allowed {
-		return true, reason(attributes), nil
+	authorized, delegateReason, err := a.delegate.Authorize(attributes)
+	if authorized == authorizer.DecisionAllow {
+		return authorizer.DecisionAllow, reason(attributes), nil
 	}
 	// errors are allowed to occur
 	if err != nil {
-		return false, "", err
+		return authorizer.DecisionDeny, "", err
 	}
 
 	denyReason, err := a.forbiddenMessageMaker.MakeMessage(attributes)
@@ -48,7 +48,7 @@ func (a *openshiftAuthorizer) Authorize(attributes authorizer.Attributes) (bool,
 		denyReason += ": " + delegateReason
 	}
 
-	return false, denyReason, nil
+	return authorizer.DecisionDeny, denyReason, nil
 }
 
 // GetAllowedSubjects returns the subjects it knows can perform the action.

pkg/authorization/authorizer/browsersafe/authorizer.go

@@ -24,7 +24,7 @@ func NewBrowserSafeAuthorizer(delegate authorizer.Authorizer, authenticatedGroup
 	}
 }
 
-func (a *browserSafeAuthorizer) Authorize(attributes authorizer.Attributes) (bool, string, error) {
+func (a *browserSafeAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) {
 	browserSafeAttributes := a.getBrowserSafeAttributes(attributes)
 	return a.delegate.Authorize(browserSafeAttributes)
 }

pkg/authorization/authorizer/browsersafe/authorizer_test.go

@@ -51,7 +51,7 @@ func TestBrowserSafeAuthorizer(t *testing.T) {
 		safeAuthorizer := NewBrowserSafeAuthorizer(delegateAuthorizer, "system:authenticated")
 
 		authorized, reason, err := safeAuthorizer.Authorize(tc.attributes)
-		if authorized || len(reason) != 0 || err != nil {
+		if authorized == authorizer.DecisionAllow || len(reason) != 0 || err != nil {
 			t.Errorf("%s: unexpected output: %v %s %v", name, authorized, reason, err)
 			continue
 		}
@@ -69,7 +69,7 @@ type recordingAuthorizer struct {
 	attributes authorizer.Attributes
 }
 
-func (t *recordingAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
+func (t *recordingAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 	t.attributes = a
-	return false, "", nil
+	return authorizer.DecisionDeny, "", nil
 }

pkg/authorization/authorizer/scope/authorizer.go

@@ -23,10 +23,10 @@ func NewAuthorizer(delegate authorizer.Authorizer, clusterRoleGetter rbaclisters
 	return &scopeAuthorizer{delegate: delegate, clusterRoleGetter: clusterRoleGetter, forbiddenMessageMaker: forbiddenMessageMaker}
 }
 
-func (a *scopeAuthorizer) Authorize(attributes authorizer.Attributes) (bool, string, error) {
+func (a *scopeAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) {
 	user := attributes.GetUser()
 	if user == nil {
-		return false, "", fmt.Errorf("user missing from context")
+		return authorizer.DecisionDeny, "", fmt.Errorf("user missing from context")
 	}
 
 	scopes := user.GetExtra()[authorizationapi.ScopesKey]
@@ -52,5 +52,5 @@ func (a *scopeAuthorizer) Authorize(attributes authorizer.Attributes) (bool, str
 		denyReason = err.Error()
 	}
 
-	return false, fmt.Sprintf("scopes %v prevent this action; %v", scopes, denyReason), kerrors.NewAggregate(nonFatalErrors)
+	return authorizer.DecisionDeny, fmt.Sprintf("scopes %v prevent this action; %v", scopes, denyReason), kerrors.NewAggregate(nonFatalErrors)
 }

pkg/authorization/authorizer/scope/authorizer_test.go

@@ -18,7 +18,7 @@ func TestAuthorize(t *testing.T) {
 		attributes          kauthorizer.AttributesRecord
 		delegateAuthAllowed bool
 		expectedCalled      bool
-		expectedAllowed     bool
+		expectedAllowed     kauthorizer.Decision
 		expectedErr         string
 		expectedMsg         string
 	}{
@@ -157,9 +157,12 @@ type fakeAuthorizer struct {
 	called  bool
 }
 
-func (a *fakeAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (bool, string, error) {
+func (a *fakeAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (kauthorizer.Decision, string, error) {
 	a.called = true
-	return a.allowed, "", nil
+	if a.allowed {
+		return kauthorizer.DecisionAllow, "", nil
+	}
+	return kauthorizer.DecisionDeny, "", nil
 }
 
 func (a *fakeAuthorizer) GetAllowedSubjects(attributes kauthorizer.Attributes) (sets.String, sets.String, error) {

pkg/authorization/registry/localsubjectaccessreview/rest_test.go

@@ -26,17 +26,17 @@ type subjectAccessTest struct {
 }
 
 type testAuthorizer struct {
-	allowed bool
+	allowed kauthorizer.Decision
 	reason  string
 	err     string
 
 	actualAttributes kauthorizer.Attributes
 }
 
-func (a *testAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (allowed bool, reason string, err error) {
+func (a *testAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (authorized kauthorizer.Decision, reason string, err error) {
 	// allow the initial check for "can I run this SAR at all"
 	if passedAttributes.GetResource() == "localsubjectaccessreviews" {
-		return true, "", nil
+		return kauthorizer.DecisionAllow, "", nil
 	}
 
 	a.actualAttributes = passedAttributes
@@ -53,7 +53,7 @@ func (a *testAuthorizer) GetAllowedSubjects(passedAttributes kauthorizer.Attribu
 func TestNoNamespace(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: false,
+			allowed: kauthorizer.DecisionDeny,
 		},
 		reviewRequest: &authorizationapi.LocalSubjectAccessReview{
 			Action: authorizationapi.Action{
@@ -72,7 +72,7 @@ func TestNoNamespace(t *testing.T) {
 
 func TestConflictingNamespace(t *testing.T) {
 	authorizer := &testAuthorizer{
-		allowed: false,
+		allowed: kauthorizer.DecisionDeny,
 	}
 	reviewRequest := &authorizationapi.LocalSubjectAccessReview{
 		Action: authorizationapi.Action{
@@ -98,7 +98,7 @@ func TestConflictingNamespace(t *testing.T) {
 func TestEmptyReturn(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: false,
+			allowed: kauthorizer.DecisionDeny,
 			reason:  "because reasons",
 		},
 		reviewRequest: &authorizationapi.LocalSubjectAccessReview{
@@ -123,7 +123,7 @@ func TestEmptyReturn(t *testing.T) {
 func TestNoErrors(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.LocalSubjectAccessReview{
@@ -171,7 +171,7 @@ func TestErrors(t *testing.T) {
 func TestRegularWithScopes(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.LocalSubjectAccessReview{
@@ -200,7 +200,7 @@ func TestRegularWithScopes(t *testing.T) {
 func TestSelfWithDefaultScopes(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.LocalSubjectAccessReview{
@@ -228,7 +228,7 @@ func TestSelfWithDefaultScopes(t *testing.T) {
 func TestSelfWithClearedScopes(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.LocalSubjectAccessReview{
@@ -259,7 +259,7 @@ func (r *subjectAccessTest) runTest(t *testing.T) {
 
 	expectedResponse := &authorizationapi.SubjectAccessReviewResponse{
 		Namespace:       r.reviewRequest.Action.Namespace,
-		Allowed:         r.authorizer.allowed,
+		Allowed:         r.authorizer.allowed == kauthorizer.DecisionAllow,
 		Reason:          r.authorizer.reason,
 		EvaluationError: r.authorizer.err,
 	}

pkg/authorization/registry/resourceaccessreview/rest.go

@@ -86,12 +86,12 @@ func (r *REST) isAllowed(user user.Info, rar *authorizationapi.ResourceAccessRev
 		Resource:        "localresourceaccessreviews",
 		ResourceRequest: true,
 	}
-	allowed, reason, err := r.authorizer.Authorize(localRARAttributes)
+	authorized, reason, err := r.authorizer.Authorize(localRARAttributes)
 
 	if err != nil {
 		return kapierrors.NewForbidden(authorizationapi.Resource(localRARAttributes.GetResource()), localRARAttributes.GetName(), err)
 	}
-	if !allowed {
+	if authorized != kauthorizer.DecisionAllow {
 		forbiddenError := kapierrors.NewForbidden(authorizationapi.Resource(localRARAttributes.GetResource()), localRARAttributes.GetName(), errors.New("") /*discarded*/)
 		forbiddenError.ErrStatus.Message = reason
 		return forbiddenError

pkg/authorization/registry/resourceaccessreview/rest_test.go

@@ -30,17 +30,17 @@ type testAuthorizer struct {
 	actualAttributes kauthorizer.Attributes
 }
 
-func (a *testAuthorizer) Authorize(attributes kauthorizer.Attributes) (allowed bool, reason string, err error) {
+func (a *testAuthorizer) Authorize(attributes kauthorizer.Attributes) (allowed kauthorizer.Decision, reason string, err error) {
 	// allow the initial check for "can I run this RAR at all"
 	if attributes.GetResource() == "localresourceaccessreviews" {
 		if len(a.deniedNamespaces) != 0 && a.deniedNamespaces.Has(attributes.GetNamespace()) {
-			return false, "denied initial check", nil
+			return kauthorizer.DecisionDeny, "denied initial check", nil
 		}
 
-		return true, "", nil
+		return kauthorizer.DecisionAllow, "", nil
 	}
 
-	return false, "", errors.New("unsupported")
+	return kauthorizer.DecisionDeny, "", errors.New("unsupported")
 }
 func (a *testAuthorizer) GetAllowedSubjects(passedAttributes kauthorizer.Attributes) (sets.String, sets.String, error) {
 	a.actualAttributes = passedAttributes

pkg/authorization/registry/subjectaccessreview/rest.go

@@ -108,10 +108,10 @@ func (r *REST) Create(ctx apirequest.Context, obj runtime.Object, _ rest.Validat
 	}
 
 	attributes := util.ToDefaultAuthorizationAttributes(userToCheck, subjectAccessReview.Action.Namespace, subjectAccessReview.Action)
-	allowed, reason, err := r.authorizer.Authorize(attributes)
+	authorized, reason, err := r.authorizer.Authorize(attributes)
 	response := &authorizationapi.SubjectAccessReviewResponse{
 		Namespace: subjectAccessReview.Action.Namespace,
-		Allowed:   allowed,
+		Allowed:   authorized == kauthorizer.DecisionAllow,
 		Reason:    reason,
 	}
 	if err != nil {
@@ -144,12 +144,12 @@ func (r *REST) isAllowed(user user.Info, sar *authorizationapi.SubjectAccessRevi
 		}
 	}
 
-	allowed, reason, err := r.authorizer.Authorize(localSARAttributes)
+	authorized, reason, err := r.authorizer.Authorize(localSARAttributes)
 
 	if err != nil {
 		return kapierrors.NewForbidden(authorizationapi.Resource(localSARAttributes.GetResource()), localSARAttributes.GetName(), err)
 	}
-	if !allowed {
+	if authorized != kauthorizer.DecisionAllow {
 		forbiddenError := kapierrors.NewForbidden(authorizationapi.Resource(localSARAttributes.GetResource()), localSARAttributes.GetName(), errors.New("") /*discarded*/)
 		forbiddenError.ErrStatus.Message = reason
 		return forbiddenError

pkg/authorization/registry/subjectaccessreview/rest_test.go

@@ -26,22 +26,22 @@ type subjectAccessTest struct {
 }
 
 type testAuthorizer struct {
-	allowed          bool
+	allowed          kauthorizer.Decision
 	reason           string
 	err              string
 	deniedNamespaces sets.String
 
 	actualAttributes kauthorizer.Attributes
 }
 
-func (a *testAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (allowed bool, reason string, err error) {
+func (a *testAuthorizer) Authorize(passedAttributes kauthorizer.Attributes) (allowed kauthorizer.Decision, reason string, err error) {
 	// allow the initial check for "can I run this SAR at all"
 	if passedAttributes.GetResource() == "localsubjectaccessreviews" {
 		if len(a.deniedNamespaces) != 0 && a.deniedNamespaces.Has(passedAttributes.GetNamespace()) {
-			return false, "denied initial check", nil
+			return kauthorizer.DecisionDeny, "denied initial check", nil
 		}
 
-		return true, "", nil
+		return kauthorizer.DecisionAllow, "", nil
 	}
 
 	a.actualAttributes = passedAttributes
@@ -58,7 +58,7 @@ func (a *testAuthorizer) GetAllowedSubjects(passedAttributes kauthorizer.Attribu
 func TestDeniedNamespace(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed:          false,
+			allowed:          kauthorizer.DecisionDeny,
 			err:              "denied initial check",
 			deniedNamespaces: sets.NewString("foo"),
 		},
@@ -80,7 +80,7 @@ func TestDeniedNamespace(t *testing.T) {
 func TestEmptyReturn(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: false,
+			allowed: kauthorizer.DecisionDeny,
 			reason:  "because reasons",
 		},
 		reviewRequest: &authorizationapi.SubjectAccessReview{
@@ -104,7 +104,7 @@ func TestEmptyReturn(t *testing.T) {
 func TestNoErrors(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.SubjectAccessReview{
@@ -150,7 +150,7 @@ func TestErrors(t *testing.T) {
 func TestRegularWithScopes(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.SubjectAccessReview{
@@ -178,7 +178,7 @@ func TestRegularWithScopes(t *testing.T) {
 func TestSelfWithDefaultScopes(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.SubjectAccessReview{
@@ -205,7 +205,7 @@ func TestSelfWithDefaultScopes(t *testing.T) {
 func TestSelfWithClearedScopes(t *testing.T) {
 	test := &subjectAccessTest{
 		authorizer: &testAuthorizer{
-			allowed: true,
+			allowed: kauthorizer.DecisionAllow,
 			reason:  "because good things",
 		},
 		reviewRequest: &authorizationapi.SubjectAccessReview{
@@ -235,7 +235,7 @@ func (r *subjectAccessTest) runTest(t *testing.T) {
 
 	expectedResponse := &authorizationapi.SubjectAccessReviewResponse{
 		Namespace:       r.reviewRequest.Action.Namespace,
-		Allowed:         r.authorizer.allowed,
+		Allowed:         r.authorizer.allowed == kauthorizer.DecisionAllow,
 		Reason:          r.authorizer.reason,
 		EvaluationError: r.authorizer.err,
 	}

pkg/cmd/server/handlers/authorization.go

@@ -8,12 +8,14 @@ import (
 	"net/http"
 
 	restful "github.com/emicklei/go-restful"
+	"k8s.io/kubernetes/pkg/api/legacyscheme"
 
 	kapierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	kauthorizer "k8s.io/apiserver/pkg/authorization/authorizer"
+	coreapi "k8s.io/kubernetes/pkg/apis/core"
 )
 
 type bypassAuthorizer struct {
@@ -27,9 +29,9 @@ func NewBypassAuthorizer(auth kauthorizer.Authorizer, paths ...string) kauthoriz
 	return bypassAuthorizer{paths: sets.NewString(paths...), authorizer: auth}
 }
 
-func (a bypassAuthorizer) Authorize(attributes kauthorizer.Attributes) (allowed bool, reason string, err error) {
+func (a bypassAuthorizer) Authorize(attributes kauthorizer.Attributes) (allowed kauthorizer.Decision, reason string, err error) {
 	if !attributes.IsResourceRequest() && a.paths.Has(attributes.GetPath()) {
-		return true, "always allowed", nil
+		return kauthorizer.DecisionAllow, "always allowed", nil
 	}
 	return a.authorizer.Authorize(attributes)
 }
@@ -57,7 +59,7 @@ func Forbidden(reason string, attributes kauthorizer.Attributes, w http.Response
 	forbiddenError.ErrStatus.Message = reason
 
 	formatted := &bytes.Buffer{}
-	output, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(legacyscheme.SchemeGroupVersion), &forbiddenError.ErrStatus)
+	output, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(coreapi.SchemeGroupVersion), &forbiddenError.ErrStatus)
 	if err != nil {
 		fmt.Fprintf(formatted, "%s", forbiddenError.Error())
 	} else {