Merge pull request #20379 from simo5/kill401MsgMkr

Drop authorizer wrapper

Author: openshift-merge-robot

pkg/authorization/authorizer/authorizer.go

@@ -1,6 +1,8 @@
 package browsersafe
 
 import (
+	"fmt"
+
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 )
@@ -25,8 +27,17 @@ func NewBrowserSafeAuthorizer(delegate authorizer.Authorizer, authenticatedGroup
 }
 
 func (a *browserSafeAuthorizer) Authorize(attributes authorizer.Attributes) (authorizer.Decision, string, error) {
-	browserSafeAttributes := a.getBrowserSafeAttributes(attributes)
-	return a.delegate.Authorize(browserSafeAttributes)
+	attrs := a.getBrowserSafeAttributes(attributes)
+	decision, reason, err := a.delegate.Authorize(attrs)
+	safeAttributes, changed := attrs.(*browserSafeAttributes)
+
+	// check if the request was not allowed and we changed the attributes
+	if decision == authorizer.DecisionAllow || !changed {
+		return decision, reason, err
+	}
+
+	// if so, use this information to update the reason
+	return decision, safeAttributes.reason(reason), err
 }
 
 func (a *browserSafeAuthorizer) getBrowserSafeAttributes(attributes authorizer.Attributes) authorizer.Attributes {
@@ -77,3 +88,19 @@ func (b *browserSafeAttributes) GetSubresource() string {
 	}
 	return b.Attributes.GetSubresource()
 }
+
+func (b *browserSafeAttributes) reason(reason string) string {
+	if b.isProxyVerb {
+		if len(reason) != 0 {
+			reason += ", "
+		}
+		reason += fmt.Sprintf("%s verb changed to %s", proxyAction, unsafeProxy)
+	}
+	if b.isProxySubresource {
+		if len(reason) != 0 {
+			reason += ", "
+		}
+		reason += fmt.Sprintf("%s subresource changed to %s", proxyAction, unsafeProxy)
+	}
+	return reason
+}

pkg/authorization/authorizer/browsersafe/authorizer.go

@@ -13,6 +13,7 @@ func TestBrowserSafeAuthorizer(t *testing.T) {
 
 		expectedVerb        string
 		expectedSubresource string
+		expectedReason      string
 	}{
 		"non-resource": {
 			attributes:   authorizer.AttributesRecord{ResourceRequest: false, Verb: "GET"},
@@ -29,15 +30,18 @@ func TestBrowserSafeAuthorizer(t *testing.T) {
 			attributes:          authorizer.AttributesRecord{ResourceRequest: true, Verb: "get", Resource: "pods", Subresource: "proxy"},
 			expectedVerb:        "get",
 			expectedSubresource: "unsafeproxy",
+			expectedReason:      "proxy subresource changed to unsafeproxy",
 		},
 		"unsafe proxy verb": {
-			attributes:   authorizer.AttributesRecord{ResourceRequest: true, Verb: "proxy", Resource: "nodes"},
-			expectedVerb: "unsafeproxy",
+			attributes:     authorizer.AttributesRecord{ResourceRequest: true, Verb: "proxy", Resource: "nodes"},
+			expectedVerb:   "unsafeproxy",
+			expectedReason: "proxy verb changed to unsafeproxy",
 		},
 		"unsafe proxy verb anonymous": {
 			attributes: authorizer.AttributesRecord{ResourceRequest: true, Verb: "proxy", Resource: "nodes",
 				User: &user.DefaultInfo{Name: "system:anonymous", Groups: []string{"system:unauthenticated"}}},
-			expectedVerb: "unsafeproxy",
+			expectedVerb:   "unsafeproxy",
+			expectedReason: "proxy verb changed to unsafeproxy",
 		},
 
 		"proxy subresource authenticated": {
@@ -51,7 +55,7 @@ func TestBrowserSafeAuthorizer(t *testing.T) {
 		safeAuthorizer := NewBrowserSafeAuthorizer(delegateAuthorizer, "system:authenticated")
 
 		authorized, reason, err := safeAuthorizer.Authorize(tc.attributes)
-		if authorized == authorizer.DecisionAllow || len(reason) != 0 || err != nil {
+		if authorized == authorizer.DecisionAllow || reason != tc.expectedReason || err != nil {
 			t.Errorf("%s: unexpected output: %v %s %v", name, authorized, reason, err)
 			continue
 		}