KNOX-3368 - Trusted OIDC Issuer admin API for Knox IDF#1321
KNOX-3368 - Trusted OIDC Issuer admin API for Knox IDF#1321hsheinblatt wants to merge 8 commits into
Conversation
* KnoxIDF - Initial commit * KnoxIDF - multi OP support * KnoxIDF - make token endpoint configurable during discovery * KnoxIDF - Code cleanup and bug fixes * KnoxIDF - Multi OP enablement improvements and code adoption to Larry's recent changes * KnoxIDF - Add REFRESH_TOKEN support * KnoxIDF - Automatically enable JdbcFederatedIdentityService when KnoxIDF is present in any topology * KnoxIDF - Added Docker-based integration tests * KnoxIDF: configurable user params provider (only LDAP for now) * KnoxIDF: add support for auth code flow with PKCE * KnoxIDF: fix an issue with the empty user params provider implementation * KnoxIDF: Refactor Docker build to use local Maven artifacts and unify CI/Dev workflows
…#1311) * KNOX-3355 - Add TrustedOidcIssuerService schema and interface Co-authored-by: Harrison <hsheinblatt@cloudera.com>
…d TrustedOidcIssuerServiceFactory (apache#1315)
How this patch was tested: New unit tests are added. Follow on changes will integrate in the implementation into KnoxIDF flows, but now there are no integration or UI changes.
and to use audit sentinel values without special characters.
Test Results46 tests 46 ✅ 8s ⏱️ Results for commit 70ca9da. |
| <mina.version>2.2.8</mina.version> | ||
| <netty.version>4.1.135.Final</netty.version> | ||
| <nimbus-jose-jwt.version>10.9.1</nimbus-jose-jwt.version> | ||
| <oauth2-oidc-sdk.version>11.37.2</oauth2-oidc-sdk.version> |
There was a problem hiding this comment.
This must be left here because of the recent changes in #1320
| protected String[] getPackages() { | ||
| return new String[] { "org.apache.knox.gateway.service.knoxidf" }; | ||
| } | ||
|
|
||
| @Override | ||
| protected String[] getPatterns() { | ||
| return new String[] { "knoxidf/api/**?**" }; | ||
| } |
There was a problem hiding this comment.
KNOXIDF_ADMIN and KNOXIDF share the same package AND URL pattern. Both contributors return:
getPackages()→{"org.apache.knox.gateway.service.knoxidf"}getPatterns()→{"knoxidf/api/**?**"}
Because Jersey scans the whole package, the TrustedOidcIssuersResource admin endpoints are served by the KNOXIDF role too, not just KNOXIDF_ADMIN.
If the intent is for admin operations to sit behind stricter authorization (a separate role usually implies a separate authz/ACL binding in the topology), that intent is defeated: the same resource is reachable through the non-admin role. Conversely, deploying both roles in one topology with the identical knoxidf/api/? pattern invites a routing conflict. The test even notes this ("Same prefix … so both roles can coexist … Jersey disambiguates by @Path"), but path-based disambiguation doesn't give you role-based access control.
Is KNOXIDF_ADMIN meant to be independently protected, and if so, shouldn't its package/pattern be scoped to the admin resources only?
| try { | ||
| final Map<String, Object> parsed; | ||
| try { | ||
| parsed = new ObjectMapper().readValue(body, new TypeReference<Map<String, Object>>() {}); |
There was a problem hiding this comment.
ObjectMapper is expensive to construct and thread-safe to reuse: make it a private static final, or parse via JsonUtils for consistency with the rest of the class.
2 participants
KNOX-3368 - Trusted OIDC Issuer admin API for Knox IDF
What changes were proposed in this pull request?
Implement the admin API for the trusted OIDC issuer service
along with the contributor for it.
How was this patch tested?
Unit tests were added
Integration Tests
N/A will be added once the flow can be tested.