smolnar82 opened a new pull request, #1422: URL: https://github.com/apache/knox/pull/1422
[KNOX-3480](https://issues.apache.org/jira/browse/KNOX-3480) - Support user-supplied clientId in the Client Credentials endpoint ## What changes were proposed in this pull request? The `clientid/api/v1/oauth/credentials` endpoint (`ClientCredentialsResource`) previously always returned a server-generated UUID as the `client_id`, which is persisted as the `KNOX_TOKENS.token_id` primary key. Well-known IdPs (Auth0, Okta, Keycloak) instead let the caller choose the client identifier at registration time. This PR adds the same capability: - **New topology service param `clientid.allowUserSuppliedClientId` (default `false`).** When enabled, a caller-supplied `clientId` query param becomes the token's `knox.id` claim / `token_id`. When disabled or the param is omitted, behavior is unchanged (a UUID is generated). - **JWT minting:** threaded an optional `tokenId` through `JWTokenAttributes` / `JWTokenAttributesBuilder`; `JWTToken` uses it as the `knox.id` claim when present, otherwise a random UUID (`TokenResource.getRequestedTokenId()` hook, defaulting to null). - **Validation:** a supplied `clientId` must match `^[A-Za-z0-9._-]{1,128}$` (matching the `token_id` column width); otherwise `400 Bad Request` (`invalid_request`). - **Uniqueness:** enforced atomically by the `token_id` primary key (no check-then-insert race across an HA pair). A duplicate surfaces as the new unchecked `TokenAlreadyExistsException` (translated from a DB unique-constraint violation in `JDBCTokenStateService`, best-effort on the in-memory `DefaultTokenStateService`), which the resource maps to `409 Conflict` (`invalid_client`) rather than a generic 500. - Documented the new param and behavior in `knox-site/docs/config_client_credentials.md`. ## How was this patch tested? - **Unit tests** (all pass): - `TokenServiceResourceTest` — enabled+valid → `client_id` equals supplied value; disabled or omitted → UUID; invalid (bad chars / >128) → 400; duplicate → 409. - `JDBCTokenStateServiceTest#testAddDuplicateTokenIdThrowsTokenAlreadyExists` — H2-backed duplicate `token_id` insert surfaces as `TokenAlreadyExistsException`, one row survives. - `JWTTokenTest` — supplied token id becomes the `knox.id` claim verbatim; blank falls back to UUID. - Run: `mvn -pl gateway-spi,gateway-service-knoxtoken,gateway-server -am test` ## Integration Tests Added `.github/workflows/tests/test_clientid_credentials.py` plus two bind-mounted topologies (`clientid.xml` with the feature ON, `clientid-default.xml` with it OFF). The CI gateway runs on H2 (persistent, PK-enforced), so the duplicate → 409 path is genuinely exercised. Cases: - supplied id used - E2E passcode authenticates (custom id becomes the audit actor username) omitted → UUID - invalid → 400 - over-long → 400 - duplicate → 409, and supplied-but-ignored on the default (feature-off) topology. **All 109 CI integration tests pass.**: ``` 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.6.0 tests-1 | rootdir: /tests tests-1 | collected 109 items tests-1 | tests-1 | test_clientid_credentials.py ....... [ 6%] tests-1 | test_delegation.py ......... [ 14%] tests-1 | test_health.py ..... [ 19%] tests-1 | test_k8s_delegation.py ... [ 22%] tests-1 | test_k8s_serviceaccount_validation.py ...... [ 27%] tests-1 | test_knox_admin_path_traversal.py ... [ 30%] tests-1 | test_knox_auth_service_and_ldap.py ... [ 33%] tests-1 | test_knox_configs.py . [ 33%] tests-1 | test_knox_ldap_cache.py ... [ 36%] tests-1 | test_knox_ldap_injection.py ....... [ 43%] tests-1 | test_knox_ldap_proxy_search.py ......... [ 51%] tests-1 | test_knoxauth_preauth_and_paths.py ...... [ 56%] tests-1 | test_knoxidf.py ....... [ 63%] tests-1 | test_knoxsso_redirect.py . [ 64%] tests-1 | test_knoxtoken_jwt.py .................... [ 82%] tests-1 | test_remote_auth.py ... [ 85%] tests-1 | test_remoteauth_extauthz_additional_path.py .... [ 88%] tests-1 | test_token_exchange.py ............ [100%] tests-1 | tests-1 | =============================== warnings summary =============================== tests-1 | ../usr/local/lib/python3.10/site-packages/ldap3/utils/asn1.py:50 ... tests-1 | -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html tests-1 | ----------------- generated xml file: /tests/test-results.xml ------------------ tests-1 | ====================== 109 passed, 101 warnings in 25.22s ====================== ``` ## UI changes N/A -- 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]
