Skip to content

fix(auth): isolate unsafe account merge flow - #639

Closed
XiaoSeS wants to merge 1 commit into
mainfrom
fix/auth-account-merge-isolation-main
Closed

fix(auth): isolate unsafe account merge flow#639
XiaoSeS wants to merge 1 commit into
mainfrom
fix/auth-account-merge-isolation-main

Conversation

@XiaoSeS

@XiaoSeS XiaoSeS commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

What

  • keep the three legacy account merge routes but make authenticated valid requests fail closed with one localized 503 Service Unavailable response
  • remove the unsafe AccountMergeService, so the old verification-token and destructive migration path is no longer a callable Spring service
  • replace the account settings merge form and token controls with an English/Chinese security notice
  • preserve the legacy table and rows without completing or deleting requests
  • add operator guidance and a detailed acceptance design for the future safe Account Merge implementation

Why

The legacy initiate route returned the secondary-account verification token to the already-authenticated primary-account session. That same session could verify and confirm the request without independently authenticating as the secondary account. The flow could then migrate bindings, API tokens, platform roles, namespace memberships, and local credentials.

This PR is the P0 containment change. It intentionally does not implement the future dual-reauthentication merge flow.

Closes #634
Parent: #628

How

AccountMergeController retains the published paths and request/response schemas for deployed-client compatibility, but no longer delegates to a mutation service. Valid authenticated requests terminate with error.auth.merge.temporarilyUnavailable. Invalid authentication or CSRF is still rejected by the existing security chain.

The legacy JPA entity, repository, schema, and rows remain available for forensics and a future explicit migration. The unsafe orchestration service and its success-path tests are removed. The replacement controller tests assert the stable 503 response and absence of proof/request data.

Testing

Clean origin/main branch:

  • AccountMergeControllerTest: 5 tests passed
  • account settings frontend test: passed
  • pnpm typecheck: passed
  • pnpm lint: passed

Equivalent implementation tree before the clean-main cherry-pick:

  • frontend full suite: 181 files / 618 tests passed
  • backend package build passed
    • ./mvnw -pl skillhub-app -am -DskipTests package
  • full backend-app suite on this host requires -Dnet.bytebuddy.experimental=true because only Java 25 is installed and the repository's Byte Buddy version officially supports through Java 22
  • after enabling that test-process-only compatibility switch, the suite reproduced the pre-existing ApiTokenAuthenticationFilterTest.shouldIgnoreNonBearerAuthorizationHeader global SecurityContextHolder order leak; that test passes alone (1/1) and is unrelated to this diff
  • local staging was not started because the shared host already has a user-owned PostgreSQL service on the staging compose's fixed port 5432; the implementation is integrated into big-main@f7360b69 for test-environment validation

Impact

  • intentional temporary removal of successful account merge behavior
  • no database migration
  • no route-path or OpenAPI schema removal
  • no existing merge request is completed, deleted, or converted
  • rollback to an older image would re-enable the unsafe implementation; operators must keep /api/v1/account/merge/* blocked if such a rollback is unavoidable
  • this draft must not merge to main until big-main and test-environment validation are complete

@CLAassistant

CLAassistant commented Jul 30, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ylhu16 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


ylhu16 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Keep the legacy routes fail-closed, remove the unsafe orchestration service, replace the UI controls with a security notice, and define the acceptance contract for the future safe merge flow.

Closes #634

Parent: #628
Signed-off-by: ylhu16 <ylhu16@iflytek.com>
@XiaoSeS
XiaoSeS force-pushed the fix/auth-account-merge-isolation-main branch from fb9a46a to 14da7d3 Compare August 4, 2026 10:49
@XiaoSeS
XiaoSeS marked this pull request as ready for review August 4, 2026 10:49
@XiaoSeS

XiaoSeS commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidating the unified identity work back into the parent tracking issue #628 and design PR #630 per maintainer direction. Closing this split delivery item so follow-up discussion and acceptance stay in one place.

@XiaoSeS XiaoSeS closed this Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(auth): disable unsafe legacy account merge flow

2 participants