Skip to content

KNOX-3395: Add KNOXTOKEN issuance and JWTProvider federation integration tests - #1325

Open
Raghav-Mah3shwari wants to merge 22 commits into
apache:masterfrom
Raghav-Mah3shwari:KNOX-3395
Open

KNOX-3395: Add KNOXTOKEN issuance and JWTProvider federation integration tests#1325
Raghav-Mah3shwari wants to merge 22 commits into
apache:masterfrom
Raghav-Mah3shwari:KNOX-3395

Conversation

@Raghav-Mah3shwari

@Raghav-Mah3shwari Raghav-Mah3shwari commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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.

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Hi @pzampino , @smolnar82, can i get a review on this? thanks

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Test Results

53 tests   53 ✅  9s ⏱️
 3 suites   0 💤
 3 files     0 ❌

Results for commit c4a0458.

♻️ This comment has been updated with latest results.

@smolnar82

Copy link
Copy Markdown
Contributor

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
Would you be up for extending the scope to cover the token lifecycle operations (renew / revoke / enable / disable) in a follow-up (or here, if you prefer)? Since KNOX-3395 indicates KNOXTOKEN issuance, it's a natural next step and there's a config prerequisite worth capturing while it's fresh:

These endpoints require server-managed token state, which neither knoxtoken.xml nor knoxldap.xml currently enables (both set only token TTL). So it's a topology-config change first, tests second. Against the current config:

  • enable / disable / revoke → tokenStateService == null → 400 CONFIGURATION_ERROR
  • renew → falls into the renewalDisabled branch and echoes the token's own expiry — no actual renewal

To exercise them you'd need, on the KNOXTOKEN service:

<param><name>knox.token.exp.server-managed</name><value>true</value></param>
<param><name>knox.token.renewer.whitelist</name><value>guest</value></param>

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:

  • Revoke → enforcement (the key one): mint → revoke → re-present to the federated endpoint and assert it's now 401. The {"revoked":"true"} response alone doesn't prove revocation is enforced.
  • Renew: returns {"renewed":"true", ...} and the renewed token still federates.
  • Authorization negative: a non-whitelisted user gets 403 on renew/revoke.
  • enable/disable: disable a token, confirm it stops working; cover the "already enabled/disabled" 400 paths.

2. Two minor notes on the existing tests:

  • test_federated_topology_rejects_invalid_token uses "not.a.valid.jwt" (structurally malformed). Consider also a well-formed-but-wrong-signature token so the RS256 signature check (jwt.expected.sigalg) is exercised, not just the parser. This is doable by removing the last character of the generated token's access_key field.
  • The topologies set knoxsso.token.ttl, but the documented KNOXTOKEN TTL param is knox.token.ttl if I were you, I'd change them in the scope of this PR.

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Hi @smolnar82 sure i will add them as well, thank you

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Hi @smolnar82 , can you review the pr again?, thanks

Comment thread .github/workflows/build/gateway.sh Outdated
Comment on lines +37 to +39
# 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

@smolnar82 smolnar82 Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Hi @smolnar82 i have made the changes can you check it once?, thanks

@smolnar82

Copy link
Copy Markdown
Contributor

@Raghav-Mah3shwari - Can you please rebase your changes on the latest HEAD on master? Last Friday we merged quite a huge PR which affects the token paths and I'd like to make sure your changes are in sync with them.
Thanks!

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Sure @smolnar82

@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Hi @smolnar82 i rebased my local branch with upstream, it is passing, i m not sure if i have the access to rebase my pr here, can you please check it and merge it ? thank you

@hanicz

hanicz commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

@Raghav-Mah3shwari Your branch is still 19 commits behind master. If you reabase locally and push it it should show up in the PR.

