open-policy-agent / opa

Open Policy Agent (OPA) is an open source, general-purpose policy engine.
https://www.openpolicyagent.org
Apache License 2.0
9.53k stars 1.32k forks source link

Handle incorrect user input in OPA envoy plugin #6901

Closed rudrakhp closed 1 day ago

rudrakhp commented 1 month ago

Short description

OPA version: 0.63.0 Today the OPA envoy plugin tries to parse the URL and return a generic error in case it fails. Code pointer.

Steps To Reproduce

Pass an invalid URL to an OPA server running the envoy plugin.

Expected behavior

Status returned should be 400/BadRequest, but is 503/Server error instead.

Additional context

Is there any way to identify these kinds of errors and return user defined status codes instead?

ashutosh-narkar commented 1 month ago

@rudrakhp the return status code is something that's controlled by Envoy. By default, when OPA returns an error, Envoy should send back a HTTP 403 Forbidden. This behavior can be controlled by the status_on_error field in the Envoy config. More details here. I'm surprised you're getting a 503.

rudrakhp commented 1 month ago

@ashutosh-narkar you are right, we have the status_on_error set to 503. But I was more concerned about handling all OPA errors with a single status. For example, the incorrect path error should return a different status (4XX) than say an OPA internal server error (5XX). That's not possible today? PS: Thanks for recategorising this as a question, incorrectly raised it as a bug.

ashutosh-narkar commented 1 month ago

For example, the incorrect path error should return a different status (4XX) than say an OPA internal server error (5XX). That's not possible today?

Currently OPA is configured to return 500, 403 and 200 codes. Feel free to submit a PR if you'd like to improve this. You should be able to start here.

rudrakhp commented 1 month ago

@ashutosh-narkar thanks for the pointer, while I have a look at this can you also confirm if the status that the plugin returns can be transparently exposed to envoy as well? Do we need to configure a single status_on_error in the envoy filter? From the documentation it does look like envoy will pass the status set as is if status_on_error is not configured explicitly:

// Status "OK" allows the request. Any other status indicates the request should be denied, and
// for HTTP filter, if not overridden by :ref:`denied HTTP response status <envoy_v3_api_field_service.auth.v3.DeniedHttpResponse.status>`
// Envoy sends "403 Forbidden" HTTP status code by default.
CheckResponse.Status
// This field allows the authorization service to send an HTTP response status code to the
// downstream client. If not set, Envoy sends "403 Forbidden" HTTP status code by default.
DeniedHttpResponse.Status
ashutosh-narkar commented 1 month ago

while I have a look at this can you also confirm if the status that the plugin returns can be transparently exposed to envoy as well?

The status_on_error is when the server returns an error or cannot be reached. I would imagine the allow/deny response status gets passed as is. But if you return an error along with the response, then I think 403 will be the default. Would be good to verify this.

stale[bot] commented 2 weeks ago

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.