Skip to content

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

Closed
XiaoSeS wants to merge 20 commits into
mainfrom
fix/auth-account-merge-isolation
Closed

fix(auth): isolate unsafe account merge flow#637
XiaoSeS wants to merge 20 commits into
mainfrom
fix/auth-account-merge-isolation

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

  • AccountMergeControllerTest: 5 tests passed
  • frontend full suite: 181 files / 618 tests passed
  • pnpm typecheck: passed
  • pnpm lint: 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; this draft must be integrated into big-main and validated in the test environment

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

XiaoSeS and others added 20 commits July 28, 2026 22:30
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Signed-off-by: ylhu16 <ylhu16@iflytek.com>
Closes #632

Signed-off-by: ylhu16 <ylhu16@iflytek.com>
Signed-off-by: ylhu16 <ylhu16@iflytek.com>
Signed-off-by: ylhu16 <ylhu16@iflytek.com>

# Conflicts:
#	docs/02-domain-model.md
#	server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/identity/IdentityBindingServiceTest.java
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>
@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 all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ XiaoSeS
❌ ylhu16


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.

@XiaoSeS

XiaoSeS commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #639. This draft was created from the big-main integration branch and therefore included unrelated integration ancestry when compared with main. #639 contains the same P0 0C change as one clean commit on top of origin/main. No review should be performed on this superseded draft.

@XiaoSeS XiaoSeS closed this Jul 30, 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