Merge pull request #53273 from mikedanese/authtristate

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

add support for short-circuit deny in union authorizer

This change has no behavioral changes.

Fixes https://github.com/kubernetes/kubernetes/issues/51862

```release-note
Add support for the webhook authorizer to make a Deny decision that short-circuits the union authorizer and immediately returns Deny.
```

Kubernetes-commit: d33077526af1e22c4d7124836a987ce108c1533b

pkg/admission/initializer/initializer_test.go

@@ -133,8 +133,8 @@ var _ initializer.WantsAuthorizer = &WantAuthorizerAdmission{}
 // TestAuthorizer is a test stub that fulfills the WantsAuthorizer interface.
 type TestAuthorizer struct{}
 
-func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
-	return false, "", nil
+func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+	return authorizer.DecisionNoOpinion, "", nil
 }
 
 // wantClientCert is a test stub for testing that fulfulls the WantsClientCert interface.

pkg/admission/plugin/initialization/initialization.go

@@ -260,7 +260,7 @@ func (i *initializer) Admit(a admission.Attributes) (err error) {
 
 func (i *initializer) canInitialize(a admission.Attributes, message string) error {
 	// caller must have the ability to mutate un-initialized resources
-	authorized, reason, err := i.authorizer.Authorize(authorizer.AttributesRecord{
+	decision, reason, err := i.authorizer.Authorize(authorizer.AttributesRecord{
 		Name:            a.GetName(),
 		ResourceRequest: true,
 		User:            a.GetUserInfo(),
@@ -273,7 +273,7 @@ func (i *initializer) canInitialize(a admission.Attributes, message string) erro
 	if err != nil {
 		return err
 	}
-	if !authorized {
+	if decision != authorizer.DecisionAllow {
 		return errors.NewForbidden(a.GetResource().GroupResource(), a.GetName(), fmt.Errorf("%s: %s", message, reason))
 	}
 	return nil

pkg/admission/plugin/initialization/initialization_test.go

@@ -109,11 +109,11 @@ type fakeAuthorizer struct {
 	accept bool
 }
 
-func (f *fakeAuthorizer) Authorize(a authorizer.Attributes) (bool, string, error) {
+func (f *fakeAuthorizer) Authorize(a authorizer.Attributes) (authorizer.Decision, string, error) {
 	if f.accept {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
-	return false, "denied", nil
+	return authorizer.DecisionNoOpinion, "denied", nil
 }
 
 func TestAdmitUpdate(t *testing.T) {

pkg/authorization/authorizer/interfaces.go

@@ -67,12 +67,12 @@ type Attributes interface {
 // zero or more calls to methods of the Attributes interface.  It returns nil when an action is
 // authorized, otherwise it returns an error.
 type Authorizer interface {
-	Authorize(a Attributes) (authorized bool, reason string, err error)
+	Authorize(a Attributes) (authorized Decision, reason string, err error)
 }
 
-type AuthorizerFunc func(a Attributes) (bool, string, error)
+type AuthorizerFunc func(a Attributes) (Decision, string, error)
 
-func (f AuthorizerFunc) Authorize(a Attributes) (bool, string, error) {
+func (f AuthorizerFunc) Authorize(a Attributes) (Decision, string, error) {
 	return f(a)
 }
 
@@ -144,3 +144,15 @@ func (a AttributesRecord) IsResourceRequest() bool {
 func (a AttributesRecord) GetPath() string {
 	return a.Path
 }
+
+type Decision int
+
+const (
+	// DecisionDeny means that an authorizer decided to deny the action.
+	DecisionDeny Decision = iota
+	// DecisionAllow means that an authorizer decided to allow the action.
+	DecisionAllow
+	// DecisionNoOpionion means that an authorizer has no opinion on wether
+	// to allow or deny an action.
+	DecisionNoOpinion
+)

pkg/authorization/authorizerfactory/authz_test.go

@@ -27,7 +27,7 @@ import (
 // and always return nil.
 func TestNewAlwaysAllowAuthorizer(t *testing.T) {
 	aaa := NewAlwaysAllowAuthorizer()
-	if authorized, _, _ := aaa.Authorize(nil); !authorized {
+	if decision, _, _ := aaa.Authorize(nil); decision != authorizer.DecisionAllow {
 		t.Errorf("AlwaysAllowAuthorizer.Authorize did not authorize successfully.")
 	}
 }
@@ -36,7 +36,7 @@ func TestNewAlwaysAllowAuthorizer(t *testing.T) {
 // and always return an error as everything is forbidden.
 func TestNewAlwaysDenyAuthorizer(t *testing.T) {
 	ada := NewAlwaysDenyAuthorizer()
-	if authorized, _, _ := ada.Authorize(nil); authorized {
+	if decision, _, _ := ada.Authorize(nil); decision == authorizer.DecisionAllow {
 		t.Errorf("AlwaysDenyAuthorizer.Authorize returned nil instead of error.")
 	}
 }
@@ -47,10 +47,10 @@ func TestPrivilegedGroupAuthorizer(t *testing.T) {
 	yes := authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"no", "allow-01"}}}
 	no := authorizer.AttributesRecord{User: &user.DefaultInfo{Groups: []string{"no", "deny-01"}}}
 
-	if authorized, _, _ := auth.Authorize(yes); !authorized {
+	if authorized, _, _ := auth.Authorize(yes); authorized != authorizer.DecisionAllow {
 		t.Errorf("failed")
 	}
-	if authorized, _, _ := auth.Authorize(no); authorized {
+	if authorized, _, _ := auth.Authorize(no); authorized == authorizer.DecisionAllow {
 		t.Errorf("failed")
 	}
 }

pkg/authorization/authorizerfactory/builtin.go

@@ -28,8 +28,8 @@ import (
 // It is useful in tests and when using kubernetes in an open manner.
 type alwaysAllowAuthorizer struct{}
 
-func (alwaysAllowAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
-	return true, "", nil
+func (alwaysAllowAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+	return authorizer.DecisionAllow, "", nil
 }
 
 func (alwaysAllowAuthorizer) RulesFor(user user.Info, namespace string) ([]authorizer.ResourceRuleInfo, []authorizer.NonResourceRuleInfo, bool, error) {
@@ -56,8 +56,8 @@ func NewAlwaysAllowAuthorizer() *alwaysAllowAuthorizer {
 // It is useful in unit tests to force an operation to be forbidden.
 type alwaysDenyAuthorizer struct{}
 
-func (alwaysDenyAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
-	return false, "Everything is forbidden.", nil
+func (alwaysDenyAuthorizer) Authorize(a authorizer.Attributes) (decision authorizer.Decision, reason string, err error) {
+	return authorizer.DecisionNoOpinion, "Everything is forbidden.", nil
 }
 
 func (alwaysDenyAuthorizer) RulesFor(user user.Info, namespace string) ([]authorizer.ResourceRuleInfo, []authorizer.NonResourceRuleInfo, bool, error) {
@@ -73,8 +73,8 @@ func NewAlwaysDenyAuthorizer() *alwaysDenyAuthorizer {
 // It is useful in unit tests to force an operation to fail with error.
 type alwaysFailAuthorizer struct{}
 
-func (alwaysFailAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
-	return false, "", errors.New("Authorization failure.")
+func (alwaysFailAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
+	return authorizer.DecisionNoOpinion, "", errors.New("Authorization failure.")
 }
 
 func NewAlwaysFailAuthorizer() authorizer.Authorizer {
@@ -85,18 +85,18 @@ type privilegedGroupAuthorizer struct {
 	groups []string
 }
 
-func (r *privilegedGroupAuthorizer) Authorize(attr authorizer.Attributes) (bool, string, error) {
+func (r *privilegedGroupAuthorizer) Authorize(attr authorizer.Attributes) (authorizer.Decision, string, error) {
 	if attr.GetUser() == nil {
-		return false, "Error", errors.New("no user on request.")
+		return authorizer.DecisionNoOpinion, "Error", errors.New("no user on request.")
 	}
 	for _, attr_group := range attr.GetUser().GetGroups() {
 		for _, priv_group := range r.groups {
 			if priv_group == attr_group {
-				return true, "", nil
+				return authorizer.DecisionAllow, "", nil
 			}
 		}
 	}
-	return false, "", nil
+	return authorizer.DecisionNoOpinion, "", nil
 }
 
 // NewPrivilegedGroups is for use in loopback scenarios

pkg/authorization/union/union.go

@@ -14,6 +14,14 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+// Package union implements an authorizer that combines multiple subauthorizer.
+// The union authorizer iterates over each subauthorizer and returns the first
+// decision that is either an Allow decision or a Deny decision. If a
+// subauthorizer returns a NoOpinion, then the union authorizer moves onto the
+// next authorizer or, if the subauthorizer was the last authorizer, returns
+// NoOpinion as the aggregate decision. I.e. union authorizer creates an
+// aggregate decision and supports short-circut allows and denies from
+// subauthorizers.
 package union
 
 import (
@@ -33,14 +41,14 @@ func New(authorizationHandlers ...authorizer.Authorizer) authorizer.Authorizer {
 }
 
 // Authorizes against a chain of authorizer.Authorizer objects and returns nil if successful and returns error if unsuccessful
-func (authzHandler unionAuthzHandler) Authorize(a authorizer.Attributes) (bool, string, error) {
+func (authzHandler unionAuthzHandler) Authorize(a authorizer.Attributes) (authorizer.Decision, string, error) {
 	var (
 		errlist    []error
 		reasonlist []string
 	)
 
 	for _, currAuthzHandler := range authzHandler {
-		authorized, reason, err := currAuthzHandler.Authorize(a)
+		decision, reason, err := currAuthzHandler.Authorize(a)
 
 		if err != nil {
 			errlist = append(errlist, err)
@@ -48,13 +56,15 @@ func (authzHandler unionAuthzHandler) Authorize(a authorizer.Attributes) (bool,
 		if len(reason) != 0 {
 			reasonlist = append(reasonlist, reason)
 		}
-		if !authorized {
-			continue
+		switch decision {
+		case authorizer.DecisionAllow, authorizer.DecisionDeny:
+			return decision, reason, err
+		case authorizer.DecisionNoOpinion:
+			// continue to the next authorizer
 		}
-		return true, reason, nil
 	}
 
-	return false, strings.Join(reasonlist, "\n"), utilerrors.NewAggregate(errlist)
+	return authorizer.DecisionNoOpinion, strings.Join(reasonlist, "\n"), utilerrors.NewAggregate(errlist)
 }
 
 // unionAuthzRulesHandler authorizer against a chain of authorizer.RuleResolver

pkg/authorization/union/union_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package union
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"testing"
@@ -26,49 +27,43 @@ import (
 )
 
 type mockAuthzHandler struct {
-	isAuthorized bool
-	err          error
+	decision authorizer.Decision
+	err      error
 }
 
-func (mock *mockAuthzHandler) Authorize(a authorizer.Attributes) (bool, string, error) {
-	if mock.err != nil {
-		return false, "", mock.err
-	}
-	if !mock.isAuthorized {
-		return false, "", nil
-	}
-	return true, "", nil
+func (mock *mockAuthzHandler) Authorize(a authorizer.Attributes) (authorizer.Decision, string, error) {
+	return mock.decision, "", mock.err
 }
 
 func TestAuthorizationSecondPasses(t *testing.T) {
-	handler1 := &mockAuthzHandler{isAuthorized: false}
-	handler2 := &mockAuthzHandler{isAuthorized: true}
+	handler1 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}
+	handler2 := &mockAuthzHandler{decision: authorizer.DecisionAllow}
 	authzHandler := New(handler1, handler2)
 
 	authorized, _, _ := authzHandler.Authorize(nil)
-	if !authorized {
+	if authorized != authorizer.DecisionAllow {
 		t.Errorf("Unexpected authorization failure")
 	}
 }
 
 func TestAuthorizationFirstPasses(t *testing.T) {
-	handler1 := &mockAuthzHandler{isAuthorized: true}
-	handler2 := &mockAuthzHandler{isAuthorized: false}
+	handler1 := &mockAuthzHandler{decision: authorizer.DecisionAllow}
+	handler2 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}
 	authzHandler := New(handler1, handler2)
 
 	authorized, _, _ := authzHandler.Authorize(nil)
-	if !authorized {
+	if authorized != authorizer.DecisionAllow {
 		t.Errorf("Unexpected authorization failure")
 	}
 }
 
 func TestAuthorizationNonePasses(t *testing.T) {
-	handler1 := &mockAuthzHandler{isAuthorized: false}
-	handler2 := &mockAuthzHandler{isAuthorized: false}
+	handler1 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}
+	handler2 := &mockAuthzHandler{decision: authorizer.DecisionNoOpinion}
 	authzHandler := New(handler1, handler2)
 
 	authorized, _, _ := authzHandler.Authorize(nil)
-	if authorized {
+	if authorized == authorizer.DecisionAllow {
 		t.Errorf("Expected failed authorization")
 	}
 }
@@ -223,3 +218,49 @@ func getNonResourceRules(infos []authorizer.NonResourceRuleInfo) []authorizer.De
 	}
 	return rules
 }
+
+func TestAuthorizationUnequivocalDeny(t *testing.T) {
+	cs := []struct {
+		authorizers []authorizer.Authorizer
+		decision    authorizer.Decision
+	}{
+		{
+			authorizers: []authorizer.Authorizer{},
+			decision:    authorizer.DecisionNoOpinion,
+		},
+		{
+			authorizers: []authorizer.Authorizer{
+				&mockAuthzHandler{decision: authorizer.DecisionNoOpinion},
+				&mockAuthzHandler{decision: authorizer.DecisionAllow},
+				&mockAuthzHandler{decision: authorizer.DecisionDeny},
+			},
+			decision: authorizer.DecisionAllow,
+		},
+		{
+			authorizers: []authorizer.Authorizer{
+				&mockAuthzHandler{decision: authorizer.DecisionNoOpinion},
+				&mockAuthzHandler{decision: authorizer.DecisionDeny},
+				&mockAuthzHandler{decision: authorizer.DecisionAllow},
+			},
+			decision: authorizer.DecisionDeny,
+		},
+		{
+			authorizers: []authorizer.Authorizer{
+				&mockAuthzHandler{decision: authorizer.DecisionNoOpinion},
+				&mockAuthzHandler{decision: authorizer.DecisionDeny, err: errors.New("webhook failed closed")},
+				&mockAuthzHandler{decision: authorizer.DecisionAllow},
+			},
+			decision: authorizer.DecisionDeny,
+		},
+	}
+	for i, c := range cs {
+		t.Run(fmt.Sprintf("case %v", i), func(t *testing.T) {
+			authzHandler := New(c.authorizers...)
+
+			decision, _, _ := authzHandler.Authorize(nil)
+			if decision != c.decision {
+				t.Errorf("Unexpected authorization failure: %v, expected: %v", decision, c.decision)
+			}
+		})
+	}
+}

pkg/endpoints/filters/authorization.go

@@ -47,7 +47,7 @@ func WithAuthorization(handler http.Handler, requestContextMapper request.Reques
 			return
 		}
 		authorized, reason, err := a.Authorize(attributes)
-		if authorized {
+		if authorized == authorizer.DecisionAllow {
 			handler.ServeHTTP(w, req)
 			return
 		}

pkg/endpoints/filters/impersonation.go

@@ -110,8 +110,8 @@ func WithImpersonation(handler http.Handler, requestContextMapper request.Reques
 				return
 			}
 
-			allowed, reason, err := a.Authorize(actingAsAttributes)
-			if err != nil || !allowed {
+			decision, reason, err := a.Authorize(actingAsAttributes)
+			if err != nil || decision != authorizer.DecisionAllow {
 				glog.V(4).Infof("Forbidden: %#v, Reason: %s, Error: %v", req.RequestURI, reason, err)
 				responsewriters.Forbidden(ctx, actingAsAttributes, w, req, reason, s)
 				return

pkg/endpoints/filters/impersonation_test.go

@@ -35,50 +35,50 @@ import (
 
 type impersonateAuthorizer struct{}
 
-func (impersonateAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
+func (impersonateAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 	user := a.GetUser()
 
 	switch {
 	case user.GetName() == "system:admin":
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 
 	case user.GetName() == "tester":
-		return false, "", fmt.Errorf("works on my machine")
+		return authorizer.DecisionNoOpinion, "", fmt.Errorf("works on my machine")
 
 	case user.GetName() == "deny-me":
-		return false, "denied", nil
+		return authorizer.DecisionNoOpinion, "denied", nil
 	}
 
 	if len(user.GetGroups()) > 0 && user.GetGroups()[0] == "wheel" && a.GetVerb() == "impersonate" && a.GetResource() == "users" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
 	if len(user.GetGroups()) > 0 && user.GetGroups()[0] == "sa-impersonater" && a.GetVerb() == "impersonate" && a.GetResource() == "serviceaccounts" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
 	if len(user.GetGroups()) > 0 && user.GetGroups()[0] == "regular-impersonater" && a.GetVerb() == "impersonate" && a.GetResource() == "users" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
 	if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "group-impersonater" && a.GetVerb() == "impersonate" && a.GetResource() == "groups" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
 	if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-scopes" && a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "scopes" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
 	if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-particular-scopes" &&
 		a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "scopes" && a.GetName() == "scope-a" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
 	if len(user.GetGroups()) > 1 && user.GetGroups()[1] == "extra-setter-project" && a.GetVerb() == "impersonate" && a.GetResource() == "userextras" && a.GetSubresource() == "project" {
-		return true, "", nil
+		return authorizer.DecisionAllow, "", nil
 	}
 
-	return false, "deny by default", nil
+	return authorizer.DecisionNoOpinion, "deny by default", nil
 }
 
 func TestImpersonationFilter(t *testing.T) {

pkg/server/genericapiserver_test.go

@@ -437,9 +437,9 @@ type mockAuthorizer struct {
 	lastURI string
 }
 
-func (authz *mockAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, reason string, err error) {
+func (authz *mockAuthorizer) Authorize(a authorizer.Attributes) (authorized authorizer.Decision, reason string, err error) {
 	authz.lastURI = a.GetPath()
-	return true, "", nil
+	return authorizer.DecisionAllow, "", nil
 }
 
 type mockAuthenticator struct {

plugin/pkg/authorizer/webhook/webhook.go

@@ -50,6 +50,7 @@ type WebhookAuthorizer struct {
 	authorizedTTL       time.Duration
 	unauthorizedTTL     time.Duration
 	initialBackoff      time.Duration
+	decisionOnError     authorizer.Decision
 }
 
 // NewFromInterface creates a WebhookAuthorizer using the given subjectAccessReview client
@@ -93,6 +94,7 @@ func newWithBackoff(subjectAccessReview authorizationclient.SubjectAccessReviewI
 		authorizedTTL:       authorizedTTL,
 		unauthorizedTTL:     unauthorizedTTL,
 		initialBackoff:      initialBackoff,
+		decisionOnError:     authorizer.DecisionNoOpinion,
 	}, nil
 }
 
@@ -140,7 +142,10 @@ func newWithBackoff(subjectAccessReview authorizationclient.SubjectAccessReviewI
 //       }
 //     }
 //
-func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bool, reason string, err error) {
+// TODO(mikedanese): We should eventually support failing closed when we
+// encounter an error. We are failing open now to preserve backwards compatible
+// behavior.
+func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (decision authorizer.Decision, reason string, err error) {
 	r := &authorization.SubjectAccessReview{}
 	if user := attr.GetUser(); user != nil {
 		r.Spec = authorization.SubjectAccessReviewSpec{
@@ -169,7 +174,7 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bo
 	}
 	key, err := json.Marshal(r.Spec)
 	if err != nil {
-		return false, "", err
+		return w.decisionOnError, "", err
 	}
 	if entry, ok := w.responseCache.Get(string(key)); ok {
 		r.Status = entry.(authorization.SubjectAccessReviewStatus)
@@ -185,7 +190,7 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bo
 		if err != nil {
 			// An error here indicates bad configuration or an outage. Log for debugging.
 			glog.Errorf("Failed to make webhook authorizer request: %v", err)
-			return false, "", err
+			return w.decisionOnError, "", err
 		}
 		r.Status = result.Status
 		if r.Status.Allowed {
@@ -194,7 +199,17 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bo
 			w.responseCache.Add(string(key), r.Status, w.unauthorizedTTL)
 		}
 	}
-	return r.Status.Allowed, r.Status.Reason, nil
+	switch {
+	case r.Status.Denied && r.Status.Allowed:
+		return authorizer.DecisionDeny, r.Status.Reason, fmt.Errorf("webhook subject access review returned both allow and deny response")
+	case r.Status.Denied:
+		return authorizer.DecisionDeny, r.Status.Reason, nil
+	case r.Status.Allowed:
+		return authorizer.DecisionAllow, r.Status.Reason, nil
+	default:
+		return authorizer.DecisionNoOpinion, r.Status.Reason, nil
+	}
+
 }
 
 //TODO: need to finish the method to get the rules when using webhook mode

plugin/pkg/authorizer/webhook/webhook_test.go

@@ -396,13 +396,13 @@ func TestTLSConfig(t *testing.T) {
 
 			// Allow all and see if we get an error.
 			service.Allow()
-			authorized, _, err := wh.Authorize(attr)
+			decision, _, err := wh.Authorize(attr)
 			if tt.wantAuth {
-				if !authorized {
+				if decision != authorizer.DecisionAllow {
 					t.Errorf("expected successful authorization")
 				}
 			} else {
-				if authorized {
+				if decision == authorizer.DecisionAllow {
 					t.Errorf("expected failed authorization")
 				}
 			}
@@ -418,7 +418,7 @@ func TestTLSConfig(t *testing.T) {
 			}
 
 			service.Deny()
-			if authorized, _, _ := wh.Authorize(attr); authorized {
+			if decision, _, _ := wh.Authorize(attr); decision == authorizer.DecisionAllow {
 				t.Errorf("%s: incorrectly authorized with DenyAll policy", tt.test)
 			}
 		}()
@@ -522,11 +522,11 @@ func TestWebhook(t *testing.T) {
 	}
 
 	for i, tt := range tests {
-		authorized, _, err := wh.Authorize(tt.attr)
+		decision, _, err := wh.Authorize(tt.attr)
 		if err != nil {
 			t.Fatal(err)
 		}
-		if !authorized {
+		if decision != authorizer.DecisionAllow {
 			t.Errorf("case %d: authorization failed", i)
 			continue
 		}
@@ -567,7 +567,7 @@ func testWebhookCacheCases(t *testing.T, serv *mockService, wh *WebhookAuthorize
 			continue
 		}
 
-		if test.expectedAuthorized != authorized {
+		if test.expectedAuthorized != (authorized == authorizer.DecisionAllow) {
 			t.Errorf("%d: expected authorized=%v, got %v", i, test.expectedAuthorized, authorized)
 		}