spacewander commented on a change in pull request #3308:
URL: https://github.com/apache/apisix/pull/3308#discussion_r561460712
##########
File path: doc/plugins/authz-keycloak.md
##########
@@ -38,24 +38,37 @@ For more information on Keycloak, refer to [Keycloak
Authorization Docs](https:/
## Attributes
-| Name | Type | Requirement | Default
| Valid
| Description
|
-| ----------------------- | ------------- | ----------- |
--------------------------------------------- |
------------------------------------------------------------------ |
-----------------------------------------------------------------------------------------------------------------------------------------------------------
|
-| discovery | string | optional |
|
https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to
discovery document for Keycloak Authorization Services.
|
-| token_endpoint | string | optional |
|
https://host.domain/auth/realms/foo/protocol/openid-connect/token | A
OAuth2-compliant Token Endpoint that supports the
`urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from
discovery, if given. |
-| grant_type | string | optional |
"urn:ietf:params:oauth:grant-type:uma-ticket" |
["urn:ietf:params:oauth:grant-type:uma-ticket"] |
|
-| audience | string | optional |
|
| The client identifier of the resource server to which the
client is seeking access. <br>This parameter is mandatory when parameter
permission is defined. |
-| permissions | array[string] | optional |
|
| A string representing a set of one or more resources and scopes
the client is seeking access. The format of the string must be:
`RESOURCE_ID#SCOPE_ID`. |
-| timeout | integer | optional | 3000
| [1000, ...]
| Timeout(ms) for the http connection with the Identity Server.
|
-| ssl_verify | boolean | optional | true
|
| Verify if SSL cert matches hostname.
|
-| policy_enforcement_mode | string | optional | "ENFORCING"
| ["ENFORCING", "PERMISSIVE"]
|
|
-
-### Endpoints
-
-Endpoints can optionally be discovered by providing a URL pointing to
Keycloak's discovery document for Authorization Services for the realm
-in the `discovery` attribute. The token endpoint URL will then be determined
from that document. Alternatively, the token endpoint can be
-specified explicitly via the `token_endpoint` attribute.
-
-One of `discovery` and `token_endpoint` has to be set. If both are given, the
value from `token_endpoint` is used.
+| Name | Type | Requirement | Default
| Valid
| Description
|
+| ------------------------------ | ------------- | ----------- |
--------------------------------------------- |
------------------------------------------------------------------ |
-----------------------------------------------------------------------------------------------------------------------------------------------------------
|
+| discovery | string | optional |
|
https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to
discovery document for Keycloak Authorization Services.
|
+| token_endpoint | string | optional |
|
https://host.domain/auth/realms/foo/protocol/openid-connect/token | A
OAuth2-compliant Token Endpoint that supports the
`urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from
discovery, if given. |
+| resource_registration_endpoint | string | optional |
|
https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak
Protection API-compliant resource registration endpoint. Overrides value from
discovery, if given. |
+| grant_type | string | optional |
"urn:ietf:params:oauth:grant-type:uma-ticket" |
["urn:ietf:params:oauth:grant-type:uma-ticket"] |
|
+| client_id | string | required |
|
| The client identifier of the resource server to which
the client is seeking access. <br>This parameter is mandatory when parameter
permission is defined. |
+| client_secret | string | optional |
|
| The client secret, if required.
|
+| policy_enforcement_mode | string | optional | "ENFORCING"
| ["ENFORCING", "PERMISSIVE"]
|
|
+| permissions | array[string] | optional |
|
| Static permission to request, an array of strings each
representing a resources and optionally one or more scopes the client is
seeking access. |
+| lazy_load_paths | boolean | optional | false
|
| Dynamically resolve the request URI to resource(s) using
the resource registration endpoint instead of using the static permission.
|
+| http_method_as_scope | boolean | optional | false
|
| Map HTTP request type to scope of same name and add to
all permissions requested.
|
Review comment:
Missing doc for `cache_ttl_seconds`?
##########
File path: doc/plugins/authz-keycloak.md
##########
@@ -38,24 +38,37 @@ For more information on Keycloak, refer to [Keycloak
Authorization Docs](https:/
## Attributes
-| Name | Type | Requirement | Default
| Valid
| Description
|
-| ----------------------- | ------------- | ----------- |
--------------------------------------------- |
------------------------------------------------------------------ |
-----------------------------------------------------------------------------------------------------------------------------------------------------------
|
-| discovery | string | optional |
|
https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to
discovery document for Keycloak Authorization Services.
|
-| token_endpoint | string | optional |
|
https://host.domain/auth/realms/foo/protocol/openid-connect/token | A
OAuth2-compliant Token Endpoint that supports the
`urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from
discovery, if given. |
-| grant_type | string | optional |
"urn:ietf:params:oauth:grant-type:uma-ticket" |
["urn:ietf:params:oauth:grant-type:uma-ticket"] |
|
-| audience | string | optional |
|
| The client identifier of the resource server to which the
client is seeking access. <br>This parameter is mandatory when parameter
permission is defined. |
-| permissions | array[string] | optional |
|
| A string representing a set of one or more resources and scopes
the client is seeking access. The format of the string must be:
`RESOURCE_ID#SCOPE_ID`. |
-| timeout | integer | optional | 3000
| [1000, ...]
| Timeout(ms) for the http connection with the Identity Server.
|
-| ssl_verify | boolean | optional | true
|
| Verify if SSL cert matches hostname.
|
-| policy_enforcement_mode | string | optional | "ENFORCING"
| ["ENFORCING", "PERMISSIVE"]
|
|
-
-### Endpoints
-
-Endpoints can optionally be discovered by providing a URL pointing to
Keycloak's discovery document for Authorization Services for the realm
-in the `discovery` attribute. The token endpoint URL will then be determined
from that document. Alternatively, the token endpoint can be
-specified explicitly via the `token_endpoint` attribute.
-
-One of `discovery` and `token_endpoint` has to be set. If both are given, the
value from `token_endpoint` is used.
+| Name | Type | Requirement | Default
| Valid
| Description
|
+| ------------------------------ | ------------- | ----------- |
--------------------------------------------- |
------------------------------------------------------------------ |
-----------------------------------------------------------------------------------------------------------------------------------------------------------
|
+| discovery | string | optional |
|
https://host.domain/auth/realms/foo/.well-known/uma2-configuration | URL to
discovery document for Keycloak Authorization Services.
|
+| token_endpoint | string | optional |
|
https://host.domain/auth/realms/foo/protocol/openid-connect/token | A
OAuth2-compliant Token Endpoint that supports the
`urn:ietf:params:oauth:grant-type:uma-ticket` grant type. Overrides value from
discovery, if given. |
+| resource_registration_endpoint | string | optional |
|
https://host.domain/auth/realms/foo/authz/protection/resource_set | A Keycloak
Protection API-compliant resource registration endpoint. Overrides value from
discovery, if given. |
+| grant_type | string | optional |
"urn:ietf:params:oauth:grant-type:uma-ticket" |
["urn:ietf:params:oauth:grant-type:uma-ticket"] |
|
+| client_id | string | required |
|
| The client identifier of the resource server to which
the client is seeking access. <br>This parameter is mandatory when parameter
permission is defined. |
Review comment:
> This parameter is mandatory when parameter permission is defined.
Look like it should be `when parameter lazy_load_paths is defined`? Should
require it in the schema and add a test for it.
##########
File path: t/plugin/authz-keycloak.t
##########
@@ -137,7 +224,7 @@ done
"authz-keycloak": {
"token_endpoint":
"http://127.0.0.1:8090/auth/realms/University/protocol/openid-connect/token";,
"permissions": ["course_resource#view"],
- "audience": "course_management",
Review comment:
Maybe we can leave `audience` unchanged in one or two cases so that we
know the old configuration is still compatible?
##########
File path: apisix/plugins/authz-keycloak.lua
##########
@@ -224,31 +275,326 @@ local function authz_keycloak_get_token_endpoint(conf)
end
-local function is_path_protected(conf)
- -- TODO if permissions are empty lazy load paths from Keycloak
- if conf.permissions == nil then
- return false
+local function authz_keycloak_get_resource_registration_endpoint(conf)
+ return authz_keycloak_get_endpoint(conf, "resource_registration_endpoint")
+end
+
+
+-- computes access_token expires_in value (in seconds)
+local function authz_keycloak_access_token_expires_in(opts, expires_in)
+ return (expires_in or opts.access_token_expires_in or 300)
+ - 1 - (opts.access_token_expires_leeway or 0)
+end
+
+
+-- computes refresh_token expires_in value (in seconds)
+local function authz_keycloak_refresh_token_expires_in(opts, expires_in)
+ return (expires_in or opts.refresh_token_expires_in or 3600)
+ - 1 - (opts.refresh_token_expires_leeway or 0)
+end
+
+
+local function authz_keycloak_ensure_sa_access_token(conf)
+ local client_id = authz_keycloak_get_client_id(conf)
+ local ttl = conf.cache_ttl_seconds
+ local token_endpoint = authz_keycloak_get_token_endpoint(conf)
+
+ if not token_endpoint then
+ log.error("Unable to determine token endpoint.")
+ return 500, "Unable to determine token endpoint."
+ end
+
+ local session = authz_keycloak_cache_get("access_tokens", token_endpoint
.. ":"
+ .. client_id)
Review comment:
The `client_id` could be nil if you don't force when lazy_load_paths is
used.
##########
File path: apisix/plugins/authz-keycloak.lua
##########
@@ -171,13 +222,13 @@ local function authz_keycloak_discover(url, ssl_verify,
keepalive, timeout,
keepalive = (keepalive ~= "no")
Review comment:
It seems a bug in the previous code: the `keepalive` and `ssl_verify`
are boolean.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
