chukmunnlee / caddy-openapi

A Caddy module to validate HTTP request and response against a OpenAPI spec (V3) file
Apache License 2.0
23 stars 7 forks source link

Possible to disable Open Policy check? #5

Closed benjaoming closed 4 months ago

benjaoming commented 10 months ago

From a recent conversation with @georgepadayatti in https://github.com/GovStackWorkingGroup/bb-consent/pull/67#issuecomment-1842852086, it appears that the policy check doesn't work and has to be disabled to make things run.

To be more specific, these are the lines that are suggested to be removed:

https://github.com/chukmunnlee/caddy-openapi/blob/8f6d29d6eb297634da9460b49c187a2b451cd45c/handler.go#L71C1-L91

@chukmunnlee I'll try to get some more input about why it's failing. Would you be interested in commenting out this code or should we run a fork without it?

chukmunnlee commented 10 months ago

Don't load any policies in your Caddyfile

georgepadayatti commented 10 months ago

Don't load any policies in your Caddyfile

Hi @chukmunnlee

Thanks for the quick response. I haven't loaded any policies in the Caddyfile. Here is the Caddyfile for reference - link.

Am I doing something wrong?

georgepadayatti commented 10 months ago

I have used for following Dockerfile to build caddy with the caddy-openapi module:

FROM caddy:2.7.3-builder AS builder

RUN xcaddy build --with github.com/georgepadayatti/caddy-openapi@v0.8.0

FROM caddy:2.7.3-alpine

COPY --from=builder /usr/bin/caddy /usr/bin/caddy

COPY consent-openapi.yaml /consent-openapi.yaml

CMD ["caddy", "run", "--config", "/etc/caddy/Caddyfile"]

At first used the module from upstream, but since the APIs were not responding I decided to remove the resolve policy code block in ServeHTTP func in my fork.

Failing codeblock for reference:

https://github.com/chukmunnlee/caddy-openapi/blob/8f6d29d6eb297634da9460b49c187a2b451cd45c/handler.go#L71-L92

georgepadayatti commented 10 months ago

Sharing the OpenAPI yaml file if it helps in debugging here - https://github.com/decentralised-dataexchange/bb-consent-wg/blob/main/examples/igrantio/consent-openapi.yaml

chukmunnlee commented 10 months ago

Thanks. Looks like a bug then. I'll have a look at it.

chukmunnlee commented 10 months ago

@benjaoming @georgepadayatti

I've tested the code. OPA is not enable if you don't load any policies. I originally suspect your custom field x-specification-usecase, etc might have triggered the evaluation. I could not reproduce that either.

This is the repo I used from testing https://github.com/chukmunnlee/deckofcards

I ran both caddy and a server natively on my machine

Since you have a Dockerfile, I though you might be deploying to k8s. I created a deployment with 1 pod, 2 containers and 1 service; port-forwarded 8080, could not reproduce the error you are discribing. See deckofcards.yaml.

Can you let me know what error you are seeing?

BTW I merged a PR for caddy-openapi but that should not affect OPA part. The PR adds a new placeholder openapi.response_error

georgepadayatti commented 10 months ago

@benjaoming @georgepadayatti

I've tested the code. OPA is not enable if you don't load any policies. I originally suspect your custom field x-specification-usecase, etc might have triggered the evaluation. I could not reproduce that either.

This is the repo I used from testing https://github.com/chukmunnlee/deckofcards

I ran both caddy and a server natively on my machine

  • caddy run --config Caddy
  • go run ./... Tried curl localhost:8080/api/decks and localhost:8080/api/abc123

Since you have a Dockerfile, I though you might be deploying to k8s. I created a deployment with 1 pod, 2 containers and 1 service; port-forwarded 8080, could not reproduce the error you are discribing. See deckofcards.yaml.

Can you let me know what error you are seeing?

BTW I merged a PR for caddy-openapi but that should not affect OPA part. The PR adds a new placeholder openapi.response_error

Will check and revert with more details. Like you suspected it could be the custom extensions.

georgepadayatti commented 10 months ago

@benjaoming @georgepadayatti

I've tested the code. OPA is not enable if you don't load any policies. I originally suspect your custom field x-specification-usecase, etc might have triggered the evaluation. I could not reproduce that either.

This is the repo I used from testing https://github.com/chukmunnlee/deckofcards

I ran both caddy and a server natively on my machine

  • caddy run --config Caddy
  • go run ./... Tried curl localhost:8080/api/decks and localhost:8080/api/abc123

Since you have a Dockerfile, I though you might be deploying to k8s. I created a deployment with 1 pod, 2 containers and 1 service; port-forwarded 8080, could not reproduce the error you are discribing. See deckofcards.yaml.

Can you let me know what error you are seeing?

BTW I merged a PR for caddy-openapi but that should not affect OPA part. The PR adds a new placeholder openapi.response_error

It's a mistake on my end. The issue is not coming from resolve policy rather from request validation. This happens when security mechanisms are mentioned in the OpenAPI spec and request doesn't contain the necessary.

Issue occurs in the following codeblock:

https://github.com/chukmunnlee/caddy-openapi/blob/ae0eff4f115fe673c4f9faea7a18b13f703b14a7/handler.go#L57-L59

Here you are trying to cast err which contains data of type *openapi3filter.SecurityRequirementsError rather than *openapi3filter.RequestError.

This can be fixed by handling *openapi3filter.SecurityRequirementsError while request validation.

georgepadayatti commented 10 months ago

I have made a PR to handle the same - https://github.com/chukmunnlee/caddy-openapi/pull/6