Skipper: OIDC and authZ support

[universam1]

Is your feature request related to a problem? Please describe.
Authentication (authN) via OIDC seems to be working fine. However there appears to be no information if and how authorization filtering (authZ) can be implemented?

Describe the solution you would like
Skipper should be able to parse the JWT and determine path based access based on group membership.

Describe alternatives you've considered (optional)
There is no other workaround than dropping Skipper and going with some other OIDC proxy

Would you like to work on it?
Yes if support is provided

[szuecs]

The authz part is missing, yes.
It should be ok to implement. The question is how to specify the configuration for authz in the filter.

[universam1]

Would a subsequent, independent filter like jwt-contains provide advantages:

• separation of concerns
• independent of OIDC implementation
• could be used at a later skipper instance as the JWT token is forwarded
• would allow abstraction, ex. on cluster level an authN gatekeeper and on application level authZ: advantage the app-ingress doesn’t have to know client secrets!

[szuecs]

We could put the parsed IDToken in the statebag in the authN (current) filter and use a new filter, like validateAnyClaims, validateAllClaims to do the authZ. Not sure about the forward, but we can use the already existent forwardToken() (maybe needs some little changes) to forward the statebag entry to the backend.

@aryszka what do you think?

@universam1 what do you think is authZ filter via Claims check (Any and All are considered set operations) good enough? Do you have other ideas?

[universam1]

@szuecs Unfortunately not for us as we use it as of now. Consider following example jwt:

JSON { "aud": "test", "exp": 1515269245, "iat": 1515182845, "email": "[email protected]", "groups": [ "group_one", "group_two" ], "name": "Rohith" }
The mere check of the claim groups does not suffice, we have to check if it contains a specific one.
It would even be better to support matching. May be that would even be a simpler approach.

This implementation might be a good ref: https://www.keycloak.org/docs/latest/securing_apps/index.html#group-claims

[szuecs]

@universam1 we have a similar custom thing called "teams".
How would you like to express this in the filter?
I also wonder if a list based key is enough or if there might be use case that need nested structures.

[universam1]

Haven't seen nested structures so far, but I might be wrong.
Maybe a validateAnyClaims could be considered as a contains-style definition in case of an list, while the validateAllClaims could be explicit. If necessary a KV version might be added.

However, if that all would have to be covered by a single filter I believe it would be almost impossible. Rather a subsequent filter chain would give way more degress of freedom to combine use cases

[szuecs]

@universam1 I agree the idea of authz filters seems to be the way to go.
So I would say we create 2 filters to do authz and change oidc filters to put the parsed ID Token into ctx.StateBag()[jwtIDTokenKey]. The 2 authz filters can work read from the stateBag and validate the settings jwtValidateAnyClaims() and jwtValidateAllClaims().

One open question is how to handle the cookie, that we set and use in later calls from oidc filters. What happens in the authz filters, if we have a valid cookie for this route?

[universam1]

Looks like we are hitting also #1089 @szuecs - with our given identity provider I'm currently unable to change that situation in order to verify the flow of the cookie ...

[universam1]

@szuecs Regarding your question how to express the filter syntax, my idea is to make use of the fact that a JWT is a json data type. Then we can simply use jq syntax to express our search and don’t have to bother about reinventing the wheel!
There are libs that provide this ready to use, example:
https://github.com/savaki/jq
https://github.com/itchyny/gojq

Let me know what you think

[universam1]

@szuecs if you like me to come up with a proposal implementation please let me know

[szuecs]

@universam1 is map[string]string not enough?
I would not like to have too much dependencies for this (gojq) and the savaki/jq seems to be either state done or abandoned.

[universam1]

I would not like to have too much dependencies for this (gojq) and the savaki/jq seems to be either state done or abandoned.

I understand the objection, and I avoid pulling dependencies as much as possible!

@universam1 is map[string]string not enough?

I guess it is not, at least I see no other, smart solution. Example if I have to 'allow members of computer department', that is a subselection of groups array, how would I achieve it?

{
  "groups": [
    "CD-xyz",
    "Group-asdf"
  ]
}

in jq language I could just state:

.groups[] | startswith("CD-")

Again, might be overkill, however the Benchmarks of the project appear to be promising. If you have another idea absolutely open to though!

[szuecs]

What we currently have is Any and All.
With Any you could configure claimsAny("groups", "CD-xyz", "groups", "CD-abc").
With All you could have claimsAll("groups", "CD-xyz", "groups", "Group-asdf").

You would need to have map[string][]string.
Of course jq is much more flexible, but not sure if this is worth the dependency.

[universam1]

Guess I could live with that when it simplifies things a lot. 👍
Of course I would like to have a simple wildcard "*" support one day, but that could be decoupled! How can we get it rolling?

[szuecs]

@aryszka what do you think about jq-style support or not?

@universam1 I think in https://github.com/zalando/skipper/issues/1282#issuecomment-578046103 , I had some hints about how the implementation can be done. stateBag is a map[string]interface{}, that is created per request, such that you can add already parsed jwtClaims in openIDC* filters and check it in a new filter created in filters/auth/jwt_claims.go for example.
Maybe https://opensource.zalando.com/skipper/tutorials/development/#filters also helps.
What is the semantics of *?

[universam1]

@szuecs Thanks for the background, I'm new to skipper so that helps.

Did some research about JSON-query languages (besides of jq) and - if the discussion is still hot for such an option - maybe a better idea would be to use a proper language for. Found this one being lean and the query language probably fitting our use case, the benchmarks are interesting too:
https://github.com/tidwall/gjson#check-for-the-existence-of-a-value
https://github.com/tidwall/gjson/blob/master/SYNTAX.md

[aryszka]

Hi, somewhat late to the party, but here's my take on the topic:

My concern would be not so much the additional dependency (these are quite modest libraries), but with having one syntax in the other, where one needs to be aware of the escaping. But TBH, this below format is quite sloppy:

claimsAny("groups", "CD-xyz", "groups", "CD-abc")

And since we already have Lua filters, I'd actually say let's use some syntax! :)

From the suggestions, I like gjson the most, because it looks relatively simple.

[universam1]

@szuecs @aryszka I've uploaded a playground what I have put together so far in https://github.com/zalando/skipper/pull/1296 - probably that allows easier discussion of the approach, happy for your initial comments

[universam1]

I believe the implementation #1296 is ready for review @szuecs - please let me know if that is acceptable

[szuecs]

@universam1 thanks, looks great to me :)

[szuecs]

was fixed in https://github.com/zalando/skipper/pull/1296 and released