Skip to content

Commit

Permalink
pkg/proxy: check all rewrite headers
Browse files Browse the repository at this point in the history
It is considered problematic, when only the first header value is
being authorized and the rest is dropped. It might give the false
impression that all given values are authorized.

Signed-off-by: Krzysztof Ostrowski <kostrows@redhat.com>
  • Loading branch information
ibihim committed May 9, 2022
1 parent 839fef0 commit 9fd649a
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 10 deletions.
7 changes: 5 additions & 2 deletions pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"fmt"
"net/http"
"net/textproto"
"strings"
"text/template"

Expand Down Expand Up @@ -187,8 +188,10 @@ func (n krpAuthorizerAttributesGetter) GetRequestAttributes(u user.Info, r *http
}
}
if n.authzConfig.Rewrites.ByHTTPHeader != nil && n.authzConfig.Rewrites.ByHTTPHeader.Name != "" {
if p := r.Header.Get(n.authzConfig.Rewrites.ByHTTPHeader.Name); p != "" {
params = append(params, p)
mimeHeader := textproto.MIMEHeader(r.Header)
mimeKey := textproto.CanonicalMIMEHeaderKey(n.authzConfig.Rewrites.ByHTTPHeader.Name)
if ps, ok := mimeHeader[mimeKey]; ok {
params = append(params, ps...)
}
}

Expand Down
55 changes: 47 additions & 8 deletions pkg/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func TestGeneratingAuthorizerAttributes(t *testing.T) {
Rewrites: &authz.SubjectAccessReviewRewrites{ByQueryParameter: &authz.QueryParameterRewriteConfig{Name: "namespace"}},
ResourceAttributes: &authz.ResourceAttributes{Namespace: "{{ .Value }}", APIVersion: "v1", Resource: "namespace", Subresource: "metrics"},
},
createRequest(map[string]string{"namespace": "tenant1"}, nil),
createRequest(map[string][]string{"namespace": {"tenant1"}}, nil),
[]authorizer.Attributes{
authorizer.AttributesRecord{
User: nil,
Expand Down Expand Up @@ -164,7 +164,7 @@ func TestGeneratingAuthorizerAttributes(t *testing.T) {
Rewrites: &authz.SubjectAccessReviewRewrites{ByHTTPHeader: &authz.HTTPHeaderRewriteConfig{Name: "namespace"}},
ResourceAttributes: &authz.ResourceAttributes{Namespace: "{{ .Value }}", APIVersion: "v1", Resource: "namespace", Subresource: "metrics"},
},
createRequest(nil, map[string]string{"namespace": "tenant1"}),
createRequest(nil, map[string][]string{"namespace": {"tenant1"}}),
[]authorizer.Attributes{
authorizer.AttributesRecord{
User: nil,
Expand All @@ -179,6 +179,38 @@ func TestGeneratingAuthorizerAttributes(t *testing.T) {
},
},
},
{
"with http header rewrites config and additional header",
&authz.Config{
Rewrites: &authz.SubjectAccessReviewRewrites{ByHTTPHeader: &authz.HTTPHeaderRewriteConfig{Name: "namespace"}},
ResourceAttributes: &authz.ResourceAttributes{Namespace: "{{ .Value }}", APIVersion: "v1", Resource: "namespace", Subresource: "metrics"},
},
createRequest(nil, map[string][]string{"namespace": {"tenant1", "tenant2"}}),
[]authorizer.Attributes{
authorizer.AttributesRecord{
User: nil,
Verb: "get",
Namespace: "tenant1",
APIGroup: "",
APIVersion: "v1",
Resource: "namespace",
Subresource: "metrics",
Name: "",
ResourceRequest: true,
},
authorizer.AttributesRecord{
User: nil,
Verb: "get",
Namespace: "tenant2",
APIGroup: "",
APIVersion: "v1",
Resource: "namespace",
Subresource: "metrics",
Name: "",
ResourceRequest: true,
},
},
},
{
"with http header rewrites config but missing header",
&authz.Config{
Expand All @@ -197,7 +229,10 @@ func TestGeneratingAuthorizerAttributes(t *testing.T) {
},
ResourceAttributes: &authz.ResourceAttributes{Namespace: "{{ .Value }}", APIVersion: "v1", Resource: "namespace", Subresource: "metrics"},
},
createRequest(map[string]string{"namespace": "tenant1"}, map[string]string{"namespace": "tenant2"}),
createRequest(
map[string][]string{"namespace": {"tenant1"}},
map[string][]string{"namespace": {"tenant2"}},
),
[]authorizer.Attributes{
authorizer.AttributesRecord{
User: nil,
Expand Down Expand Up @@ -237,17 +272,21 @@ func TestGeneratingAuthorizerAttributes(t *testing.T) {
}
}

func createRequest(queryParams, headers map[string]string) *http.Request {
func createRequest(queryParams, headers map[string][]string) *http.Request {
r := httptest.NewRequest("GET", "/accounts", nil)
if queryParams != nil {
q := r.URL.Query()
for k, v := range queryParams {
q.Add(k, v)
for key, values := range queryParams {
for _, value := range values {
q.Add(key, value)
}
}
r.URL.RawQuery = q.Encode()
}
for k, v := range headers {
r.Header.Set(k, v)
for key, values := range headers {
for _, value := range values {
r.Header.Add(key, value)
}
}
return r
}
Expand Down

0 comments on commit 9fd649a

Please sign in to comment.