Repository navigation
Swap custom JdbcRegisteredClientRepository for Spring's stock implementation #127
Description
Summary
The auth-server uses a hand-written JdbcRegisteredClientRepository (openespi-authserver/src/main/java/org/greenbuttonalliance/espi/authserver/repository/JdbcRegisteredClientRepository.java, 335 lines) instead of Spring's stock org.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository. The custom implementation has at least two confirmed defects and is a recurring source of bugs.
Discovered during Phase 2.0 boot verification (#122 defect #8 and during PR #124 verification — see comments on those for the diagnostic trail).
Confirmed defects in the custom implementation
TokenSettingsClassCastException —new ObjectMapper()in the constructor withoutOAuth2AuthorizationServerJackson2Moduleregistered.OAuth2TokenFormat.REFERENCEserializes to JSON correctly but reads back asLinkedHashMap(line 326:objectMapper.readValue(tokenSettings, new TypeReference<Map<String, Object>>() {})).TokenSettings.withSettings(settings).build()doesn't reconstruct the typed value, so later calls toTokenSettings.getAccessTokenFormat()throwClassCastException: LinkedHashMap cannot be cast to OAuth2TokenFormat. TheJwtGeneratorthen runs for clients configured as opaque, returning HTTP 500 on everyPOST /oauth2/tokenrequest.- Workaround landed in PR Auth-server SecurityFilterChain: replace non-canonical pattern with Spring Authorization Server canonical config #124 ("narrow Jackson modules fix"): register
SecurityJackson2Modules+OAuth2AuthorizationServerJackson2Moduleon the existing customObjectMapper. Verified to unblock token issuance.
- Workaround landed in PR Auth-server SecurityFilterChain: replace non-canonical pattern with Spring Authorization Server canonical config #124 ("narrow Jackson modules fix"): register
- Defect refactor: standardize repository naming convention #8 from #122 audit —
findAll()returned empty list while DB had rows. May have been the autocommit defect (fixed in fix(authserver): six pre-existing defects blocking dev-mysql boot #125) manifesting through this code path, or a separate row-mapping bug. Worth re-verifying once stock is in place.
What the custom implementation provides beyond stock
| Custom behavior | Used by | Replacement plan if swapping to stock |
|---|---|---|
findAll() |
OAuthAdminController.listClients() (GET /clients) |
Add small admin DAO using JdbcTemplate.query("SELECT * FROM oauth2_registered_client", ...) |
deleteById(String id) |
OAuthAdminController.deleteClient() (DELETE /clients/{id}) |
Same admin DAO — JdbcTemplate.update("DELETE FROM oauth2_registered_client WHERE id = ?", id) |
passwordEncoder.encode(secret) on save |
AuthorizationServerConfig.initializeDefaultClients() seed loop |
Change seed .clientSecret("{bcrypt}secret") to .clientSecret("{noop}secret") (DelegatingPasswordEncoder treats {noop} prefix as cleartext). Cleartext test credential becomes just secret instead of literal {bcrypt}secret (which is what today's seed effectively created via double-encoding). Production seeds (env-driven) would use pre-bcrypted hashes directly. |
Cost of swap
| File | Change |
|---|---|
AuthorizationServerConfig.java |
Change import; change constructor call from new JdbcRegisteredClientRepository(jdbcTemplate, passwordEncoder) to new JdbcRegisteredClientRepository(jdbcTemplate); change initializeDefaultClients parameter type to RegisteredClientRepository; change 3 .clientSecret("{bcrypt}secret") to .clientSecret("{noop}secret") |
OAuthAdminController.java |
Remove 2 instanceof JdbcRegisteredClientRepository jdbcRepo checks; inject and use new RegisteredClientAdminDao for list/delete |
RegisteredClientAdminDao.java (NEW) |
Small JdbcTemplate-backed DAO for the 2 non-interface ops |
AuthorizationServerConfigTest.java |
Update 4 isInstanceOf(JdbcRegisteredClientRepository.class) assertions to use the stock class |
OAuthAdminControllerTest.java |
Update mocks to reflect new admin DAO injection |
MySqlTestcontainersIntegrationTest.java |
Update field type |
JdbcRegisteredClientRepository.java |
DELETE |
JdbcRegisteredClientRepositoryTest.java |
DELETE |
Estimated 2-3 hours for the refactor + regression test.
Why this matters
The custom implementation is a maintenance liability:
- Reimplements ~300 lines of Spring code that the Spring Security team already maintains
- Uses plain
new ObjectMapper()instead of the Jackson modules Spring provides for this exact purpose - Auto-encoding of client secrets on save is non-standard and led to the
{bcrypt}secretconfusion that required separate fixes during PR fix(authserver): six pre-existing defects blocking dev-mysql boot #125 verification - Has already surfaced two distinct serialization defects; further bugs are likely
Acceptance criteria
-
openespi-authserver/src/main/java/org/greenbuttonalliance/espi/authserver/repository/JdbcRegisteredClientRepository.javadeleted -
AuthorizationServerConfig.registeredClientRepository()constructs Spring's stockorg.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository(JdbcOperations) - Seeded test clients use
{noop}secretin dev, allowing cleartext basic-auth with secretsecret - New
RegisteredClientAdminDao(or equivalent) providesfindAll()anddeleteById()for theOAuthAdminControlleradmin endpoints - All 4 test files updated to use the stock class
- Integration test:
POST /oauth2/token→ 200 with opaque token;POST /oauth2/introspect→ RFC 7662 response. Same as Auth-server SecurityFilterChain: replace non-canonical pattern with Spring Authorization Server canonical config #124 acceptance, but verified against the stock repo. - PR Auth-server SecurityFilterChain: replace non-canonical pattern with Spring Authorization Server canonical config #124's "narrow Jackson modules fix" is reverted (the workaround becomes unnecessary)
- Defect refactor: standardize repository naming convention #8 (
findAll()returning empty when rows exist) re-verified — may already be fixed by autocommit change in fix(authserver): six pre-existing defects blocking dev-mysql boot #125, or by this refactor
Related
- #122 Phase 2.0 auth-server bring-up (where this defect class was discovered)
- #124 Filter chain canonical pattern (will land a narrow workaround for the Jackson-modules defect; this issue removes the need for that workaround)
- #125 Six pre-existing defects (where this was originally tagged as defect refactor: standardize repository naming convention #8)
Activity
fix(authserver): canonical Spring Security 7.x filter chain + Jackson…