Skip to content

Swap custom JdbcRegisteredClientRepository for Spring's stock implementation #127

@dfcoffin

Description

@dfcoffin

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

  1. TokenSettings ClassCastExceptionnew ObjectMapper() in the constructor without OAuth2AuthorizationServerJackson2Module registered. OAuth2TokenFormat.REFERENCE serializes to JSON correctly but reads back as LinkedHashMap (line 326: objectMapper.readValue(tokenSettings, new TypeReference<Map<String, Object>>() {})). TokenSettings.withSettings(settings).build() doesn't reconstruct the typed value, so later calls to TokenSettings.getAccessTokenFormat() throw ClassCastException: LinkedHashMap cannot be cast to OAuth2TokenFormat. The JwtGenerator then runs for clients configured as opaque, returning HTTP 500 on every POST /oauth2/token request.
  2. Defect refactor: standardize repository naming convention #8 from #122 auditfindAll() 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}secret confusion 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

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    ESPI 4.0Touches the NAESB ESPI 4.0 implementationbugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions