Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: auth-keycloak plugin - Dynamically resolve resources and scopes from incoming request. #3274

Closed
jenskeiner opened this issue Jan 13, 2021 · 4 comments · Fixed by #3308
Labels
enhancement New feature or request

Comments

@jenskeiner
Copy link
Contributor

I'm opening this issue to get some feedback on some proposed changes to the auth-keycloak plugin before I shoot off and start working on them.

The current incarnation of the plugin has some limitations that limit its usefulness. The most visible limitation, to me at least, is that you can only configure a static set of resource/scope pairs.

Without delving into too many details of the Keycloak Authorization Services API (described here in all its glory: https://www.keycloak.org/docs/latest/authorization_services/index.html), it's worth mentioning that permissions are defined in terms of resource/scope pairs. If you think of some REST service that you want to protect, then a resource would typically be a subset of the available endpoints, defined by some URI pattern, e.g. /foo/bar/*. A scope typically describes the type of action to perform on the resource, e.g. read, write, delete, but can also really be something completely different and abstract, depending on the use case.

To check if an incoming request should be allowed to proceed to the upstream, or if it should be rejected, the plugin currently uses the token endpoint (which is also used for OIDC authentication) with a special grant type. The requested permissions, specified by resource/scope pairs are sent as a parameter to that endpoint together with the bearer token from the request. Keycloak evaluates the permissions and answers with the decision, i.e. whether the permissions are granted or not. If yes, then the request can proceed, otherwise, it is rejected.

As I said above, the set of resource/scope pairs is currently fixed in the plugin configuration. But often, what you want to do is to determine the resource and scope from features of the incoming request like HTTP method and URI. Actually, the documentation mentions this as future work, but that has so far not materialized.

Just upfront: The changes I want to propose would need to be split into smaller PRs that build onto each other.

Basically, I want to propose to implement (optional) dynamic resource and scope resolution as follows:

  • The scope is determined by the HTTP method of the request (i.e. a GET request maps to scope GET).
  • The resource is determined by using Keycloak's resource registration endpoint which can return the resources that match a given URI.

I have a working example of this in a branch. There are some points to observe though:

  • Using the resource registration endpoint requires another request to Keycloak on top of the one to the token endpoint that returns the decision. Longer term, but maybe not in the first iteration, caching can be implemented to reduce the number of necessary requests.
  • To be able to invoke the resource registration endpoint, the plugin first needs to obtain a separate access token for itself from the token endpoint via client ID and secret. This is a special token that has the uma_protection scope which allows access to the Protection API and hence the resource registration endpoint. It's relative easy to obtain the token, but some management needs to be implemented because the token needs to be refreshed regularly as it will typically be short-lived. This will make the code base a bit more complex, but cannot be avoided.
  • Longer term, it may be better to load all resources from Keycloak once (maybe with periodic updates), and match the request URI to the resources in the plugin, e.g. with the help of the radix tree library already used for routes. This would avoid the requests that are needed just to determine the resource.

To sum up, I would propose above changes, but with the focus on first getting the scope and resource discovery working properly, before focusing on further optimizations like caching.

Would love to hear anyone's thoughts. Maybe @sshniro can chime in because you've worked a lot with the plugin already.

@sshniro
Copy link
Member

sshniro commented Jan 13, 2021

Hi @jenskeiner I agree with your points, and we can get some design decisions from the official adapter from Keycloak.

The scope is determined by the HTTP method of the request (i.e. a GET request maps to scope GET).

I would suggest using the http-method-as-scope configuration option to enforce this behavior, as some may enforce policies via custom scope names. The adapters also provide this option.

Longer term, it may be better to load all resources from Keycloak once (maybe with periodic updates), and match the request URI to the resources in the plugin, e.g. with the help of the radix tree library already used for routes. This would avoid the requests that are needed just to determine the resource.

Agreed, the lazy-load-paths configuration option can be used to develop this enhancement so it tallies with the official adapter configurations.

The resource is determined by using Keycloak's resource registration endpoint which can return the resources that match a given URI.

I may be mistaken, does Keycloak matches the URI and returns results, or do we have to obtain all the paths and resolve it in APISIX?

Using the resource registration endpoint requires another request to Keycloak on top of the one to the token endpoint that returns the decision. Longer term, but maybe not in the first iteration, caching can be implemented to reduce the number of necessary requests.

Agreed, we need a separate token for APISIX to orchestrate this flow. +1 for caching instead of straining the Keycloak for token per request.

+1 for the proposed changes, and looking forward to your PR.

@sshniro sshniro added the enhancement New feature or request label Jan 13, 2021
@jenskeiner
Copy link
Contributor Author

jenskeiner commented Jan 13, 2021

Thanks for the feedback @sshniro.

The resource is determined by using Keycloak's resource registration endpoint which can return the resources that match a given URI.

I may be mistaken, does Keycloak matches the URI and returns results, or do we have to obtain all the paths and resolve it in APISIX?

The resource registration endpoint can be used to offload the path matching to Keycloak. You need to invoke it with parameters uri set to the request URI and matchingUri set to true. Got that from the official adapter's code. I tested it that way and it works. The result just contains a JSON-style array with the IDs of matching resources. It seems to prefer stricter matches, not sure if anything can be further configured within Keycloak regarding the matching algo. Also, the endpoint has more options and I didn't test every possible combination.

Importantly, one can then use the returned IDs when querying the token endpoint for the permission decision. All that works in a little POC of mine.

I think, as a first step, it's best to make that extra call to Keycloak and swallow the added cost, but avoid the paths management inside the plugin. In a following change, I would suggest look at the matching on the plugin side as an optimization.

@sshniro
Copy link
Member

sshniro commented Jan 14, 2021

Thanks for the detailed information, and agreed, path matching inside the plugin will need additional testing and makes sense to implement in another phase. Also, I assume for the test cases you have to update the Keycloak permissions with paths? If so I can add you as a collaborator to the docker registry. I tried to move the existing image to a common one, but still no luck.

@jenskeiner
Copy link
Contributor Author

Thanks for the detailed information, and agreed, path matching inside the plugin will need additional testing and makes sense to implement in another phase. Also, I assume for the test cases you have to update the Keycloak permissions with paths? If so I can add you as a collaborator to the docker registry. I tried to move the existing image to a common one, but still no luck.

@sshniro If you can, just add my user on DockerHub (jenskeiner) to your repo and I can push an updated version of the image. In the meantime, I've pushed it into my own repo already, so I can test. It really only needed some minor adjustments like adding a path pattern to the course_resource resource, adding GET and DELETE scopes, adding the student to the Student role (looks like an oversight in the original version), and adjusting the permissions.

@spacewander spacewander added bug Something isn't working and removed bug Something isn't working labels Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants