nic-6443 commented on code in PR #13509:
URL: https://github.com/apache/apisix/pull/13509#discussion_r3393130882
##########
apisix/plugins/authz-keycloak.lua:
##########
@@ -30,8 +30,8 @@ local schema = {
discovery = {type = "string", minLength = 1, maxLength = 4096},
token_endpoint = {type = "string", minLength = 1, maxLength = 4096},
resource_registration_endpoint = {type = "string", minLength = 1,
maxLength = 4096},
- client_id = {type = "string", minLength = 1, maxLength = 100},
- client_secret = {type = "string", minLength = 1, maxLength = 100},
+ client_id = {type = "string", minLength = 1, maxLength = 4096},
+ client_secret = {type = "string", minLength = 1, maxLength = 4096},
Review Comment:
The rationale for this bump does not hold on master. Since #13312,
`core.schema.check` skips validation for `$secret://` / `$env://` references on
string fields (`skip_validation = secret.is_secret_ref` in
apisix/core/schema.lua), so a long reference string already passes the 100-char
limit. And on reload, `plugin_checker` runs `decrypt_conf` before the schema
check, so validation sees the original value, not the AES output — what #13493
describes is real on 3.16.0 but is already addressed here.
Raising the limit might still be worth doing for genuinely long client
secrets, but then it should be justified on those grounds; otherwise I would
drop this hunk from the PR and keep it focused on the new secret manager.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
