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

Fix client request body buffer options in combination with ac #415

Merged
merged 5 commits into from
Jan 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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 requires a parsed client-request or backend-repsonse body.
malud marked this conversation as resolved.
Show resolved Hide resolved
func MustBuffer(bodies ...hcl.Body) BufferOption {
result := BufferNone

Expand Down
53 changes: 53 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,55 @@ 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()

type testcase struct {
name string
path string
token string
expectedStatus int
expectedErrorType string
}

for _, tc := range []testcase{
{"with ac and wrong token", "/with-ac", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.p_L2kBaXuGvD2AhW5WEheAKLErYXPDR-LKj_dZ5G_XI", http.StatusForbidden, "jwt"},
{"with ac and without token", "/with-ac", "", http.StatusUnauthorized, "jwt_token_missing"},
{"with ac and valid token", "/with-ac", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.6M2CwQMZ-PkeSyREi5scviq0EilhUUSgax6W9TmxmS8", http.StatusOK, ""},
{"without ac", "/without-ac", "", http.StatusOK, ""},
} {
t.Run(tc.name, func(st *testing.T) {
hook.Reset()
h := test.New(st)

body := url.Values{"token": []string{tc.token}}.Encode()
req, err := http.NewRequest(http.MethodPost, "http://domain.local:8080"+tc.path, strings.NewReader(body))
h.Must(err)

req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

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"])
}
}
}
})
}
}
23 changes: 23 additions & 0 deletions server/testdata/endpoints/17_couper.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
server {
access_control = ["in-form_body"]
endpoint "/with-ac" { # error_type=jwt ... status=403
response {
body = request.url
}
}

endpoint "/without-ac" { # error_type=jwt_token_missing ... status=401
disable_access_control = ["in-form_body"]
response {
body = request.url
}
}
}

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