From 8a7dfc5336d50902fff45c59c2ad792802d347ff Mon Sep 17 00:00:00 2001 From: Xiaoran Li <146931635+Jacky1n7@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:48:30 +0800 Subject: [PATCH] fix: use the user id returned by login instead of guessing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: use the user id returned by login instead of guessing it New API's /api/user/login returns the account's own id in `data.id`, but `login()` extracted only the access token and threw the rest away. Callers therefore fell back to `guessPlatformUserIdFromUsername()`, whose trailing digits regex only matches usernames that happen to end with the id (e.g. `linuxdo_80305`). For an email login such as `alice@example.com` it yields nothing, so requests go out without the `New-Api-User` header, the site answers 401, the balance stays 0 and the session is misjudged as expired — which sends every scheduled round back through a full password login. Surface `platformUserId` on LoginResult, populate it from the login payload, prefer it over the guess when creating an account, and persist it on auto-relogin too (merging extraConfig so the stored autoRelogin credentials survive). This also avoids the extra `discoverUserId()` round trip that checkin and balance refresh currently pay on every single call. * fix: carry the re-login identity back to the caller Addresses the review feedback on #610. `tryAutoRelogin()` persisted the authoritative `platformUserId` but returned only the access token, so callers kept using pre-login state: - the immediate retry still sent the stale `New-Api-User`, so it could fail even though the re-login succeeded - the later `mergeAccountExtraConfig(account.extraConfig, ...)` writes started from the pre-login snapshot and wrote the old id back over the one that had just been persisted Both helpers now return `{ accessToken, platformUserId, extraConfig }`. `checkinService` adopts the id and refreshes `account.extraConfig` before retrying; `balanceService` does the same via its existing `activeExtraConfig` (`readBalance` closes over `platformUserId`, so the retry picks it up). Also from the review: - `extractLoginUserId()` no longer uses `Number.parseInt`, which accepted "80305abc" and "80305.9" as 80305 and would have produced a wrong `New-Api-User`. It now requires an all-digit string and a safe integer. - `login()` returns the exported `LoginResult` instead of redeclaring the shape inline, so the base contract and the adapter cannot drift apart. * fix: never let the guessed id overwrite the one login reported Follow-up to the review on #610. `storedPlatformUserId` and `guessedPlatformUserId` are computed once, before any re-login. When an account had no stored id, the guess was non-empty, and a re-login then reported the authoritative id, the success path still tested the pre-login snapshot and merged the guess back in — replacing a known-good id with digits scraped off the end of the username, which is the exact failure this PR exists to remove. The condition now checks the *current* `account.extraConfig` instead of the pre-login snapshot. `guessedPlatformUserId` is only ever set when `storedPlatformUserId` was empty, so behaviour is unchanged when no re-login happened. * test: cover the relogin identity path Per the review, the existing auto-relogin test only asserted the refreshed access token, so nothing pinned down the behaviour this PR is about. The new case uses `alice_1999` — a username whose trailing digits are not the account id, which is exactly what `guessPlatformUserIdFromUsername()` gets wrong — and asserts three things: - the first attempt falls back to the guess (1999), since nothing better exists yet - the retry after re-login uses the id the site reported (500123) - no subsequent write puts the guess back over it Verified the test actually bites: reverting either the identity propagation or the guarded guess-persist makes it fail. * test: also pin that relogin keeps the autoRelogin credentials Per the review, asserting only `platformUserId` left a hole: a write that *replaced* `extraConfig` with just `{ platformUserId }` would still satisfy the test, while dropping the `autoRelogin` credentials and leaving nothing to re-login with on the next round. The assertion now parses the whole persisted config and requires both the reported id and the intact credentials. Verified it bites: swapping `mergeAccountExtraConfig` for a plain overwrite makes it fail. * fix: resolve the login user id for every adapter, not just New API `VeloeraAdapter` inherits `BasePlatformAdapter.login()` untouched, and that base implementation discarded `res.data.id` exactly like the New API one did. Its `authHeaders()` only sends `Veloera-User` / `New-API-User` / `User-id` when an id was resolved, so the same e-mail-login failure this PR fixes for New API was still present on the adjacent adapter — and on anything else inheriting the base login path. Moved `extractLoginUserId()` from `NewApiAdapter` onto `BasePlatformAdapter` as a protected helper and used it in the base `login()`. `NewApiAdapter` now inherits it instead of carrying a private copy, so the diff gets smaller while covering more adapters. Also tightened the test assertion: credentials are now required to survive *every* persisted config, not just one of them. `toContainEqual` would have passed while a later write dropped them. * refactor: keep login identity resolution out of route * fix: preserve concurrent config during relogin * fix: avoid replaying relogin config snapshots --------- Co-authored-by: Jacky Li Co-authored-by: cita-777 --- src/server/routes/api/accounts.ts | 10 +- .../services/accountExtraConfig.test.ts | 6 + src/server/services/accountExtraConfig.ts | 14 ++ .../balanceService.autoRelogin.test.ts | 136 +++++++++++++++++- src/server/services/balanceService.ts | 89 ++++++++++-- .../checkinService.autoRelogin.test.ts | 129 ++++++++++++++++- src/server/services/checkinService.ts | 62 +++++++- src/server/services/platforms/base.ts | 46 +++++- src/server/services/platforms/newApi.ts | 7 +- 9 files changed, 471 insertions(+), 28 deletions(-) diff --git a/src/server/routes/api/accounts.ts b/src/server/routes/api/accounts.ts index 0d36b3c..44ca89d 100644 --- a/src/server/routes/api/accounts.ts +++ b/src/server/routes/api/accounts.ts @@ -11,12 +11,12 @@ import { import { getCredentialModeFromExtraConfig, getProxyUrlFromExtraConfig, - guessPlatformUserIdFromUsername, hasOauthProvider, getSub2ApiAuthFromExtraConfig, mergeAccountExtraConfig, normalizeCredentialMode as normalizeCredentialModeInput, resolvePlatformUserId, + resolvePlatformUserIdFromLogin, type AccountCredentialMode, } from "../../services/accountExtraConfig.js"; import { encryptAccountPassword } from "../../services/accountCredentialService.js"; @@ -535,7 +535,13 @@ export async function accountsRoutes(app: FastifyInstance) { }; } - const guessedPlatformUserId = guessPlatformUserIdFromUsername(username); + // The id reported by the site itself is authoritative; guessing from the + // username only works when it ends with the id (e.g. `linuxdo_80305`) + // and silently fails for addresses like `alice@example.com`. + const guessedPlatformUserId = resolvePlatformUserIdFromLogin( + loginResult.platformUserId, + username, + ); // Auto-fetch API token(s) let apiToken: string | null = null; diff --git a/src/server/services/accountExtraConfig.test.ts b/src/server/services/accountExtraConfig.test.ts index bb7600d..479e929 100644 --- a/src/server/services/accountExtraConfig.test.ts +++ b/src/server/services/accountExtraConfig.test.ts @@ -13,6 +13,7 @@ import { mergeAccountExtraConfig, normalizeCredentialMode, resolvePlatformUserId, + resolvePlatformUserIdFromLogin, requiresManagedAccountTokens, supportsDirectAccountRoutingConnection, } from './accountExtraConfig.js'; @@ -36,6 +37,11 @@ describe('accountExtraConfig', () => { expect(resolvePlatformUserId(JSON.stringify({ platformUserId: 5001 }), 'linuxdo_7659')).toBe(5001); }); + it('prefers the login-reported user id over the username fallback', () => { + expect(resolvePlatformUserIdFromLogin(500123, 'alice_1999')).toBe(500123); + expect(resolvePlatformUserIdFromLogin(undefined, 'alice_1999')).toBe(1999); + }); + it('merges platformUserId into existing config without dropping keys', () => { const merged = mergeAccountExtraConfig( JSON.stringify({ diff --git a/src/server/services/accountExtraConfig.ts b/src/server/services/accountExtraConfig.ts index 2d0d9b4..9a29bac 100644 --- a/src/server/services/accountExtraConfig.ts +++ b/src/server/services/accountExtraConfig.ts @@ -345,6 +345,20 @@ export function resolvePlatformUserId(extraConfig?: ExtraConfigInput, username?: return getPlatformUserIdFromExtraConfig(extraConfig) || guessPlatformUserIdFromUsername(username); } +/** + * Resolve the platform identity returned by a login, with the legacy username + * suffix guess retained only as a compatibility fallback. + * + * Keeping this policy in the account service layer prevents HTTP route adapters + * from encoding platform-specific identity rules themselves. + */ +export function resolvePlatformUserIdFromLogin( + reportedPlatformUserId: number | undefined, + username?: string | null, +): number | undefined { + return normalizeUserId(reportedPlatformUserId) || guessPlatformUserIdFromUsername(username); +} + export function mergeAccountExtraConfig( extraConfig: ExtraConfigInput, patch: Record, diff --git a/src/server/services/balanceService.autoRelogin.test.ts b/src/server/services/balanceService.autoRelogin.test.ts index 482fd18..1d3d162 100644 --- a/src/server/services/balanceService.autoRelogin.test.ts +++ b/src/server/services/balanceService.autoRelogin.test.ts @@ -6,6 +6,7 @@ const adapterMock = { }; const selectAllMock = vi.fn(); +const selectGetMock = vi.fn(); const updateSetMock = vi.fn(); const insertValuesMock = vi.fn(); const reportTokenExpiredMock = vi.fn(); @@ -18,6 +19,7 @@ const undiciFetchMock = vi.fn(); vi.mock('../db/index.js', () => { const selectChain = { all: () => selectAllMock(), + get: () => selectGetMock(), where: () => selectChain, innerJoin: () => selectChain, from: () => selectChain, @@ -51,7 +53,7 @@ vi.mock('../db/index.js', () => { insert: () => insertChain, }, schema: { - accounts: { id: 'id', siteId: 'siteId', status: 'status' }, + accounts: { id: 'id', siteId: 'siteId', status: 'status', extraConfig: 'extraConfig' }, sites: { id: 'id' }, events: {}, }, @@ -88,6 +90,7 @@ describe('balanceService auto relogin', () => { adapterMock.getBalance.mockReset(); adapterMock.login.mockReset(); selectAllMock.mockReset(); + selectGetMock.mockReset(); updateSetMock.mockReset(); insertValuesMock.mockReset(); reportTokenExpiredMock.mockReset(); @@ -144,6 +147,137 @@ describe('balanceService auto relogin', () => { expect(reportTokenExpiredMock).not.toHaveBeenCalled(); }); + it('does not replay the relogin config snapshot after the balance retry', async () => { + const autoRelogin = { username: 'alice@example.com', passwordCipher: 'cipher' }; + let currentExtraConfig = JSON.stringify({ + autoRelogin, + proxyUrl: 'http://old-proxy.example', + }); + selectAllMock.mockReturnValue([ + { + accounts: { + id: 14, + username: 'alice@example.com', + accessToken: 'stale-token', + status: 'active', + extraConfig: currentExtraConfig, + }, + sites: { + id: 14, + name: 'concurrent-settings', + url: 'https://example.com', + platform: 'new-api', + }, + }, + ]); + selectGetMock.mockImplementation(() => ({ extraConfig: currentExtraConfig })); + + adapterMock.getBalance + .mockRejectedValueOnce(new Error('HTTP 401: access token required')) + .mockImplementationOnce(async () => { + currentExtraConfig = JSON.stringify({ + autoRelogin, + platformUserId: 500123, + proxyUrl: 'http://changed-during-retry.example', + }); + return { balance: 12, used: 1, quota: 13 }; + }); + decryptPasswordMock.mockReturnValue('plain-password'); + adapterMock.login.mockImplementation(async () => { + currentExtraConfig = JSON.stringify({ + autoRelogin, + proxyUrl: 'http://changed-during-login.example', + }); + return { + success: true, + accessToken: 'fresh-token', + platformUserId: 500123, + }; + }); + + const { refreshBalance } = await import('./balanceService.js'); + await refreshBalance(14); + + expect(selectGetMock).toHaveBeenCalledTimes(1); + const reloginWrite = updateSetMock.mock.calls + .map((call) => call[0] as Record) + .find((payload) => payload.accessToken === 'fresh-token'); + expect(JSON.parse(String(reloginWrite?.extraConfig))).toMatchObject({ + autoRelogin, + platformUserId: 500123, + proxyUrl: 'http://changed-during-login.example', + }); + + const finalBalanceWrite = updateSetMock.mock.calls + .map((call) => call[0] as Record) + .find((payload) => payload.balance === 12); + expect(finalBalanceWrite?.extraConfig).toBeUndefined(); + }); + + it('merges final balance metadata into configuration changed during the retry', async () => { + const autoRelogin = { username: 'alice@example.com', passwordCipher: 'cipher' }; + let currentExtraConfig = JSON.stringify({ + autoRelogin, + proxyUrl: 'http://old-proxy.example', + }); + selectAllMock.mockReturnValue([ + { + accounts: { + id: 16, + username: 'alice@example.com', + accessToken: 'stale-token', + status: 'active', + extraConfig: currentExtraConfig, + }, + sites: { + id: 16, + name: 'concurrent-settings-with-income', + url: 'https://example.com', + platform: 'new-api', + }, + }, + ]); + selectGetMock.mockImplementation(() => ({ extraConfig: currentExtraConfig })); + + adapterMock.getBalance + .mockRejectedValueOnce(new Error('HTTP 401: access token required')) + .mockImplementationOnce(async () => { + currentExtraConfig = JSON.stringify({ + autoRelogin, + platformUserId: 500123, + proxyUrl: 'http://changed-during-retry.example', + }); + return { balance: 12, used: 1, quota: 13, todayIncome: 2.5 }; + }); + decryptPasswordMock.mockReturnValue('plain-password'); + adapterMock.login.mockImplementation(async () => { + currentExtraConfig = JSON.stringify({ + autoRelogin, + proxyUrl: 'http://changed-during-login.example', + }); + return { + success: true, + accessToken: 'fresh-token', + platformUserId: 500123, + }; + }); + + const { refreshBalance } = await import('./balanceService.js'); + await refreshBalance(16); + + expect(selectGetMock).toHaveBeenCalledTimes(2); + const finalBalanceWrite = updateSetMock.mock.calls + .map((call) => call[0] as Record) + .find((payload) => payload.balance === 12); + const finalExtraConfig = JSON.parse(String(finalBalanceWrite?.extraConfig)); + expect(finalExtraConfig).toMatchObject({ + autoRelogin, + platformUserId: 500123, + proxyUrl: 'http://changed-during-retry.example', + }); + expect(finalExtraConfig.todayIncomeSnapshot?.latest).toBe(2.5); + }); + it('reports token expired when relogin is unavailable', async () => { selectAllMock.mockReturnValue([ { diff --git a/src/server/services/balanceService.ts b/src/server/services/balanceService.ts index 721cfe9..52e6c31 100644 --- a/src/server/services/balanceService.ts +++ b/src/server/services/balanceService.ts @@ -208,7 +208,22 @@ async function fetchTodayIncomeFromLogs(params: { return Math.round(totalIncome * 1_000_000) / 1_000_000; } -async function tryAutoRelogin(account: any, site: any): Promise { +/** + * Result of a successful automatic re-login. + * + * The access token alone is not enough: the login may also have reported the + * authoritative `platformUserId`. Callers need it for the retry that follows, + * and they need the merged `extraConfig` so their own later + * `mergeAccountExtraConfig(account.extraConfig, ...)` writes do not put the + * pre-login copy back and undo what was just persisted. + */ +type AutoReloginResult = { + accessToken: string; + platformUserId?: number; + extraConfig?: string; +}; + +async function tryAutoRelogin(account: any, site: any): Promise { const adapter = getAdapter(site.platform); if (!adapter) return null; @@ -224,16 +239,37 @@ async function tryAutoRelogin(account: any, site: any): Promise { ); if (!loginResult.success || !loginResult.accessToken) return null; + // Re-read after the network request: account settings may have changed while + // login was in flight, and merging into the original snapshot would overwrite + // those newer fields when the whole extraConfig value is persisted. + const latestAccount = loginResult.platformUserId + ? await db.select({ extraConfig: schema.accounts.extraConfig }) + .from(schema.accounts) + .where(eq(schema.accounts.id, account.id)) + .get() + : undefined; + const reloginExtraConfig = loginResult.platformUserId + ? mergeAccountExtraConfig( + latestAccount ? latestAccount.extraConfig : account.extraConfig, + { platformUserId: loginResult.platformUserId }, + ) + : undefined; + await db.update(schema.accounts) .set({ accessToken: loginResult.accessToken, + ...(reloginExtraConfig ? { extraConfig: reloginExtraConfig } : {}), status: account.status === 'expired' ? 'active' : account.status, updatedAt: new Date().toISOString(), }) .where(eq(schema.accounts.id, account.id)) .run(); - return loginResult.accessToken; + return { + accessToken: loginResult.accessToken, + platformUserId: loginResult.platformUserId, + extraConfig: reloginExtraConfig, + }; } export async function refreshBalance(accountId: number) { @@ -277,7 +313,7 @@ export async function refreshBalance(accountId: number) { }; } - const platformUserId = resolvePlatformUserId(account.extraConfig, account.username); + let platformUserId = resolvePlatformUserId(account.extraConfig, account.username); let activeAccessToken = account.accessToken; let activeExtraConfig = account.extraConfig; let balanceInfo: BalanceInfo | null = null; @@ -343,9 +379,16 @@ export async function refreshBalance(accountId: number) { await handleBalanceError(retryErr); } } else if (shouldAttemptAutoRelogin(message)) { - const refreshedAccessToken = await tryAutoRelogin(account, site); - if (refreshedAccessToken) { - activeAccessToken = refreshedAccessToken; + const relogin = await tryAutoRelogin(account, site); + if (relogin) { + activeAccessToken = relogin.accessToken; + // Adopt the id the re-login reported and advance activeExtraConfig. + // `readBalance` closes over `platformUserId`, so without this the retry + // still sends the stale `New-Api-User`; and `nextExtraConfig` further down + // starts from `activeExtraConfig`, so the pre-login copy would be written + // back over the id tryAutoRelogin() just persisted. + if (relogin.platformUserId) platformUserId = relogin.platformUserId; + if (relogin.extraConfig) activeExtraConfig = relogin.extraConfig; try { balanceInfo = await readBalance(activeAccessToken); } catch (retryErr: any) { @@ -380,14 +423,32 @@ export async function refreshBalance(accountId: number) { } catch {} } + const hasTodayIncomeUpdate = + typeof balanceInfo.todayIncome === 'number' && Number.isFinite(balanceInfo.todayIncome); + const hasSubscriptionUpdate = + !!balanceInfo.subscriptionSummary && isSub2ApiPlatform(site.platform); let nextExtraConfig = activeExtraConfig; - if (typeof balanceInfo.todayIncome === 'number' && Number.isFinite(balanceInfo.todayIncome)) { - nextExtraConfig = updateTodayIncomeSnapshot(nextExtraConfig, balanceInfo.todayIncome); - } - if (balanceInfo.subscriptionSummary && isSub2ApiPlatform(site.platform)) { - nextExtraConfig = mergeAccountExtraConfig(nextExtraConfig, { - sub2apiSubscription: buildStoredSub2ApiSubscriptionSummary(balanceInfo.subscriptionSummary), - }); + let shouldPersistNextExtraConfig = false; + + if (hasTodayIncomeUpdate || hasSubscriptionUpdate) { + // A retried balance request or income fallback may outlive another settings + // update. Merge only the new balance metadata into the latest configuration + // instead of replaying the snapshot captured immediately after re-login. + const latestAccount = await db.select({ extraConfig: schema.accounts.extraConfig }) + .from(schema.accounts) + .where(eq(schema.accounts.id, account.id)) + .get(); + nextExtraConfig = latestAccount ? latestAccount.extraConfig : activeExtraConfig; + + if (hasTodayIncomeUpdate) { + nextExtraConfig = updateTodayIncomeSnapshot(nextExtraConfig, balanceInfo.todayIncome!); + } + if (hasSubscriptionUpdate) { + nextExtraConfig = mergeAccountExtraConfig(nextExtraConfig, { + sub2apiSubscription: buildStoredSub2ApiSubscriptionSummary(balanceInfo.subscriptionSummary!), + }); + } + shouldPersistNextExtraConfig = true; } const existingRuntimeHealth = extractRuntimeHealth(nextExtraConfig); @@ -401,7 +462,7 @@ export async function refreshBalance(accountId: number) { lastBalanceRefresh: new Date().toISOString(), updatedAt: new Date().toISOString(), }; - if (nextExtraConfig !== account.extraConfig) { + if (shouldPersistNextExtraConfig) { updates.extraConfig = nextExtraConfig; } diff --git a/src/server/services/checkinService.autoRelogin.test.ts b/src/server/services/checkinService.autoRelogin.test.ts index 913c6e3..bb07bfa 100644 --- a/src/server/services/checkinService.autoRelogin.test.ts +++ b/src/server/services/checkinService.autoRelogin.test.ts @@ -11,12 +11,14 @@ const refreshBalanceMock = vi.fn(); const decryptPasswordMock = vi.fn(); const selectAllMock = vi.fn(); +const selectGetMock = vi.fn(); const insertValuesMock = vi.fn(); const updateSetMock = vi.fn(); vi.mock('../db/index.js', () => { const selectChain = { all: () => selectAllMock(), + get: () => selectGetMock(), where: () => selectChain, innerJoin: () => selectChain, from: () => selectChain, @@ -50,7 +52,7 @@ vi.mock('../db/index.js', () => { }), }, schema: { - accounts: { id: 'id', siteId: 'siteId', checkinEnabled: 'checkinEnabled', status: 'status' }, + accounts: { id: 'id', siteId: 'siteId', checkinEnabled: 'checkinEnabled', status: 'status', extraConfig: 'extraConfig' }, sites: { id: 'id' }, checkinLogs: {}, events: {}, @@ -87,6 +89,7 @@ describe('checkinService auto relogin', () => { refreshBalanceMock.mockReset(); decryptPasswordMock.mockReset(); selectAllMock.mockReset(); + selectGetMock.mockReset(); insertValuesMock.mockReset(); updateSetMock.mockReset(); }); @@ -130,6 +133,130 @@ describe('checkinService auto relogin', () => { expect(updateSetMock).toHaveBeenCalledWith(expect.objectContaining({ accessToken: 'fresh-token' })); }); + it('prefers the id reported by relogin over the guess, and never writes the guess back', async () => { + // `alice_1999` ends in digits that are not the account id — precisely the case + // guessPlatformUserIdFromUsername() gets wrong, since its /(\d{3,8})$/ only + // knows how to read a trailing number. + selectAllMock.mockReturnValue([ + { + accounts: { + id: 1, + username: 'alice_1999', + accessToken: 'expired-token', + status: 'active', + extraConfig: JSON.stringify({ + autoRelogin: { username: 'alice_1999', passwordCipher: 'cipher' }, + }), + }, + sites: { + id: 3, + name: 'kfc', + url: 'https://kfc-api.sxxe.net', + platform: 'new-api', + }, + }, + ]); + + adapterMock.checkin + .mockResolvedValueOnce({ success: false, message: '无权进行此操作，未登录且未提供 access token' }) + .mockResolvedValueOnce({ success: true, message: 'checked in' }); + decryptPasswordMock.mockReturnValue('plain-password'); + adapterMock.login.mockResolvedValue({ + success: true, + accessToken: 'fresh-token', + platformUserId: 500123, + }); + + const { checkinAccount } = await import('./checkinService.js'); + await checkinAccount(1); + + // The first attempt has nothing better than the guess... + expect(adapterMock.checkin.mock.calls[0][2]).toBe(1999); + // ...but the retry must use the id the site itself reported. + expect(adapterMock.checkin.mock.calls[1][2]).toBe(500123); + + // And no later write may put the guess back over it. Parse the whole config, + // not just the id: a write that *replaced* extraConfig with only + // `{ platformUserId }` would satisfy an id-only assertion while dropping the + // autoRelogin credentials, leaving nothing to re-login with next time. + const writtenConfigs = updateSetMock.mock.calls + .map((call) => (call[0] as Record | undefined)?.extraConfig) + .filter((cfg): cfg is string => typeof cfg === 'string') + .map((cfg) => JSON.parse(cfg) as { + platformUserId?: number; + autoRelogin?: { username?: string; passwordCipher?: string }; + }); + expect(writtenConfigs.length).toBeGreaterThan(0); + // Credentials must survive *every* write. `toContainEqual` would only prove one + // write got it right, while a later one could still drop them — and a single + // drop leaves nothing to re-login with next round. + for (const cfg of writtenConfigs) { + expect(cfg.autoRelogin).toEqual({ username: 'alice_1999', passwordCipher: 'cipher' }); + } + const writtenIds = writtenConfigs.map((cfg) => cfg.platformUserId); + expect(writtenIds).toContain(500123); + expect(writtenIds).not.toContain(1999); + }); + + it('preserves account configuration updated while auto relogin is in flight', async () => { + const autoRelogin = { username: 'alice@example.com', passwordCipher: 'cipher' }; + let currentExtraConfig = JSON.stringify({ + autoRelogin, + proxyUrl: 'http://old-proxy.example', + }); + selectAllMock.mockReturnValue([ + { + accounts: { + id: 15, + username: 'alice@example.com', + accessToken: 'expired-token', + status: 'active', + extraConfig: currentExtraConfig, + }, + sites: { + id: 15, + name: 'concurrent-settings', + url: 'https://example.com', + platform: 'new-api', + }, + }, + ]); + selectGetMock.mockImplementation(() => ({ extraConfig: currentExtraConfig })); + + adapterMock.checkin + .mockResolvedValueOnce({ success: false, message: 'access token required' }) + .mockResolvedValueOnce({ success: true, message: 'checked in' }); + decryptPasswordMock.mockReturnValue('plain-password'); + adapterMock.login.mockImplementation(async () => { + currentExtraConfig = JSON.stringify({ + autoRelogin, + proxyUrl: 'http://new-proxy.example', + }); + return { + success: true, + accessToken: 'fresh-token', + platformUserId: 500123, + }; + }); + + const { checkinAccount } = await import('./checkinService.js'); + await checkinAccount(15); + + expect(selectGetMock).toHaveBeenCalled(); + const writtenConfigs = updateSetMock.mock.calls + .map((call) => call[0]?.extraConfig) + .filter((value): value is string => typeof value === 'string') + .map((value) => JSON.parse(value) as Record); + expect(writtenConfigs.length).toBeGreaterThan(0); + for (const config of writtenConfigs) { + expect(config).toMatchObject({ + autoRelogin, + proxyUrl: 'http://new-proxy.example', + }); + } + expect(writtenConfigs.some((config) => config.platformUserId === 500123)).toBe(true); + }); + it('passes guessed platform user id when config does not include it', async () => { selectAllMock.mockReturnValue([ { diff --git a/src/server/services/checkinService.ts b/src/server/services/checkinService.ts index 06107ef..03e77ed 100644 --- a/src/server/services/checkinService.ts +++ b/src/server/services/checkinService.ts @@ -92,7 +92,22 @@ function inferRewardFromBalanceDelta(previousBalance: unknown, latestBalance: un return Math.round(delta * 1_000_000) / 1_000_000; } -async function tryAutoRelogin(account: any, site: any): Promise { +/** + * Result of a successful automatic re-login. + * + * The access token alone is not enough: the login may also have reported the + * authoritative `platformUserId`. Callers need it for the retry that follows, + * and they need the merged `extraConfig` so their own later + * `mergeAccountExtraConfig(account.extraConfig, ...)` writes do not put the + * pre-login copy back and undo what was just persisted. + */ +type AutoReloginResult = { + accessToken: string; + platformUserId?: number; + extraConfig?: string; +}; + +async function tryAutoRelogin(account: any, site: any): Promise { const adapter = getAdapter(site.platform); if (!adapter) return null; @@ -108,16 +123,37 @@ async function tryAutoRelogin(account: any, site: any): Promise { ); if (!result.success || !result.accessToken) return null; + // Re-read after the network request: account settings may have changed while + // login was in flight, and merging into the original snapshot would overwrite + // those newer fields when the whole extraConfig value is persisted. + const latestAccount = result.platformUserId + ? await db.select({ extraConfig: schema.accounts.extraConfig }) + .from(schema.accounts) + .where(eq(schema.accounts.id, account.id)) + .get() + : undefined; + const reloginExtraConfig = result.platformUserId + ? mergeAccountExtraConfig( + latestAccount ? latestAccount.extraConfig : account.extraConfig, + { platformUserId: result.platformUserId }, + ) + : undefined; + await db.update(schema.accounts) .set({ accessToken: result.accessToken, + ...(reloginExtraConfig ? { extraConfig: reloginExtraConfig } : {}), updatedAt: new Date().toISOString(), status: account.status === 'expired' ? 'active' : account.status, }) .where(eq(schema.accounts.id, account.id)) .run(); - return result.accessToken; + return { + accessToken: result.accessToken, + platformUserId: result.platformUserId, + extraConfig: reloginExtraConfig, + }; } export async function checkinAccount(accountId: number, options?: { skipEvent?: boolean; scheduleMode?: 'cron' | 'interval' }) { @@ -175,7 +211,7 @@ export async function checkinAccount(accountId: number, options?: { skipEvent?: const guessedPlatformUserId = storedPlatformUserId ? undefined : guessPlatformUserIdFromUsername(account.username); - const platformUserId = resolvePlatformUserId(account.extraConfig, account.username); + let platformUserId = resolvePlatformUserId(account.extraConfig, account.username); const accountProxyUrl = resolveProxyUrlFromExtraConfig(account.extraConfig); let activeAccessToken = account.accessToken; @@ -183,9 +219,15 @@ export async function checkinAccount(accountId: number, options?: { skipEvent?: () => adapter.checkin(site.url, activeAccessToken, platformUserId)); if (!result.success && shouldAttemptAutoRelogin(result.message)) { - const refreshedAccessToken = await tryAutoRelogin(account, site); - if (refreshedAccessToken) { - activeAccessToken = refreshedAccessToken; + const relogin = await tryAutoRelogin(account, site); + if (relogin) { + activeAccessToken = relogin.accessToken; + // Adopt whatever the re-login reported before retrying, and refresh our + // in-memory copy of extraConfig — the merge writes further down start from + // `account.extraConfig`, so leaving the stale copy here would overwrite the + // id tryAutoRelogin() just persisted. + if (relogin.platformUserId) platformUserId = relogin.platformUserId; + if (relogin.extraConfig) account.extraConfig = relogin.extraConfig; result = await withAccountProxyOverride(accountProxyUrl, () => adapter.checkin(site.url, activeAccessToken, platformUserId)); } @@ -224,7 +266,13 @@ export async function checkinAccount(accountId: number, options?: { skipEvent?: if (shouldAdvanceLastCheckinAt) { updates.lastCheckinAt = new Date().toISOString(); } - if (!storedPlatformUserId && guessedPlatformUserId) { + // Both ids above were computed before any re-login. If a re-login has since + // reported the authoritative one, `account.extraConfig` already carries it, + // and persisting the guess here would replace a known-good id with digits + // scraped off the end of the username. Check the current config, not the + // pre-login snapshot. (`guessedPlatformUserId` is only set when + // `storedPlatformUserId` was empty, so this stays equivalent otherwise.) + if (guessedPlatformUserId && !getPlatformUserIdFromExtraConfig(account.extraConfig)) { updates.extraConfig = mergeAccountExtraConfig(account.extraConfig, { platformUserId: guessedPlatformUserId, }); diff --git a/src/server/services/platforms/base.ts b/src/server/services/platforms/base.ts index 6b87351..dcd639b 100644 --- a/src/server/services/platforms/base.ts +++ b/src/server/services/platforms/base.ts @@ -37,11 +37,18 @@ export interface BalanceInfo { subscriptionSummary?: SubscriptionSummary; } -interface LoginResult { +export interface LoginResult { success: boolean; accessToken?: string; username?: string; message?: string; + /** + * Site-side user id (the value of the `New-Api-User` header). + * Most New API compatible sites return it in the login payload as + * `data.id`, so adapters can surface it here instead of forcing callers + * to guess it from the username or re-discover it on every request. + */ + platformUserId?: number; } export interface UserInfo { @@ -156,6 +163,42 @@ export abstract class BasePlatformAdapter implements PlatformAdapter { } } + /** + * Pull the site-side user id out of the login payload. + * + * New API compatible sites return `{ success: true, data: { id, ... } }` on + * `/api/user/login`, so the id is already there at login time. Without it, + * callers fall back to `guessPlatformUserIdFromUsername()`, which only works + * when the username happens to end with the id. + * + * This lives on the base adapter so every subclass benefits. Veloera, for + * one, inherits `login()` as-is, yet its `authHeaders()` only sends + * `Veloera-User` / `New-API-User` / `User-id` when an id was resolved — so + * without this it hits the same e-mail-login failure. + */ + protected extractLoginUserId(payload: any): number | undefined { + const candidates: unknown[] = [ + payload?.data?.id, + payload?.data?.user?.id, + payload?.user?.id, + payload?.id, + ]; + for (const candidate of candidates) { + if (typeof candidate === 'number') { + if (Number.isSafeInteger(candidate) && candidate > 0) return candidate; + continue; + } + // Only accept a string that is *entirely* digits. `Number.parseInt` would + // happily turn "80305abc" or "80305.9" into 80305 and we would then send a + // wrong `New-Api-User` header. + if (typeof candidate === 'string' && /^\d+$/.test(candidate.trim())) { + const value = Number(candidate.trim()); + if (Number.isSafeInteger(value) && value > 0) return value; + } + } + return undefined; + } + async login(baseUrl: string, username: string, password: string): Promise { try { const res = await this.fetchJson(`${baseUrl}/api/user/login`, { @@ -167,6 +210,7 @@ export abstract class BasePlatformAdapter implements PlatformAdapter { success: true, accessToken: typeof res.data === 'string' ? res.data : res.data.token || res.data.access_token, username, + platformUserId: this.extractLoginUserId(res), }; } return { success: false, message: res?.message || '登录失败' }; diff --git a/src/server/services/platforms/newApi.ts b/src/server/services/platforms/newApi.ts index d3d82db..076dc69 100644 --- a/src/server/services/platforms/newApi.ts +++ b/src/server/services/platforms/newApi.ts @@ -1,4 +1,4 @@ -import { ApiTokenInfo, BasePlatformAdapter, CheckinResult, BalanceInfo, UserInfo, TokenVerifyResult, CreateApiTokenOptions, type SiteAnnouncement } from './base.js'; +import { ApiTokenInfo, BasePlatformAdapter, CheckinResult, BalanceInfo, UserInfo, TokenVerifyResult, CreateApiTokenOptions, type SiteAnnouncement, type LoginResult } from './base.js'; import type { RequestInit as UndiciRequestInit } from 'undici'; import { createContext, runInContext } from 'node:vm'; import { withSiteProxyRequestInit } from '../siteProxy.js'; @@ -946,7 +946,7 @@ export class NewApiAdapter extends BasePlatformAdapter { baseUrl: string, username: string, password: string, - ): Promise<{ success: boolean; accessToken?: string; username?: string; message?: string }> { + ): Promise { try { const { data: res, cookieHeader } = await this.fetchJsonRawWithCookie(`${baseUrl}/api/user/login`, { method: 'POST', @@ -960,11 +960,13 @@ export class NewApiAdapter extends BasePlatformAdapter { } const accessToken = this.extractLoginAccessToken(res); + const platformUserId = this.extractLoginUserId(res); if (res?.success && accessToken) { return { success: true, accessToken, username, + platformUserId, }; } if (res?.success && this.hasUsableSessionCookie(cookieHeader)) { @@ -972,6 +974,7 @@ export class NewApiAdapter extends BasePlatformAdapter { success: true, accessToken: cookieHeader, username, + platformUserId, }; }