smolnar82 and others added 16 commits August 24, 2026 02:34
…eported by Quinn Nguyen) (apache#1341)

* KNOX-3409: Validate knoxauth theme name to prevent DOM-based XSS

The knoxauth login page read the "theme" query parameter and wrote it into
a <link> tag via document.write() with no validation, allowing arbitrary
markup injection on the page that collects user credentials. The value was
also persisted to localStorage and replayed on later visits, so a single
malicious link kept executing on subsequent visits from a clean URL.

Theme names are now validated against ^[a-zA-Z0-9_-]{1,64}$ on every path
(URL parameter, localStorage, and the configured default), which rejects
quotes, angle brackets, dots and path separators. The stylesheet element is
built with DOM APIs instead of string concatenation, so a theme name can
never be parsed as markup. A stored value that fails validation, or that
does not resolve to an installed theme, is discarded rather than replayed.

Because the only URL the loader can produce is styles/themes/<name>/theme.css,
the themes actually installed on the server remain the effective allowlist and
no new configuration is required. The README and deployment guide are updated
to describe the validation, replacing two security claims that were inaccurate
as written.

Reported by Quinn Nguyen. Not present in any released version - the theming
feature (KNOX-3283) has only ever been on master.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* KNOX-3409: Discard an unresolvable theme preference on the same visit

Addresses review feedback. The cleanup flag tracked whether the theme had
been read from localStorage rather than whether it is currently held there.
A theme supplied via ?theme= is persisted immediately, so it is equally
eligible for cleanup, but the flag stayed false and no onerror handler was
attached. A name that passed validation without resolving to an installed
theme was therefore saved and replayed once before being cleared.

The flag is renamed to themeIsPersisted and set after a successful
setItem, so it reflects the invariant the cleanup actually depends on.
Setting it inside the try means a failed write - localStorage disabled -
correctly leaves nothing to clean up. Behaviour for an admin-configured
theme is unchanged: a deployment error does not discard a user preference.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
apache#1343)

* KNOX-3410: KnoxToken enable/disable endpoints perform no caller authorization

* KNOX-3410: Address review comments
apache#1344)

* KNOX-3411: KnoxToken getUserTokens returns every user's token metadata without a caller authorization check

* KNOX-3411: Add test clean-up
* KNOX-3416: KnoxSSO redirects to untrusted site

* KNOX-3416: Clarify error message for userInfo
Apache Knox as an OAuth 2.0 / OpenID Connect Authorization Server. 
Supports the Authorization Code flow (with PKCE for public clients), refresh-token and token-exchange grants, dynamic client registration, a discovery document + JWKS endpoint, and brokered login to external OIDC Providers with persisted federated identities.

Includes the Trusted OIDC Issuer registry and admin API (KNOX-3355,
KNOX-3368), gateway-config plumbing (KNOX-3390), and token-exchange
extensions for dynamic JWKS and issuer attributes (KNOX-3405, KNOX-3408).

Security hardening (KNOX-3414) reviewed and tested across the feature:

- Client authentication on every code/refresh redemption — PKCE (S256
  only) or a constant-time client_secret check; no unauthenticated path.
- Single-use authorization codes and refresh tokens, atomically consumed
  before issuance to close replay/rotation races.
- Full federated id_token validation (signature, issuer, audience,
  exp/nbf, required sub, and OIDC nonce binding); fail closed on
  misconfiguration.
- Open-redirect prevention at registration and authorize (HTTPS-only
  except loopback, normalized-path wildcard match, scope whitelist).
- Secrets at rest via AliasService (fail closed); federated access tokens
  never persisted; consent bound to the initiating subject (POST-only).
- Secure-by-default flags (anonymous registration, auto-consent), correct
  RFC 6749/6750 error statuses, XSS escaping, structured audit records,
  and signing-key rotation (multi-JWK publication, kid-based verification).

Documented in a new KnoxIDF book (overview, getting started, endpoint and
configuration reference, security, federation, operations).

Co-authored-by: hsheinblatt <hs7@hotmail.com>
Co-authored-by: Harrison <hsheinblatt@cloudera.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pache#1347)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
apache#1350)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Raghav-Mah3shwari and others added 3 commits August 24, 2026 02:34
…integration test README (apache#1324)

Co-authored-by: Raghav <maheshwari@Raghavs-MacBook-Air.local>
* KNOX-3278: Update jline.version to 3.25.1.

* KNOX-3278: Update groovy to 5.0.4 and jline to 3.30.6, plus jna to 5.18.1.

* KNOX-3278: refactor commands and REPL shell.

* KNOX-3278: fix checkstyle errors and logging configuration in maven-shade-plugin and knoxshell-log4j. Update aspectj for JDK 17.

* KNOX-3278: Fix forbiddenapis check: use Locale.ROOT in printf.

* KNOX-3278: Fix KnoxShellTableCallHistoryTest shouldRollbackToValidPreviousStep

* KNOX-3278: Removing comments and unused methods

* KNOX-3278: Fix missing newlines at end of file.

* KNOX-3278: Restoring SelectCommand to use Swing JTextArea for multiline edits.

* KNOX-3278: Removing KnoxLoginDialog (JLine3 supports password input).

* KNOX-3278: Correcting undeclared jline module dependencies.

* KNOX-3278: Correcting pmd findings.

* KNOX-3278: Update rest-assured to 6.0.0 (needed for Groovy 5)

* KNOX-3278: command completion pt1

* KNOX-3278: command completion pt2

* KNOX-3278: correct checkstyle errors

* KNOX-3278: correct undeclared jline module dependencies.

* KNOX-3278: cleanup DataSourceCommand completer.

* KNOX-3278: add :x and :q as exit commands.

* KNOX-3278: add '?' as help command alias.

* KNOX-3278: import, load and show commands added.

* KNOX-3278: purge command added.

* KNOX-3278: Correcting load command and alias.

* KNOX-3278: Adding completer for ShowCommand.

* KNOX-3278: Correct checkstyle error on shortcut handling

* KNOX-3278: Renaming SimpleCommandRegistry to KnoxShellCommandRegistry and cleaning up command handling. Overriding name() so that tab completion after semicolon does not show the name of the command registry.

* KNOX-3278: No need to exclude Log4j2Plugins.dat from the shaded knoxhsell jar.

* KNOX-3278: correcting knoxshell.sh to use -jar instead of main class (to use Launcher taken from knoxshell.jar manifest main class)

* KNOX-3278: excluding Log4j2Plugins.dat from the shaded knoxhsell jar and making knoxshell-log4j2.xml consistent with other log4j2 configurations.

* KNOX-3278: get rid of useless warning javax.* types are not being woven

* KNOX-3278: fix log4j2 WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance.

* KNOX-3278: Fix purge command, simplify import and show commands.

* KNOX-3278: Fix ImportCommand and add support for static imports.

* KNOX-3278: Catch Throwable similarly to legacy GroovySh.

* KNOX-3278: Make LoadCommand handle multi-file loading as in legacy GroovySh 4.

* KNOX-3278: wrap GroovyEngine completer into SafeCompleter to handle unexpected exceptions.

* KNOX-3278: correct knoxshell.sh to use launcher and correctly load classes from the lib directory.

* KNOX-3278: fix no endline at end of file.

* KNOX-3278: refactor Shell.java pt1

* KNOX-3278: refactor Shell.java pt2

* KNOX-3238: review findings: keep Launcher because of logging and remove duplicate classpath argument.

* KNOX-3278: review findings: Restore KnoxLoginDialog for credential collection logic.

* KNOX-3278: review findings: Revert LoginCommand to use KnoxLoginDialog.

Reverts LoginCommand.execute() to use KnoxLoginDialog for credential collection
instead of JLine3 terminal prompts, restoring the Swing GUI popup dialog behavior.

This maintains consistency with the `:login` command behavior from before KNOX-3278 refactoring, as documented in the original pre-refactor version.

* KNOX-3278: review findings: Replace inline FQN with import

* KNOX-3278: review findings: revert trailing newlines in files that would otherwise not be changed.

* KNOX-3278: review findings: remove override for KnoxShellCommandRegistry.name()

* KNOX-3278: correcting load command to parse filenames with spaces as well

* KNOX-3278: correcting Groovy 5 extension module registration.

* KNOX-3278: Ctrl+C continues back to the prompt; Ctrl+D exits the shell.

* KNOX-3278: review findings: revert trailing newlines in files that would otherwise not be changed.

* KNOX-3278: review findings: fix purge command javadoc and help.
* KNOX-3421 - Upgrade Shiro to 2.2.1

* KNOX-3421 - Review fixes

* KNOX-3421 - Test and add LDAP caching tests

* KNOX-3421 - Address build failures
@Raghav-Mah3shwari

Copy link
Copy Markdown
Contributor Author

Hi @smolnar82 i did rebase and pushed but i m not sure if this is correct or not, let me know if this looks bad, i will create a new branch and raise pr for jwt tests from there, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants