smolnar82 opened a new pull request, #1467: URL: https://github.com/apache/knox/pull/1467
[KNOX-3529](https://issues.apache.org/jira/browse/KNOX-3529) - Count the per-user token limit per token type ## What changes were proposed in this pull request? The token state is shared by the whole gateway, so `knox.token.limit.per.user` used to count **all** server-managed tokens of a user (except KnoxSSO cookies), no matter which API issued them. As a result, a user's client credentials and API keys used up their JWT budget, and a `CLIENTID` limit could reject new credentials because the user had JWTs from an unrelated topology. This PR makes each API count only its own token types: - `KNOXTOKEN` (including the OAuth token endpoint and KnoxIDF): JWTs, KnoxIDF authorization codes and refresh tokens - `CLIENTID` (and KnoxIDF client registration): client credentials - `APIKEY`: API keys KnoxSSO cookies still never count. With `REMOVE_OLDEST`, the oldest token **of the counted types** is removed. The new service parameter `knox.token.limit.per.user.by.type.enabled` controls this. It defaults to `true`; setting it to `false` restores the previous behavior of counting all token types together. `CLIENTID` and `APIKEY` take the usual prefixes (`clientid.` / `apikey.`). Code changes: - `TokenResource`: filters the user's tokens through `getTokenTypesCountedTowardsLimit()`; `ClientCredentialsResource` and `APIKeyResource` override it. The chosen mode is logged at startup. - `TokenMetadata.getTokenType()`: new helper that returns the `TokenMetadataType`. Tokens with no type or an unknown type are treated as JWTs. - Docs: `config_knox_token.md`, `config_client_credentials.md`, `config_apikey.md` and `knoxidf/configuration.md`. **Behavior change on upgrade:** limits get looser, not stricter. For example, a user can now have up to `knox.token.limit.per.user` JWTs **plus** that many client credentials, where before the two together were capped at that number. The docs include an upgrade note: deployments that relied on one combined cap should lower the per-API limits or set `knox.token.limit.per.user.by.type.enabled=false`. ## How was this patch tested? - New unit tests in `TokenServiceResourceTest`: - JWT limits ignore client credentials and API keys, both by default and when explicitly enabled. - Authorization codes and refresh tokens count towards the JWT limit. - With `false`, all types count (the old behavior). - `REMOVE_OLDEST` revokes the oldest token of the same type. - The client credentials limit ignores JWTs by default and when enabled, and counts them with `false`. - New `TokenMetadataTest.testGetTokenType` covering missing, known and unknown types. - `mvn verify` passes for `gateway-spi`, `gateway-service-knoxtoken` and `gateway-service-knoxidf`. - The full Docker Compose integration suite passes locally (142 tests, pylint 10.00/10). ## Integration Tests New `test_token_limit_by_type.py` with two new topologies: - `tokenlimitbytype.xml`: `CLIENTID`, `APIKEY` and `KNOXTOKEN`, each limited to 2 per user, using the default (per-type) counting. - `tokenlimit-combined.xml`: `KNOXTOKEN` limited to 2 with `knox.token.limit.per.user.by.type.enabled=false`. The token store is shared by all test files, so each test has `admin` mint tokens on behalf of (`doAs`) a fresh random user and starts from zero tokens. Tests: - `test_jwt_is_issued_for_the_impersonated_user`: sanity check that the token's `sub` is the `doAs` user - `test_jwts_ignore_client_credentials_and_api_keys` - `test_client_credentials_and_api_keys_ignore_jwts` - `test_combined_counting_includes_client_credentials`: with `false`, client credentials use up the JWT limit; the per-type topology still issues JWTs ``` tests-1 | ------------------------------------ tests-1 | Your code has been rated at 10.00/10 tests-1 | tests-1 | Waiting for knox... tests-1 | ============================= test session starts ============================== tests-1 | platform linux -- Python 3.10.20, pytest-9.0.3, pluggy-1.7.0 tests-1 | rootdir: /tests tests-1 | plugins: platformdirs-4.12.4 tests-1 | collected 142 items tests-1 | tests-1 | test_clientid_credentials.py ....... [ 4%] tests-1 | test_delegation.py ......... [ 11%] tests-1 | test_health.py ..... [ 14%] tests-1 | test_k8s_delegation.py ........ [ 20%] tests-1 | test_k8s_jwks_ca.py ........... [ 28%] tests-1 | test_k8s_serviceaccount_validation.py ...... [ 32%] tests-1 | test_knox_admin_path_traversal.py ... [ 34%] tests-1 | test_knox_auth_service_and_ldap.py ... [ 36%] tests-1 | test_knox_configs.py . [ 37%] tests-1 | test_knox_ldap_backend_password_alias_retry.py .. [ 38%] tests-1 | test_knox_ldap_cache.py ... [ 40%] tests-1 | test_knox_ldap_injection.py ....... [ 45%] tests-1 | test_knox_ldap_proxy_search.py ......... [ 52%] tests-1 | test_knoxauth_preauth_and_paths.py ...... [ 56%] tests-1 | test_knoxauth_token_forwarding.py ........... [ 64%] tests-1 | test_knoxidf.py ....... [ 69%] tests-1 | test_knoxsso_redirect.py . [ 69%] tests-1 | test_knoxtoken_jwt.py .................... [ 83%] tests-1 | test_remote_auth.py ... [ 85%] tests-1 | test_remoteauth_extauthz_additional_path.py .... [ 88%] tests-1 | test_token_exchange.py ............ [ 97%] tests-1 | test_token_limit_by_type.py .... [100%] tests-1 | tests-1 | =============================== warnings summary =============================== ... tests-1 | ----------------- generated xml file: /tests/test-results.xml ------------------ tests-1 | ================= 142 passed, 132 warnings in 73.84s (0:01:13) ================= tests-1 exited with code 0 Aborting on container exit... Container compose-tests-1 Stopping ``` -- 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]
