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

Add a test case on input contract validation #3315

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package opaauthorizerequest

import (
"fmt"
"github.com/stretchr/testify/require"
"github.com/zalando/skipper/filters/builtin"
"io"
"log"
Expand Down Expand Up @@ -617,9 +618,157 @@ func TestCreateFilterArguments(t *testing.T) {
assert.ErrorIs(t, err, filters.ErrInvalidFilterParameters)
}

func TestAuthorizeRequestInputContract(t *testing.T) {
for _, ti := range []struct {
msg string
filterName string
bundleName string
regoQuery string
requestPath string
requestMethod string
requestHeaders http.Header
body string
contextExtensions string
expectedBody string
expectedHeaders http.Header
expectedStatus int
backendHeaders http.Header
removeHeaders http.Header
}{
{
msg: "Input contract validation",
filterName: "opaAuthorizeRequestWithBody",
bundleName: "somebundle.tar.gz",
regoQuery: "envoy/authz/allow",
requestPath: "/users/profile/amal?param=1",
requestMethod: "GET",
contextExtensions: "",
body: `{ "key" : "value" }`,
requestHeaders: http.Header{
"accept": []string{"*/*"},
"user-agent": []string{"curl/7.68.0-DEV"},
"x-request-id": []string{"1455bbb0-0623-4810-a2c6-df73ffd8863a"},
"content-type": {"application/json"},
},
expectedStatus: http.StatusOK,
expectedBody: "Welcome!",
expectedHeaders: map[string][]string{"user-agent": {"curl/7.68.0-DEV"}},
},
} {
t.Run(ti.msg, func(t *testing.T) {
t.Logf("Running test for %v", ti)
clientServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte("Welcome!"))

body, err := io.ReadAll(r.Body)
require.NoError(t, err)
assert.Equal(t, ti.body, string(body))
}))
defer clientServer.Close()

opaControlPlane := opasdktest.MustNewServer(
opasdktest.MockBundle("/bundles/"+ti.bundleName, map[string]string{
"main.rego": `
package envoy.authz

default allow = false

allow {
input.attributes.request.http.path == "/users/profile/amal?param=1"
input.parsed_path == ["users", "profile", "amal"]
input.parsed_query == {"param": ["1"]}
input.attributes.request.http.headers["accept"] == "*/*"
input.attributes.request.http.headers["user-agent"] == "curl/7.68.0-DEV"
input.attributes.request.http.headers["x-request-id"] == "1455bbb0-0623-4810-a2c6-df73ffd8863a"
input.attributes.request.http.headers["content-type"] == "application/json"
input.attributes.metadataContext.filterMetadata["envoy.filters.http.header_to_metadata"].policy_type == "ingress"
opa.runtime().config.labels.environment == "test"
input.parsed_body.key == "value"
}
`,
}),
)

fr := make(filters.Registry)

config := []byte(fmt.Sprintf(`{
"services": {
"test": {
"url": %q
}
},
"bundles": {
"test": {
"resource": "/bundles/{{ .bundlename }}"
}
},
"labels": {
"environment": "test"
},
"plugins": {
"envoy_ext_authz_grpc": {
"path": %q,
"dry-run": false
}
}
}`, opaControlPlane.URL(), ti.regoQuery))

envoyMetaDataConfig := []byte(`{
"filter_metadata": {
"envoy.filters.http.header_to_metadata": {
"policy_type": "ingress"
}
}
}`)

opts := make([]func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error, 0)
opts = append(opts,
openpolicyagent.WithConfigTemplate(config),
openpolicyagent.WithEnvoyMetadataBytes(envoyMetaDataConfig))

opaFactory := openpolicyagent.NewOpenPolicyAgentRegistry(openpolicyagent.WithTracer(tracingtest.NewTracer()))
ftSpec := NewOpaAuthorizeRequestSpec(opaFactory, opts...)
fr.Register(ftSpec)
ftSpec = NewOpaAuthorizeRequestWithBodySpec(opaFactory, opts...)
fr.Register(ftSpec)
fr.Register(builtin.NewSetPath())

r := eskip.MustParse(fmt.Sprintf(`* -> %s("%s", "%s") -> "%s"`, ti.filterName, ti.bundleName, ti.contextExtensions, clientServer.URL))

proxy := proxytest.New(fr, r...)

var bodyReader io.Reader
if ti.body != "" {
bodyReader = strings.NewReader(ti.body)
}

req, err := http.NewRequest(ti.requestMethod, proxy.URL+ti.requestPath, bodyReader)

require.NoError(t, err)

for name, values := range ti.requestHeaders {
for _, value := range values {
req.Header.Add(name, value)
}
}
Comment on lines +749 to +753
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can simply assign here like req.Header = ti.requestHeaders

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prefer to not do this as it internally initiated with default values—including the "User-Agent: Go-http-client/1.1" header leading to some hidden side effects.


rsp, err := proxy.Client().Do(req)
require.NoError(t, err)

assert.Equal(t, ti.expectedStatus, rsp.StatusCode, "HTTP status does not match")

defer rsp.Body.Close()
body, err := io.ReadAll(rsp.Body)
require.NoError(t, err)
assert.Equal(t, ti.expectedBody, string(body), "HTTP Body does not match")
})
}
}

func isHeadersPresent(t *testing.T, expectedHeaders http.Header, headers http.Header) bool {
for headerName, expectedValues := range expectedHeaders {
actualValues, headerFound := headers[headerName]

if !headerFound {
return false
}
Expand Down