From 294e02ed4b341fe9497cdfadb93cf19f1e64243f Mon Sep 17 00:00:00 2001 From: Samuel Davidson Date: Fri, 26 Oct 2018 15:58:09 -0700 Subject: [PATCH 1/2] Revert "limit forbidden error to details of what was forbidden" This reverts commit ecbd0137957b4afd4cdd94c0209998228fd70e99. --- .../src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go | 2 +- .../src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go | 2 +- test/integration/master/synthetic_master_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go index 998c05bcf73..4c9f140ca30 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go @@ -73,7 +73,7 @@ func WithAuthorization(handler http.Handler, a authorizer.Authorizer, s runtime. glog.V(4).Infof("Forbidden: %#v, Reason: %q", req.RequestURI, reason) audit.LogAnnotation(ae, decisionAnnotationKey, decisionForbid) audit.LogAnnotation(ae, reasonAnnotationKey, reason) - responsewriters.Forbidden(ctx, attributes, w, req, "", s) + responsewriters.Forbidden(ctx, attributes, w, req, reason, s) }) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go index 38414a6afa7..726cbe4d565 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/impersonation.go @@ -110,7 +110,7 @@ func WithImpersonation(handler http.Handler, a authorizer.Authorizer, s runtime. 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, "", s) + responsewriters.Forbidden(ctx, actingAsAttributes, w, req, reason, s) return } } diff --git a/test/integration/master/synthetic_master_test.go b/test/integration/master/synthetic_master_test.go index d0190830dff..a4ef671983a 100644 --- a/test/integration/master/synthetic_master_test.go +++ b/test/integration/master/synthetic_master_test.go @@ -175,7 +175,7 @@ func TestStatus(t *testing.T) { statusCode: http.StatusForbidden, reqPath: "/apis", reason: "Forbidden", - message: `forbidden: User "" cannot get path "/apis"`, + message: `forbidden: User "" cannot get path "/apis": Everything is forbidden.`, }, { name: "401", From 3558f839577effe9e4f5b3fcd995c12de7869738 Mon Sep 17 00:00:00 2001 From: Samuel Davidson Date: Mon, 29 Oct 2018 11:05:45 -0700 Subject: [PATCH 2/2] Revert "Improve multi-authorizer errors" This reverts commit 1c012f1c4966dfe6124192abafeba892ae9510a0. --- pkg/auth/authorizer/abac/abac.go | 2 +- plugin/pkg/auth/authorizer/rbac/rbac.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/auth/authorizer/abac/abac.go b/pkg/auth/authorizer/abac/abac.go index fc3a9dfcf9b..86e7f8ed3e0 100644 --- a/pkg/auth/authorizer/abac/abac.go +++ b/pkg/auth/authorizer/abac/abac.go @@ -227,7 +227,7 @@ func (pl policyList) Authorize(a authorizer.Attributes) (authorizer.Decision, st return authorizer.DecisionAllow, "", nil } } - return authorizer.DecisionNoOpinion, "no ABAC policy matched", nil + return authorizer.DecisionNoOpinion, "No policy matched.", nil // TODO: Benchmark how much time policy matching takes with a medium size // policy file, compared to other steps such as encoding/decoding. // Then, add Caching only if needed. diff --git a/plugin/pkg/auth/authorizer/rbac/rbac.go b/plugin/pkg/auth/authorizer/rbac/rbac.go index 1701bbc3176..a0f173c393b 100644 --- a/plugin/pkg/auth/authorizer/rbac/rbac.go +++ b/plugin/pkg/auth/authorizer/rbac/rbac.go @@ -121,8 +121,6 @@ func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (aut reason := "" if len(ruleCheckingVisitor.errors) > 0 { reason = fmt.Sprintf("RBAC: %v", utilerrors.NewAggregate(ruleCheckingVisitor.errors)) - } else { - reason = "no RBAC policy matched" } return authorizer.DecisionNoOpinion, reason, nil }