opa icon indicating copy to clipboard operation
opa copied to clipboard

Handle incorrect user input in OPA envoy plugin

Open rudrakhp opened this issue 1 year ago • 5 comments

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?

rudrakhp avatar Jul 29 '24 11:07 rudrakhp

@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.

ashutosh-narkar avatar Jul 29 '24 17:07 ashutosh-narkar

@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.

rudrakhp avatar Jul 30 '24 17:07 rudrakhp

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.

ashutosh-narkar avatar Jul 30 '24 17:07 ashutosh-narkar

@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

rudrakhp avatar Jul 31 '24 08:07 rudrakhp

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.

ashutosh-narkar avatar Jul 31 '24 15:07 ashutosh-narkar

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.

stale[bot] avatar Aug 31 '24 03:08 stale[bot]