fix(authserver): six pre-existing defects blocking dev-mysql boot#125
Merged
Conversation
Discovered during Phase 2.0 auth-server bring-up (#122). The auth-server had not been booted clean against a fresh MySQL in living memory; static audit only surfaced these defects under actual runtime. Six independent blockers patched here; one more (filter chain canonical pattern) tracked as #124, and ApplicationInformation schema repair as #123. Patch 1 — MySQL V1 FK uniqueness V1_0_0__create_oauth2_schema.sql declared an FK from espi_application_info.client_id to oauth2_registered_client.client_id, but the referenced column had no unique constraint (only PRIMARY KEY on id). MySQL rejected the CREATE TABLE. Added UNIQUE KEY on oauth2_registered_client.client_id and removed the now-redundant non-unique CREATE INDEX. PostgreSQL V1 was already correct. Patch 2 — Flyway location config / source layout mismatch Source files live at db/vendor/{h2,mysql,postgresql}/ but 4 profile YAMLs plus base application.yml plus docker-compose env vars pointed at classpath:db/migration/{mysql,postgresql,h2}/. Boot only "worked" historically because of stale target/classes/db/migration/ artifacts from a prior layout; mvn clean exposed the gap. Updated all 6 config references to classpath:db/vendor/... per the user's preference to keep the current vendor-organized source layout. Patch 3 — Flyway target=2.0.0 (workaround for #123) V3+ migrations INSERT into espi_application_info columns that don't exist in MySQL V1 (client_description, contact_name, contact_email, scope, grant_types, response_types). Root cause is schema drift from the ESPI 4.0 XSD; full repair tracked as #123. Set spring.flyway.target: "2.0.0" in application-dev-mysql.yml so boot succeeds with V1+V2 schema (sufficient for OAuth2 grant + introspect; V3+ is seed/demo data). Patch 4 — JWT-only resource server filter chain AuthorizationServerConfig.authorizationServerSecurityFilterChain declared both .opaqueToken(Customizer.withDefaults()) and OIDC (which auto-wires JWT validation) on the same chain. Spring Security 7.x refuses by design: "Spring Security only supports JWTs or Opaque Tokens, not both at the same time." Removed .opaqueToken; the chain now uses .jwt(Customizer.withDefaults()). Outbound tokens to ESPI clients remain opaque — controlled per-RegisteredClient via accessTokenFormat(OAuth2TokenFormat.REFERENCE), unaffected by this filter-chain change. Patch 5 — Disabled @order(0) header-injection chains HttpsEnforcementConfig declared two SecurityFilterChain @order(0) beans (httpsEnforcementFilterChain for prod, developmentSecurityFilterChain for dev profiles) with securityMatcher("/**") that monopolized every request, preempting the auth-server's @order(1) chain. Result: every OAuth2 endpoint returned 404. Header injection should be a HeaderWriter or Filter, not a SecurityFilterChain claiming /**. Disabled both @bean annotations; AuthorizationServerConfig's own chain already configures equivalent security headers. Re-introducing the dev-friendly cache-control headers via a proper mechanism is a follow-up cleanup item. Patch 6 — HikariCP auto-commit application-dev-mysql.yml had auto-commit: false but JdbcRegisteredClientRepository.save() (and likely other repository methods) declares no @transactional. With auto-commit off and no declared transaction, INSERTs execute against an uncommitted connection and silently roll back when the connection returns to the pool. Boot logged "Inserted registered client: 1" three times while the DB had 0 rows. Set auto-commit: true to make the seed flow work. Architectural fix (proper @transactional throughout the repository) deferred — tracked as part of #122 / Phase 2.0 cleanup. What works after these six patches - mvn clean install of openespi-authserver succeeds - mvn -pl openespi-authserver spring-boot:run reaches Started state in ~35-42s - Flyway V1+V2 apply cleanly to a fresh MySQL 8.4 - Tomcat listens on port 9999 - Three default ESPI clients persist correctly: data_custodian_admin, third_party, third_party_admin - Discovery endpoints return 200: /.well-known/oauth-authorization-server /.well-known/openid-configuration /oauth2/jwks /login What does NOT work yet (tracked separately) - POST /oauth2/token returns 401 — resource-server bearer filter preempts the token endpoint. Fix tracked as #124 (filter chain needs the canonical OAuth2AuthorizationServerConfiguration .applyDefaultSecurity pattern with a second @order(2) chain). - JdbcRegisteredClientRepository.findAll() returns empty list even when rows exist. Boot logs "Default ESPI Clients: 0" while DB has 3 rows. Defect #8 from the #122 audit; not yet diagnosed. - V3-V6 Flyway migrations skipped via target=2.0.0 pending #123. Refs: #122 #123 #124 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
dfcoffin
added
bug
4 tasks
dfcoffin
added a commit
that referenced
this pull request
May 25, 2026
PR #125 patched defects discovered while booting dev-mysql, but two of those patches needed to be propagated to other profiles to prevent the same bugs from biting elsewhere. Audit summarized at #122 Patch 1 — prod profile flyway.target=2.0.0 application-prod.yml uses MySQL vendor migrations, which means a first deploy against a clean prod DB would hit the same V3 schema drift that #125 patched on dev-mysql ("Unknown column 'client_description'"). Added the same target=2.0.0 workaround with a pointer to #123. Will be removed once #123 lands. Patch 2 — H2 V1 UNIQUE on oauth2_registered_client.client_id H2 V1 schema had only PRIMARY KEY (id) on oauth2_registered_client, no unique constraint on client_id. Not blocking H2 boot today (H2's espi_application_info table doesn't declare an FK referencing it), but client_id is unique by OAuth2 semantics and MySQL/PostgreSQL V1 both enforce uniqueness. Added UNIQUE constraint and removed the now-redundant non-unique CREATE INDEX, mirroring the MySQL cleanup from #125. Audited but no change needed - HikariCP auto-commit (patch #6 from #125): dev-postgresql, local, prod, and docker all rely on the HikariCP default (true). The dev-mysql auto-commit: false was an outlier bug, not a shared default. - PostgreSQL V3 INSERT: PostgreSQL V1 already has the columns V3 targets (client_description, contact_*, scope, grant_types, response_types). Different drift pattern from MySQL — no target=2.0.0 workaround needed on dev-postgresql at this time. (V4-V6 drift TBD as part of #123.) Refs: #122 #123 #125 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 25, 2026
dfcoffin
added a commit
that referenced
this pull request
May 26, 2026
…pring stock (#130) Closes #127. The hand-written JdbcRegisteredClientRepository (335 lines) reimplemented Spring Authorization Server's stock repository and carried multiple defects: the TokenSettings/ClientSettings serialization bug worked around in #128, auto-encoding of client secrets on save, and a findAll() that returned empty under the autocommit defect fixed in #125. This replaces it with Spring's stock org.springframework.security.oauth2.server.authorization.client.JdbcRegisteredClientRepository, which is maintained upstream and uses the correct Jackson modules out of the box. Changes: - AuthorizationServerConfig: construct the stock JdbcRegisteredClientRepository (JdbcOperations-only constructor); initializeDefaultClients() now takes the RegisteredClientRepository interface and no longer calls the custom findAll(). - OAuth2ClientManagementConfig: PasswordEncoder bean is now PasswordEncoderFactories.createDelegatingPasswordEncoder() instead of a bare BCryptPasswordEncoder(12). The stock repo stores client secrets verbatim (prefix included) rather than re-encoding on save, so authentication needs a prefix-aware encoder: {bcrypt}... for production, {noop}... for dev seeds. The old bare bcrypt encoder ignored the prefix and is why {noop}secret failed with invalid_client until this change. - Default seed clients use distinct secrets ({noop}dc-secret, {noop}tp-secret, {noop}tpadmin-secret). The stock repo enforces client-secret uniqueness (a security check the custom repo lacked); three identical {noop}secret values were rejected with "duplicate client secret". - New RegisteredClientAdminDao: small JdbcTemplate-backed component exposing the two operations not on the RegisteredClientRepository interface that the admin UI needs - findAllClientIds() and deleteById(). OAuthAdminController now resolves each id through RegisteredClientRepository.findByClientId(), so listing goes through Spring's tested deserialization path. - OAuthAdminController: dropped the `instanceof JdbcRegisteredClientRepository` branches (and their dead fallback paths) in favor of the admin DAO. - Deleted the custom JdbcRegisteredClientRepository and its 559-line test. Updated AuthorizationServerConfigTest (stock class + 1-arg bean signature), OAuthAdminControllerTest (admin DAO mocks; removed 2 tests for the now-gone instanceof fallback), and the MySQL/PostgreSQL TestContainers tests (findAll/deleteById -> RegisteredClientAdminDao). Verified end-to-end against a fresh MySQL container (dev-mysql): POST /oauth2/token (data_custodian_admin:dc-secret, client_credentials) -> 200, 128-char opaque token (0 dots, REFERENCE format) POST /oauth2/introspect -> 200, RFC 7662 response (active, sub, aud, scope, iss, exp, iat, jti, client_id, token_type) POST /oauth2/token with wrong secret -> 401 (delegating encoder enforces auth) Net: +112 / -1045 lines. Pre-existing test debt (NOT introduced here): AuthorizationServerConfigTest's mock-HttpSecurity unit tests and OAuthAdminControllerTest's standaloneSetup security tests fail because of how they're written, and the authserver module is excluded from CI entirely. Both are documented in #129; this change leaves them no worse (OAuthAdminControllerTest 5 -> 4 failures). Refs: #122 #128 #129 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes six independent pre-existing defects that prevented
openespi-authserverfrom booting clean against MySQL. Discovered during Phase 2.0 auth-server bring-up (#122). Static audit had been optimistic; runtime exposed the full picture.This PR's scope is narrowly "boot reaches
Started AuthorizationServerApplicationstate" — it deliberately does NOT include the patches needed to actually mint and introspect tokens. Those are tracked as separate issues so each fix can be reviewed on its own merits.What's in this PR
oauth2_registered_client.client_idfails — referenced column has no unique constraintdb/vendor/mysql/V1_0_0__create_oauth2_schema.sqlUNIQUE KEY uk_oauth2_registered_client_client_id; removed redundant non-unique indexdb/migration/{vendor}/but source files live atdb/vendor/{vendor}/. Boot only "worked" before due to staletarget/classes/db/migration/artifacts.docker-compose.ymlclasspath:db/vendor/...client_description,contact_name, etc.). Tracked as #123.application-dev-mysql.ymlspring.flyway.target: "2.0.0"(V1+V2 are sufficient for OAuth2 grant + introspection; V3+ is seed/demo data)oauth2ResourceServer().opaqueToken(...)declared on a chain that also has OIDC (which auto-wires JWT) → Spring Security 7.x fails fastAuthorizationServerConfig.java.jwt(Customizer.withDefaults()). Outbound opaque tokens to ESPI clients are unaffected — controlled per-RegisteredClientviaaccessTokenFormat(OAuth2TokenFormat.REFERENCE).SecurityFilterChain @Order(0)beans withsecurityMatcher("/**")preempted the auth-server's@Order(1)chain → every OAuth2 endpoint 404HttpsEnforcementConfig.java@Beanannotations. Auth-server's own chain already configures equivalent headers. Re-introducing dev-friendly cache-control headers via a properHeaderWriter/Filteris a follow-up cleanup.auto-commit: false+ repositorysave()lacking@Transactional→ seed INSERTs silently roll back when the connection returns to the poolapplication-dev-mysql.ymlauto-commit: true. Proper architectural fix (@Transactionalthroughout repository) deferred.Diff: 9 files changed, +32 / -15 lines.
What works after this PR
mvn clean install -pl openespi-authserver -amsucceedsmvn -pl openespi-authserver -Dspring-boot.run.profiles=dev-mysql spring-boot:runreachesStarted AuthorizationServerApplication in ~35-42 secondsdata_custodian_admin,third_party,third_party_admin/.well-known/oauth-authorization-server/.well-known/openid-configuration/oauth2/jwks/loginWhat does NOT work yet (deliberately out of scope, tracked separately)
POST /oauth2/tokenreturns 401 — resource-server bearer filter preempts token endpoint. Filed as #124 (filter chain needs canonicalOAuth2AuthorizationServerConfiguration.applyDefaultSecurity()pattern + second@Order(2)chain).JdbcRegisteredClientRepository.findAll()returns empty even when rows persisted. Boot logsDefault ESPI Clients: 0despite 3 actual rows. Defect refactor: standardize repository naming convention #8 from the #122 audit; not yet investigated.target=2.0.0pending #123 schema repair.Why six patches in one PR
Each patch independently blocks the next defect from being discovered. Patches #1-#6 form a single dependency chain — none can be tested in isolation against MySQL because each one is masking the next. Splitting them would create six PRs that fail their own CI individually. Reviewing them together with the audit context (#122 comment) is the cleanest path.
Test plan
mvn clean install -pl openespi-authserversucceedsmvn spring-boot:run -Dspring-boot.run.profiles=dev-mysqlreachesStarted AuthorizationServerApplicationcurl http://localhost:9999/.well-known/oauth-authorization-serverreturns 200 with correct token/introspection URLsRelated
🤖 Generated with Claude Code