oathkeeper icon indicating copy to clipboard operation
oathkeeper copied to clipboard

Implement GRPC response handler in Decisions API

Open lsjostro opened this issue 7 years ago • 14 comments
trafficstars

Is your feature request related to a problem? Please describe.]

Implement GRPC response in the Decisions API. If Content-type is application/grpc ORY Oathkeeper should response with the appropriate GRPC response.

Describe the solution you'd like

Something like this in the judge handler:

func grpcHandlerFunc(rpcServer *rpc.Server, other http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        ct := r.Header.Get("Content-Type")
        if r.ProtoMajor == 2 && strings.Contains(ct, "application/grpc") {
            rpcServer.ServeHTTP(w, r)
        } else {
            other.ServeHTTP(w, r)
        }
    })
}

lsjostro avatar Nov 05 '18 10:11 lsjostro

Would it be possible to give more context on this? I've never used grpc myself, so I lack some background. Is grpc running over HTTP(s)? Are the same headers used and do they represent the same things? If grpc runs as a (binary) format over HTTP, why do we even care here? We're not transforming/modifying the payload, right?

aeneasr avatar Nov 05 '18 10:11 aeneasr

Yes gRPC runs over HTTP/2. The headers are more or less the same as in HTTP/1 /w Authorization headers etc (read more here https://apievangelist.com/2018/02/05/headers-used-for-grpc-over-http2/). But gRPC have some addtitional headers as well. gRPC client except "binary" payload back, whenever a json payload is returned it will not be recognized but the grpc client, like a auth failure from oathkeeper.

Empty body response would be fine but with the grpc-status and grpc-message header with the error.

But your are totally right whenever the request is granted by oathkeeper everything works as excepted. This fix would make the gRPC client act accordingly if we get a auth failure for instance.

some more info here: https://www.d3void.net/post/grpc-with-http/

lsjostro avatar Nov 05 '18 11:11 lsjostro

Ok, I think this requires a bit more thinking around gRPC in general:

  1. Do we provide gRPC APIs alongside REST ones?
  2. How do we document, generate, and publish gRPC APIs?
  3. What security mechanisms of gRPC (if any) can we use or enhance?

I think this will be a longer discussion, but it will be a good one!

aeneasr avatar Nov 05 '18 11:11 aeneasr

A quick fix which works sufficient is to do something like this: image

from previously getting this reponse: rpc error: code = Internal desc = transport: received the unexpected content-type "application/json" and now getting: rpc error: code = Unauthenticated desc = Unauthorized

lsjostro avatar Nov 05 '18 12:11 lsjostro

probably would be enough to implement the content type in https://github.com/ory/herodot

this workaround is ok for me now. image

lsjostro avatar Nov 05 '18 12:11 lsjostro

Yeah, I think it makes sense to have the workaround in oathkeeper directly. We'll need a bit more work on herodot to support grpc properly. Feel free to PR!

aeneasr avatar Nov 07 '18 16:11 aeneasr

Looks like this issue still prevails. Any workaround in 2020?

rverma-jm avatar Jan 18 '20 16:01 rverma-jm

PRs and contributions are always welcome, so if you want to give this a shot @rverma-jm

aeneasr avatar Jan 18 '20 17:01 aeneasr

@lsjostro Are you please able to indicate where you would add this work around currently within the current release?

Would it be here:

https://github.com/ory/oathkeeper/blob/master/api/decision.go#L86

And would you completely replace the current function?

Thanks.

a75c6 avatar Apr 06 '20 04:04 a75c6

We are also interested in this feature.

Is there any progress on this? If not, is there anything we still need to discuss before changes can be made?

I could look into this and try to contribute this feature, if there are no blockers.

hypnoglow avatar Nov 20 '20 09:11 hypnoglow

Thank you, we are currently implementing GRPC in Ory Keto and plan to do the same in Ory Oathkeeper once we habe the build and toolchain figured out! If you want check over to https://github.com/ory/keto for issues on GRPC or reach out to @zepatrik / Patrik (ory) on Slack

On 20. Nov 2020, at 10:44, Igor Zibarev [email protected] wrote:

 We are also interested in this feature.

Is there any progress on this? If not, is there anything we still need to discuss before changes can be made?

I could look into this and try to contribute this feature, if there are no blockers.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

aeneasr avatar Nov 24 '20 08:11 aeneasr

I am marking this issue as stale as it has not received any engagement from the community or maintainers in over half a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • open a new issue with updated details and a plan on resolving the issue.

We are cleaning up issues every now and then, primarily to keep the 4000+ issues in our backlog in check and to prevent maintainer burnout. Burnout in open source maintainership is a widespread and serious issue. It can lead to severe personal and health issues as well as enabling catastrophic attack vectors.

Thank you for your understanding and to anyone who participated in the issue! 🙏✌️

If you feel strongly about this issues and have ideas on resolving it, please comment. Otherwise it will be closed in 30 days!

github-actions[bot] avatar Sep 21 '21 00:09 github-actions[bot]

Incorrect stalebot detection - this was assigned a milestone.

aeneasr avatar Sep 21 '21 05:09 aeneasr

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • leave a comment and describe in detail why this issue is critical for your use case;
  • open a new issue with updated details and a plan on resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneous you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

github-actions[bot] avatar Sep 22 '22 00:09 github-actions[bot]

This is still an issue in 2024

meysam81 avatar Feb 12 '24 07:02 meysam81