Skip to content

Commit

Permalink
Fix client request body buffer options in combination with ac (#415)
Browse files Browse the repository at this point in the history
* Fix client request body buffer options in combination with access controls

* Add changelog entry

* Update eval/buffer.go

Co-authored-by: Johannes Koch <53434855+johakoch@users.noreply.github.com>

* refactor test for additional cases

* add cases for token in json_body and body

Co-authored-by: Johannes Koch <53434855+johakoch@users.noreply.github.com>
Co-authored-by: Johannes Koch <johannes.koch@avenga.com>
  • Loading branch information
3 people authored Jan 6, 2022
1 parent 2204229 commit 9b81fa7
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* possible race conditions while updating JWKS for the [JWT access control](./docs/REFERENCE.md#jwt-block) ([#398](https://github.com/avenga/couper/pull/398))
* panic while accessing primitive variables with a key ([#377](https://github.com/avenga/couper/issues/377))
* [`default()`](./docs/REFERENCE.md#functions) function continues to their fallback value if this is a string type and an argument evaluates to an empty string ([#408](https://github.com/avenga/couper/issues/408))
* missing read of client-request bodies if related variables are used in referenced access controls only (e.g. jwt token source) ([#415](https://github.com/avenga/couper/pull/415))

* **Dependencies**
* Update modules for [OpenAPI](./docs/REFERENCE.md#openapi-block) validation ([#399](https://github.com/avenga/couper/pull/399))
Expand Down
7 changes: 7 additions & 0 deletions config/runtime/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,13 @@ func NewServerConfiguration(conf *config.Couper, log *logrus.Entry, memStore *ca
return nil, err
}

// Evaluate access-control related buffer options.
acBodies := bodiesWithACBodies(conf.Definitions,
newAC(srvConf, parentAPI).
Merge(config.
NewAccessControl(endpointConf.AccessControl, endpointConf.DisableAccessControl)).List(), nil)
epOpts.BufferOpts |= eval.MustBuffer(acBodies...)

errorHandlerDefinitions := ACDefinitions{ // misuse of definitions obj for now
"endpoint": &AccessControl{ErrorHandler: endpointConf.ErrorHandler},
}
Expand Down
2 changes: 1 addition & 1 deletion eval/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (i BufferOption) Response() bool {
}

// MustBuffer determines if any of the hcl.bodies makes use of 'body', 'form_body' or 'json_body' or
// of known attributes and variables which requires a parsed client-request body.
// of known attributes and variables which require a parsed client-request or backend-response body.
func MustBuffer(bodies ...hcl.Body) BufferOption {
result := BufferNone

Expand Down
77 changes: 77 additions & 0 deletions server/http_endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"path/filepath"
"reflect"
"strconv"
Expand Down Expand Up @@ -757,3 +758,79 @@ func TestEndpointErrorHandler(t *testing.T) {
})
}
}

func TestEndpointACBufferOptions(t *testing.T) {
client := test.NewHTTPClient()
helper := test.New(t)

shutdown, hook := newCouper(filepath.Join(testdataPath, "17_couper.hcl"), helper)
defer shutdown()

invalidToken := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.p_L2kBaXuGvD2AhW5WEheAKLErYXPDR-LKj_dZ5G_XI"
validToken := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.6M2CwQMZ-PkeSyREi5scviq0EilhUUSgax6W9TmxmS8"

urlencoded := func(token string) string {
return url.Values{"token": []string{token}}.Encode()
}
json := func(token string) string {
return fmt.Sprintf("{%q: %q}", "token", token)
}
plain := func(token string) string {
return token
}

type testcase struct {
name string
path string
token string
bodyFunc func(string) string
contentType string
expectedStatus int
expectedErrorType string
}

for _, tc := range []testcase{
{"with ac (token in-form_body) and wrong token", "/in-form_body", invalidToken, urlencoded, "application/x-www-form-urlencoded", http.StatusForbidden, "jwt"},
{"with ac (token in-form_body) and without token", "/in-form_body", "", urlencoded, "application/x-www-form-urlencoded", http.StatusUnauthorized, "jwt_token_missing"},
{"with ac (token in-form_body) and valid token", "/in-form_body", validToken, urlencoded, "application/x-www-form-urlencoded", http.StatusOK, ""},
{"with ac (token in-json_body) and wrong token", "/in-json_body", invalidToken, json, "application/json", http.StatusForbidden, "jwt"},
{"with ac (token in-json_body) and without token", "/in-json_body", "", json, "application/json", http.StatusUnauthorized, "jwt_token_missing"},
{"with ac (token in-json_body) and valid token", "/in-json_body", validToken, json, "application/json", http.StatusOK, ""},
{"with ac (token in-body) and wrong token", "/in-body", invalidToken, plain, "text/plain", http.StatusForbidden, "jwt"},
{"with ac (token in-body) and without token", "/in-body", "", plain, "text/plain", http.StatusUnauthorized, "jwt_token_missing"},
{"with ac (token in-body) and valid token", "/in-body", validToken, plain, "text/plain", http.StatusOK, ""},
{"without ac", "/without-ac", "", nil, "text/plain", http.StatusOK, ""},
} {
t.Run(tc.name, func(st *testing.T) {
hook.Reset()
h := test.New(st)

body := ""
if tc.bodyFunc != nil {
body = tc.bodyFunc(tc.token)
}
req, err := http.NewRequest(http.MethodPost, "http://domain.local:8080"+tc.path, strings.NewReader(body))
h.Must(err)

req.Header.Set("Content-Type", tc.contentType)

res, err := client.Do(req)
h.Must(err)

if res.StatusCode != tc.expectedStatus {
st.Errorf("want: %d, got: %d", tc.expectedStatus, res.StatusCode)
}

if tc.expectedErrorType != "" {
for _, e := range hook.AllEntries() {
if e.Data["type"] != "couper_access" {
continue
}
if e.Data["error_type"] != tc.expectedErrorType {
st.Errorf("want: %q, got: %v", tc.expectedErrorType, e.Data["error_type"])
}
}
}
})
}
}
48 changes: 48 additions & 0 deletions server/testdata/endpoints/17_couper.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
server {
endpoint "/in-form_body" {
access_control = ["in-form_body"]
response {
body = request.url
}
}

endpoint "/in-json_body" {
access_control = ["in-json_body"]
response {
body = request.url
}
}

endpoint "/in-body" {
access_control = ["in-body"]
response {
body = request.url
}
}

endpoint "/without-ac" {
response {
body = request.url
}
}
}

definitions {
jwt "in-form_body" {
signature_algorithm = "HS256"
key = "test123"
token_value = request.form_body.token[0]
}

jwt "in-json_body" {
signature_algorithm = "HS256"
key = "test123"
token_value = request.json_body.token
}

jwt "in-body" {
signature_algorithm = "HS256"
key = "test123"
token_value = request.body
}
}

0 comments on commit 9b81fa7

Please sign in to comment.