Skip to content

feat(authz-keycloak): Add Keycloak Authorization Services endpoint discovery.#3263

Merged
spacewander merged 23 commits intoapache:masterfrom
jenskeiner:uma-discovery
Jan 13, 2021
Merged

feat(authz-keycloak): Add Keycloak Authorization Services endpoint discovery.#3263
spacewander merged 23 commits intoapache:masterfrom
jenskeiner:uma-discovery

Conversation

@jenskeiner
Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

The authz-keycloak plugin needs to know the Keycloak token endpoint to use. A configuration option to point to the token endpoint is already there.

Additionally, Keycloak supports discovery of Authorization Services-related endpoints, like the token endpoint, through a discovery document served under a well-known path relative to the realm, e.g. https://keycloak-host/auth/realms/foo/.well-known/uma2-configuration.

It can be convenient to use the discovery document to get the token endpoint URL, instead of specifying the token endpoint explicitly, since the URL is typically shorter. More importantly, future build out of the authz-keycloak may require access to additional related endpoints. In this case, just providing the single path to the discovery document would be more convenient than specifying each endpoint URL explicitly.

This PR adds an attribute that allows to specify the discovery document URL and makes the token endpoint attribute optional. However, it is checked that at least one, discovery or token endpoint, is given.

The plugin now fetches the discovery document lazily and caches it in shared storage already used by the openid-connect plugin (better: the openidc module used therein) for similar purposes in the context of OIDC endpoint discovery.

If both, discovery and token endpoint are given, the plugin prefers the explicit token endpoint value over the one from discovery.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@jenskeiner
Copy link
Copy Markdown
Contributor Author

jenskeiner commented Jan 12, 2021

@sshniro: Penny for your thoughts. I have a POC in a separate branch that dynamically resolves URIs to resources defined in Keycloak, so this is just a first change in that direction.

@jenskeiner jenskeiner changed the title Uma discovery feat: authz-keycloak plugin - Keycloak Authorizationn Services endpoint discovery Jan 12, 2021
@jenskeiner jenskeiner changed the title feat: authz-keycloak plugin - Keycloak Authorizationn Services endpoint discovery feat: authz-keycloak plugin - Keycloak Authorization Services endpoint discovery Jan 12, 2021
Comment on lines +22 to +23
local cjson = require("cjson")
local cjson_s = require("cjson.safe")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to use the public methods provided here.https://github.com/apache/apisix/blob/master/apisix/core/json.lua

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, using that one now. We're incurring a slight performance penalty by now using cjson.safe.decode through apisix.core.json.decode to decode values from the shared memory which are known to have been encoded correctly. But not sure how much of a hit that actually is. Just this line here: https://github.com/jenskeiner/apisix/blob/26277f1ea98366ac852ee6673667e1be1320001c/apisix/plugins/authz-keycloak.lua#L190

@jenskeiner jenskeiner changed the title feat: authz-keycloak plugin - Keycloak Authorization Services endpoint discovery feat: authz-keycloak plugin - Add Keycloak Authorization Services endpoint discovery. Jan 13, 2021
@spacewander spacewander changed the title feat: authz-keycloak plugin - Add Keycloak Authorization Services endpoint discovery. feat(authz-keycloak): Add Keycloak Authorization Services endpoint discovery. Jan 13, 2021
@spacewander spacewander merged commit 9f6e605 into apache:master Jan 13, 2021
sysulq pushed a commit to sysulq/apisix that referenced this pull request Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants