smolnar82 opened a new pull request, #1319: URL: https://github.com/apache/knox/pull/1319
[KNOX-3390](https://issues.apache.org/jira/browse/KNOX-3390) - Address review feedback for TrustedOidcIssuerService ## What changes were proposed in this pull request? Follow-up to #1315 addressing review comments on the `TrustedOidcIssuerService` implementation. Four items: 1. Surface registry-reload failures instead of swallowing them (J`dbcTrustedOidcIssuerService). reloadRegistrySnapshot()` previously caught, logged, and discarded any exception. Because it runs after a committed insert/delete, a reload failure would leave the in-memory snapshot silently diverged from the database while register/deregister still returned success. It now logs and re-throws as a `RuntimeException`, which register/deregister propagate (matching the interface's documented "throws RuntimeException on storage error") and which `init()` continues to wrap into `ServiceLifecycleException` so startup still fails cleanly. 2. Canonicalize issuer URLs on write and lookup (`JdbcTrustedOidcIssuerService`). Added trailing-slash normalization (a single trailing / stripped, null-safe, consistent with the stripping OIDCDiscoveryHelper already applies when building the discovery URL). `register` now persists the canonical form, and all lookup paths (`isTrusted`, `isDynamicJwks`, `resolveJwksUri`, `refreshJwksUri`, `deregister`) normalize their argument. This prevents a registration of `https://idp.example.com/` from failing to match an `iss` claim of `https://idp.example.com` (and vice versa). 3. Remove dead code: dropped the unused `count()` method and `COUNT_SQL` constant from `TrustedOidcIssuerDatabase` (the max-issuers check uses the in-memory snapshot size, so `count()` had no callers). 4. Restore alphabetical ordering in `pom.xml`: moved the `oauth2-oidc-sdk.version` property after `nodejs.version`. ## How was this patch tested? Existing unit tests plus a new one, run via Maven: ``` mvn -pl gateway-server test -Dtest='JdbcTrustedOidcIssuerServiceTest,\ TrustedOidcIssuerServiceFactoryTest,OIDCDiscoveryHelperTest,\ EmptyTrustedOidcIssuerServiceTest,TrustedOidcIssuersSchemaTest' Result: 46 tests, 0 failures, 0 errors (JdbcTrustedOidcIssuerServiceTest 17, OIDCDiscoveryHelperTest 12, TrustedOidcIssuerServiceFactoryTest 8, EmptyTrustedOidcIssuerServiceTest 7, TrustedOidcIssuersSchemaTest 2). ``` Added `JdbcTrustedOidcIssuerServiceTest#testIssuerUrlTrailingSlashNormalized`, which verifies: registering with a trailing slash and looking up without one (and vice versa) matches; the canonical (slash-stripped) form is what gets persisted; and deregistering via the non-canonical form still removes the entry. ## Integration Tests N/A ## 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]
