Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: pass only essential and configured headers to authenticator #952

Merged
merged 42 commits into from
Jun 23, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
f0cb78f
fix: added gzip support for cookie_session authenticator
gen1us2k Apr 11, 2022
6999a5f
fix: added proxy_headers config variable for authenticators
gen1us2k May 9, 2022
5b94457
fix: better design for forwardRequestToSessionStore
gen1us2k May 9, 2022
50f01b1
fix: better solution to convert array to map
gen1us2k May 10, 2022
85d391d
renamed field
gen1us2k May 13, 2022
0cceab9
added header package to improve readability
gen1us2k May 13, 2022
6cbc29a
fixed build
gen1us2k May 13, 2022
f4b9936
updated schema
gen1us2k May 13, 2022
4f5d8af
removed old test
gen1us2k May 13, 2022
5c35b6d
added tests
gen1us2k May 13, 2022
27e785a
better backwards compatibility
gen1us2k May 23, 2022
e456520
Merge branch 'master' into gzip_support
gen1us2k May 23, 2022
0a9c066
fix: added gzip support for cookie_session authenticator
gen1us2k Apr 11, 2022
9906ce4
fix: added proxy_headers config variable for authenticators
gen1us2k May 9, 2022
198902c
fix: better design for forwardRequestToSessionStore
gen1us2k May 9, 2022
134c9dc
fix: better solution to convert array to map
gen1us2k May 10, 2022
eea93ee
renamed field
gen1us2k May 13, 2022
3d4333d
added header package to improve readability
gen1us2k May 13, 2022
6f0982f
fixed build
gen1us2k May 13, 2022
8941f8b
updated schema
gen1us2k May 13, 2022
9417a30
removed old test
gen1us2k May 13, 2022
81c0fae
added tests
gen1us2k May 13, 2022
aeba3ce
better backwards compatibility
gen1us2k May 23, 2022
b671e8b
Update pipeline/authn/authenticator_cookie_session.go
gen1us2k May 30, 2022
f7f6da9
Merge
gen1us2k May 30, 2022
0ca5c51
Small refactoring
gen1us2k May 30, 2022
339e8e7
fix: Don't use maps anymore
gen1us2k Jun 9, 2022
affc702
Merge branch 'master' into gzip_support
gen1us2k Jun 9, 2022
0bbc5fd
Small fixes
gen1us2k Jun 10, 2022
da038f8
fixed test
gen1us2k Jun 10, 2022
690d766
Merge branch 'master' into gzip_support
gen1us2k Jun 21, 2022
ba19be3
chore: code review
aeneasr Jun 23, 2022
afaf5af
Remove header constants
gen1us2k Jun 23, 2022
fe4bd05
fixed tests. Check only canonical header names
gen1us2k Jun 23, 2022
5e16805
Merge branch 'master' into gzip_support
gen1us2k Jun 23, 2022
046422f
Drop header values
gen1us2k Jun 23, 2022
f2d31ea
Added tests for the cookie session authenticator
gen1us2k Jun 23, 2022
a19b283
Added test for token authenticator
gen1us2k Jun 23, 2022
c66ba2a
run prettifier
gen1us2k Jun 23, 2022
ab72681
Fixed linter
gen1us2k Jun 23, 2022
4b735e3
Drop dead headers
gen1us2k Jun 23, 2022
028c091
not canonical header
gen1us2k Jun 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .schema/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,14 @@
"POST"
]
},
"proxy_headers": {
gen1us2k marked this conversation as resolved.
Show resolved Hide resolved
"title": "Set Proxy HTTP Headers",
"type": "array",
"description": "Set HTTP Headers allowed to proxy to upstream.",
"additionalProperties": {
"type": "string"
}
},
"additional_headers": {
"title": "Set Additional HTTP Headers",
"type": "object",
Expand Down Expand Up @@ -552,6 +560,14 @@
"POST"
]
},
"proxy_headers": {
"title": "Set Proxy HTTP Headers",
"type": "array",
"description": "Set HTTP Headers allowed to proxy to upstream.",
"additionalProperties": {
"type": "string"
}
},
"additional_headers": {
"title": "Set Additional HTTP Headers",
"type": "object",
Expand Down
9 changes: 9 additions & 0 deletions pipeline/authn/authenticator.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ type MatchContext struct {
Method string `json:"method"`
Header http.Header `json:"header"`
}
type AuthenticatorForwardConfig struct {
CheckSessionURL string
PreserveQuery bool
PreservePath bool
PreserveHost bool
ProxyHeadersMap map[string]string
SetHeaders map[string]string
ForceMethod string
}

func (a *AuthenticationSession) SetHeader(key, val string) {
if a.Header == nil {
Expand Down
23 changes: 22 additions & 1 deletion pipeline/authn/authenticator_bearer_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,22 @@ type AuthenticatorBearerTokenConfiguration struct {
PreserveHost bool `json:"preserve_host"`
ExtraFrom string `json:"extra_from"`
SubjectFrom string `json:"subject_from"`
ProxyHeaders []string `json:"proxy_headers"`
SetHeaders map[string]string `json:"additional_headers"`
ForceMethod string `json:"force_method"`
ProxyHeadersMap map[string]string `json:"-"`
}

func (a *AuthenticatorBearerTokenConfiguration) ToAuthenticatorForwardConfig() *AuthenticatorForwardConfig {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A better approach would be to define an interface which is consumed by forwardRequestToSessionStore

type AuthenticatorForwardConfigProvider interface {
  GetCheckSessionURL() string
  // ...
}

// Type Guard
var _ AuthenticatorForwardConfigProvider = (*AuthenticatorBearerTokenConfiguration)(nil)

or alternatively use an embedded struct:

type authenticatorForwardConfig struct {
 	CheckSessionURL    string `json:"..."`
}

type AuthenticatorBearerTokenConfiguration struct {
  authenticatorForwardConfig

  // ...
}

// ...
ody, err := forwardRequestToSessionStore(r, cf. authenticatorForwardConfig)

That will save some copy & pasting and potential sources of errors (e.g. forgetting to copy a field) and is in general a cleaner approach to Go code :)

return &AuthenticatorForwardConfig{
CheckSessionURL: a.CheckSessionURL,
PreserveQuery: a.PreserveQuery,
PreservePath: a.PreservePath,
PreserveHost: a.PreserveHost,
ProxyHeadersMap: a.ProxyHeadersMap,
SetHeaders: a.SetHeaders,
ForceMethod: a.ForceMethod,
}
}

type AuthenticatorBearerToken struct {
Expand Down Expand Up @@ -71,6 +85,13 @@ func (a *AuthenticatorBearerToken) Config(config json.RawMessage) (*Authenticato
if len(c.SubjectFrom) == 0 {
c.SubjectFrom = "sub"
}
if len(c.ProxyHeaders) == 0 {
c.ProxyHeaders = []string{"Authorization", "Cookie"}
}

for _, h := range c.ProxyHeaders {
c.ProxyHeadersMap[h] = h
}

return &c, nil
}
Expand All @@ -86,7 +107,7 @@ func (a *AuthenticatorBearerToken) Authenticate(r *http.Request, session *Authen
return errors.WithStack(ErrAuthenticatorNotResponsible)
}

body, err := forwardRequestToSessionStore(r, cf.CheckSessionURL, cf.PreserveQuery, cf.PreservePath, cf.PreserveHost, cf.SetHeaders, cf.ForceMethod)
body, err := forwardRequestToSessionStore(r, cf.ToAuthenticatorForwardConfig())
if err != nil {
return err
}
Expand Down
14 changes: 7 additions & 7 deletions pipeline/authn/authenticator_bearer_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_query": true}`),
config: []byte(`{"preserve_query": true, "proxy_headers": ["Authorization"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand All @@ -113,7 +113,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_path": true, "preserve_query": false}`),
config: []byte(`{"preserve_path": true, "preserve_query": false, "proxy_headers": ["Authorization"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand All @@ -128,7 +128,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_path": true, "force_method": "GET"}`),
config: []byte(`{"preserve_path": true, "force_method": "GET", "proxy_headers": ["Authorization"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand All @@ -145,7 +145,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_path": false, "preserve_query": true}`),
config: []byte(`{"preserve_path": false, "preserve_query": true, "proxy_headers": ["Authorization"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand All @@ -160,7 +160,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_path": true, "preserve_query": true, "check_session_url": "http://origin-replaced-in-test/configured/path?q=configured-query"}`),
config: []byte(`{"preserve_path": true, "preserve_query": true, "check_session_url": "http://origin-replaced-in-test/configured/path?q=configured-query", "proxy_headers": ["Authorization", "X-Forwared-Host"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand All @@ -176,7 +176,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_host": true}`),
config: []byte(`{"preserve_host": true, "proxy_headers": ["Authorization"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand All @@ -193,7 +193,7 @@ func TestAuthenticatorBearerToken(t *testing.T) {
w.WriteHeader(200)
w.Write([]byte(`{"sub": "123"}`))
},
config: []byte(`{"preserve_host": true, "additional_headers": {"X-Foo": "bar","X-Forwarded-For": "not-some-host"}}`),
config: []byte(`{"preserve_host": true, "additional_headers": {"X-Foo": "bar","X-Forwarded-For": "not-some-host"}, "proxy_headers":["Authorization"]}`),
expectErr: false,
expectSess: &AuthenticationSession{
Subject: "123",
Expand Down
47 changes: 35 additions & 12 deletions pipeline/authn/authenticator_cookie_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ type AuthenticatorCookieSessionConfiguration struct {
ExtraFrom string `json:"extra_from"`
SubjectFrom string `json:"subject_from"`
PreserveHost bool `json:"preserve_host"`
ProxyHeaders []string `json:"proxy_headers"`
SetHeaders map[string]string `json:"additional_headers"`
ForceMethod string `json:"force_method"`
ProxyHeadersMap map[string]string `json:"-"`
}

type AuthenticatorCookieSession struct {
Expand All @@ -49,6 +51,17 @@ func NewAuthenticatorCookieSession(c configuration.Provider) *AuthenticatorCooki
}
}

func (a *AuthenticatorCookieSessionConfiguration) ToAuthenticatorForwardConfig() *AuthenticatorForwardConfig {
return &AuthenticatorForwardConfig{
CheckSessionURL: a.CheckSessionURL,
PreserveQuery: a.PreserveQuery,
PreservePath: a.PreservePath,
PreserveHost: a.PreserveHost,
ProxyHeadersMap: a.ProxyHeadersMap,
SetHeaders: a.SetHeaders,
ForceMethod: a.ForceMethod,
}
}
func (a *AuthenticatorCookieSession) GetID() string {
return "cookie_session"
}
Expand All @@ -75,6 +88,12 @@ func (a *AuthenticatorCookieSession) Config(config json.RawMessage) (*Authentica
if len(c.SubjectFrom) == 0 {
c.SubjectFrom = "subject"
}
if len(c.ProxyHeaders) == 0 {
c.ProxyHeaders = []string{"Authorization", "Cookie"}
}
for _, h := range c.ProxyHeaders {
c.ProxyHeadersMap[h] = h
}

return &c, nil
}
Expand All @@ -89,7 +108,7 @@ func (a *AuthenticatorCookieSession) Authenticate(r *http.Request, session *Auth
return errors.WithStack(ErrAuthenticatorNotResponsible)
}

body, err := forwardRequestToSessionStore(r, cf.CheckSessionURL, cf.PreserveQuery, cf.PreservePath, cf.PreserveHost, cf.SetHeaders, cf.ForceMethod)
body, err := forwardRequestToSessionStore(r, cf.ToAuthenticatorForwardConfig())
if err != nil {
return err
}
Expand Down Expand Up @@ -129,40 +148,42 @@ func cookieSessionResponsible(r *http.Request, only []string) bool {
return false
}

func forwardRequestToSessionStore(r *http.Request, checkSessionURL string, preserveQuery bool, preservePath bool, preserveHost bool, setHeaders map[string]string, m string) (json.RawMessage, error) {
reqUrl, err := url.Parse(checkSessionURL)
func forwardRequestToSessionStore(r *http.Request, cf *AuthenticatorForwardConfig) (json.RawMessage, error) {
reqUrl, err := url.Parse(cf.CheckSessionURL)
if err != nil {
return nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to parse session check URL: %s", err))
}

if !preservePath {
if !cf.PreservePath {
reqUrl.Path = r.URL.Path
}

if !preserveQuery {
if !cf.PreserveQuery {
reqUrl.RawQuery = r.URL.RawQuery
}

if m == "" {
m = r.Method
if cf.ForceMethod == "" {
cf.ForceMethod = r.Method
}

req := http.Request{
Method: m,
Method: cf.ForceMethod,
URL: reqUrl,
Header: http.Header{},
}

// We need to make a COPY of the header, not modify r.Header!
// We need to copy only essential and configurable headers
for k, v := range r.Header {
req.Header[k] = v
if _, ok := cf.ProxyHeadersMap[k]; ok {
req.Header[k] = v
}
}

for k, v := range setHeaders {
for k, v := range cf.SetHeaders {
req.Header.Set(k, v)
}

if preserveHost {
if cf.PreserveHost {
req.Header.Set("X-Forwarded-Host", r.Host)
}

Expand All @@ -171,6 +192,8 @@ func forwardRequestToSessionStore(r *http.Request, checkSessionURL string, prese
return nil, helper.ErrForbidden.WithReason(err.Error()).WithTrace(err)
}

defer res.Body.Close()

if res.StatusCode == 200 {
body, err := ioutil.ReadAll(res.Body)
if err != nil {
Expand Down
26 changes: 26 additions & 0 deletions pipeline/authn/authenticator_cookie_session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package authn_test

import (
"bytes"
"compress/gzip"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -246,6 +247,31 @@ func TestAuthenticatorCookieSession(t *testing.T) {
Extra: map[string]interface{}{"session": map[string]interface{}{"foo": "bar"}, "identity": map[string]interface{}{"id": "123"}},
}, session)
})
t.Run("description=should handle gzip requests", func(t *testing.T) {
responseBody := `{"identity": {"id": "123"}, "session": {"foo": "bar"}}`
requestRecorder := &RequestRecorder{}
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
requestRecorder.requests = append(requestRecorder.requests, r)
requestBody, _ := ioutil.ReadAll(r.Body)
requestRecorder.bodies = append(requestRecorder.bodies, requestBody)
w.Header().Set("Content-Encoding", "gzip")
w.WriteHeader(http.StatusOK)
gz := gzip.NewWriter(w)
gz.Write([]byte(responseBody))
gz.Close()
}))
err := pipelineAuthenticator.Authenticate(
makeRequest("GET", "/", "", map[string]string{"sessionid": "zyx"}, ""),
session,
json.RawMessage(fmt.Sprintf(`{"check_session_url": "%s", "subject_from": "identity.id", "extra_from": "@this"}`, testServer.URL)),
nil,
)
require.NoError(t, err, "%#v", errors.Cause(err))
assert.Equal(t, &AuthenticationSession{
Subject: "123",
Extra: map[string]interface{}{"session": map[string]interface{}{"foo": "bar"}, "identity": map[string]interface{}{"id": "123"}},
}, session)
})
})
}

Expand Down