feat: match_json_field in remote_json authorizer (#1164)

[jaspeen]

Implements response content match in remote_json authorizer as described in rfc #1169

Related issue(s)

#1169

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  [email protected]) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further Comments

[CLAassistant]

All committers have signed the CLA.

[aeneasr]

Thank you for your contributino and work on this feature! Unfortunately, I feel that the work can not be merged to upstream. I think there are two topics at play here:

1. Oathkeeper assumes that the upstream gives the correct response it expects (versus it must interpret the response)
2. We have an explicit contract that says "yes ok" or "no not ok". As soon as we go into dynamic parsing this becomes muddied
3. In general I think that the evaluation logic is thin. It probably would make more sense to use Google CEL or something like that. But that again is absolute overkill in my view.

Honestly, I don't see a clear way forward with this PR because I don't really think that we should allow dynamic matching here as it is - in my view - easy to misconfigure.

[jaspeen]

@aeneasr Thank's for your review.
I understand your concerns and agree that degree of freedom in such things should be minimal but please look from the user(sre/devops) perspective.
Some external ready-to-go authorizers don't return HTTP 403 and they require some sort of glue to be integrated into an oathkeeper pipeline.
And such kind of dynamic integration glue should be executed somewhere - in oathkleeper itself or custom integration service which then return 403. From my point of view another integration service may increase chance of error or misconfiguration.

I also understand why RP's like envoy or nginx don't bother with authorization response checks - they focused on other things. But for oathkeeper as auth decision engine i see it quite natural.

We can minimize flexibility of this and just check a top level boolean JSON field with specified name in response - e.g. "allow_json_field": "allowed". It will cover both mentioned external authorizer cases. and will explicitly declare that authorization passes if and only if response is successfull and field allowed has value true.

What do you think?

[aeneasr]

To be honest, I'm undecided on this topic.

• As discussed prior there currently no real request transformation in the authn / authz pipeline (only mutator) right now. We forward the request as is to the upstream.
• Adding functionality to "generally" support more upstream servers does makes sense. The upstream server not responding with a 4xx error is a very interesting choice in my view though.
• How far does this go? Do we support XML and others as well? How complex should rules be?

To me, right now, allow_json_field seems like a solution specific to one or two providers that probably isn't applicable for a big enough chunk (assuming that we might also get other fields returned like a scope or whatever).

[jaspeen]

currently no real request transformation in the authn / authz

Not exactly. We have a request payload transformation using jsonnet for remote_json. But my point is actually that this is not a transformation but matching. Here we already doing kind of a matching of remote authorizer HTTP status with 200 and this is going to be an enhanced version of response matching.

Regarding more 'general' solution for matching i think JSON will cover most of the cases with modern software which accessible via HTTP REST (gRPC and so on are currently out of scope anyway for all rules). And we can also use jsonnet for sophisticated multifield matching. Since i see already a lot of expression in gotemplate and jsonnet so i think adding another one is not the best idea.

Anyway when you get any certain idea how to do this properly i will be glad to adopt this solution.