KNOX-3395: Add KNOXTOKEN issuance and JWTProvider federation integration tests - #1325
KNOX-3395: Add KNOXTOKEN issuance and JWTProvider federation integration tests#1325Raghav-Mah3shwari wants to merge 3 commits into
Conversation
|
Hi @pzampino , @smolnar82, can i get a review on this? thanks |
Test Results53 tests 53 ✅ 9s ⏱️ Results for commit c4a0458. ♻️ This comment has been updated with latest results. |
|
Nice addition, @Raghav-Mah3shwari, the end-to-end path here (mint a token in knoxldap → present it to the JWTProvider-federated knoxtoken topology → assert x-knox-actor-username) is the highest-value scenario and it's covered cleanly. The file follows the suite conventions (common_utils, unittest, HTTPBasicAuth) and the issuance + negative auth cases all look correct. A couple of things before merge: 1. Missing token lifecyle operation test cases These endpoints require server-managed token state, which neither
To exercise them you'd need, on the Without the whitelist, token renewal or revocation will return 403 (revoking your own token is the one exception). Cases I'd suggest for that round:
2. Two minor notes on the existing tests:
|
|
Hi @smolnar82 sure i will add them as well, thank you |
|
Hi @smolnar82 , can you review the pr again?, thanks |
| # 3) Provision the gateway-level JWK required for server-managed Knox token state | ||
| # (renew / revoke / enable / disable and JWTProvider enforcement). | ||
| /knox-runtime/bin/knoxcli.sh generate-jwk --jwkAlg HS256 --saveAlias knox.token.hash.key |
There was a problem hiding this comment.
The comment change here is misleading. You removed the original
Trust that certificate in the JVM default truststore (cacerts) so the JNDI-based
Shiro LDAP realm accepts it. This is additive - it does not remove the default CAs.
before the keytool command.
I thinks it should look like this:
# 3) Provision the gateway-level JWK required for server-managed Knox token state (renew / revoke / enable / disable and JWTProvider enforcement).
/knox-runtime/bin/knoxcli.sh generate-jwk --jwkAlg HS256 --saveAlias knox.token.hash.key
# 4) Trust that certificate in the JVM default truststore (cacerts) so the JNDI-based
Shiro LDAP realm accepts it. This is additive - it does not remove the default CAs.
keytool -exportcert -alias ldaps -rfc
-keystore "$KEYSTORE" -storepass "$KEYSTORE_PASSWORD" -file /tmp/ldaps-cert.pem
| expected_username, | ||
| ) | ||
|
|
||
| def assert_federation_rejected(self, access_token): |
There was a problem hiding this comment.
Every other helper in the class uses a _ prefix (_issue_token, _federate, _assert_federates). assert_federation_rejected is the only exception; it'll also show up in test discovery frameworks as a test method (they scan for names starting with assert). Please rename to _assert_federation_rejected.
|
Hi @smolnar82 done, can you check it? thanks |
What changes were proposed in this pull request?
This PR adds Docker Compose integration coverage for the knoxtoken topology (JWTProvider federation) and the KNOXTOKEN service. Both were baked into the CI test image but previously had no integration tests.
End-to-end flow:
Mint a Knox JWT from the KNOXTOKEN service (gateway/knoxldap/knoxtoken/api/v1/token) using Basic auth.
Present that bearer token to the JWTProvider-protected topology (gateway/knoxtoken/auth/api/v1/pre) and assert caller identity.
Exercise lifecycle operations (renew / revoke / enable / disable) and verify they are enforced at federation, not only acknowledged by the management API.
Issuance / federation:
test_token_endpoint_returns_jwt_and_metadata — access_token, token_type: Bearer, expires_in, well-formed 3-segment JWT
test_token_requires_authentication — anonymous request → 401
test_jwt_grants_access_to_federated_topology — valid JWT → x-knox-actor-username: guest
test_federated_topology_requires_token — no token → 401
test_federated_topology_rejects_malformed_token — structurally invalid bearer token → 401
test_federated_topology_rejects_wrong_signature — parseable JWT with a tampered signature segment → 401 (RS256 verification)
Lifecycle (server-managed token state):
test_revoke_is_enforced_at_federation — mint → federate OK → revoke → federate 401
test_renew_extends_and_token_still_federates — renewed: true and token still federates
test_renew_forbidden_for_non_whitelisted_user — non-whitelisted admin renew → 403
test_revoke_forbidden_for_non_owner_non_whitelisted_user — admin revoke of guest token → 403
test_disable_is_enforced_at_federation — disable → 401; enable → federation restored
test_enable_already_enabled_returns_400 / test_disable_already_disabled_returns_400
Topology / harness prerequisites
Lifecycle APIs require server-managed token state. This PR also:
How was this patch tested?
Verified API contracts against the codebase (TokenResource / TokenResourceV2, PreAuthResource, JWTProvider signature verification).
Ran the Docker Compose integration suite locally:
docker compose -f ./.github/workflows/compose/docker-compose.yml build knox-dev
docker compose -f ./.github/workflows/compose/docker-compose.yml up -d --force-recreate knox
docker compose -f ./.github/workflows/compose/docker-compose.yml run --rm tests
Confirmed the new file is auto-discovered by the default pytest run (not in the single-EKU --ignore list).
Integration Tests
This PR adds/extends integration tests under .github/workflows/tests. No new Java unit tests are included; KNOXTOKEN and JWTProvider already have module-level Java coverage. The black-box suite covers the packaged-gateway end-to-end path that was previously missing.
UI changes
N/A — no UI changes.