diff --git a/plugin/pkg/authorizer/webhook/webhook.go b/plugin/pkg/authorizer/webhook/webhook.go index 83577496e..e9efac307 100644 --- a/plugin/pkg/authorizer/webhook/webhook.go +++ b/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,9 +142,9 @@ func newWithBackoff(subjectAccessReview authorizationclient.SubjectAccessReviewI // } // } // -// TODO(mikedanese): We should eventually fail closed when we encounter and -// error. We are failing open now to preserve backwards compatible behavior. -// Fix this after deprecation. +// 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 { @@ -172,7 +174,7 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (decision auth } key, err := json.Marshal(r.Spec) if err != nil { - return authorizer.DecisionNoOpinion, "", err + return w.decisionOnError, "", err } if entry, ok := w.responseCache.Get(string(key)); ok { r.Status = entry.(authorization.SubjectAccessReviewStatus) @@ -188,7 +190,7 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (decision auth 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 authorizer.DecisionNoOpinion, "", err + return w.decisionOnError, "", err } r.Status = result.Status if r.Status.Allowed { @@ -197,9 +199,14 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (decision auth w.responseCache.Add(string(key), r.Status, w.unauthorizedTTL) } } - if r.Status.Allowed { + 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 - } else { + default: return authorizer.DecisionNoOpinion, r.Status.Reason, nil }