Skip to content

Commit 1261cda

Browse files
os-zhuangclaude
andauthored
fix(plugin-auth): the issuer is the provider's to declare — Google links stop resolving under a synthesized one (#4552)
better-auth 1.7 keys every account on `(issuer, providerAccountId)`, and `resolveOAuthAccountKey` takes the issuer the PROVIDER declares, falling back to the synthetic `local:oauth:<id>` only for a provider that declares none. The boot backfill synthesized that value for EVERY social provider, so a Google link was stamped `local:oauth:google` while sign-in looked it up under `https://accounts.google.com`. The row went invisible: `findAccountOwnerByKey` missed it, the callback fell through to "link this provider", and the insert collided with that very row on the `(provider_id, account_id)` unique index — reaching the user as `?error=unable_to_link_account` on an account that had signed in with Google before. Seven of better-auth's built-in providers declare their own issuer (google, apple, facebook, cognito, line, paybin, entra), so all seven were mis-stamped; google is the one wired on the hosted control plane. - Read the issuer off the providers better-auth INSTANTIATED (`authContext.socialProviders`) instead of deriving it from configured ids. A provider whose `accountIssuer` is a function resolves it per login (Entra reads the tenant `iss` off the profile) — that stays underivable at boot and is reported, not guessed. - Repair, not just stamp: correcting the derivation does nothing for a database already stamped wrong, so the pass re-stamps rows carrying the synthetic issuer for a provider that declares a real one. Only that one wrong-by-construction value is rewritten. - New parity gate gets its assertion from better-auth's own factory list, so a bump that gives another provider an issuer of its own turns red instead of silently mis-stamping that provider's users on the next boot. Verified to catch this defect: reverting the derivation reddens it, naming all six fixed-issuer providers. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 4b6cac7 commit 1261cda

4 files changed

Lines changed: 445 additions & 23 deletions

File tree

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
socialProviderList,
6+
socialProviders as socialProviderFactories,
7+
} from '@better-auth/core/social-providers';
8+
import { backfillAccountIssuer, oauthIssuerFor } from './backfill-account-issuer.js';
9+
10+
/**
11+
* Account-issuer parity gate.
12+
*
13+
* better-auth 1.7 keys every account on `(issuer, providerAccountId)`, and the
14+
* issuer is the PROVIDER's to declare: `resolveOAuthAccountKey` takes
15+
* `provider.accountIssuer` when there is one and synthesizes
16+
* `local:oauth:<id>` only when there is not. A boot-time backfill that stamps a
17+
* different value than sign-in looks the row up under does not merely fail to
18+
* help — it hides the account and parks it on the `(provider_id, account_id)`
19+
* unique slot the correct row needs, which is how a working Google login turned
20+
* into `?error=unable_to_link_account`.
21+
*
22+
* So this gate asserts the two agree, provider by provider, against better-auth's
23+
* OWN factory list. A dependency bump that gives another provider an issuer of
24+
* its own (or takes one away) turns this red instead of silently mis-stamping
25+
* that provider's users on the next boot.
26+
*/
27+
28+
/** Enough options to construct every factory; unused keys are ignored. */
29+
const PROBE_OPTIONS = {
30+
clientId: 'probe-client-id',
31+
clientSecret: 'probe-client-secret',
32+
tenantId: 'probe-tenant',
33+
region: 'us-east-1',
34+
userPoolId: 'pool-1',
35+
domain: 'probe.example.com',
36+
issuer: 'https://probe.example.com',
37+
} as const;
38+
39+
interface ProbedProvider {
40+
id: string;
41+
accountIssuer: unknown;
42+
}
43+
44+
function instantiateAll(): ProbedProvider[] {
45+
const probed: ProbedProvider[] = [];
46+
const broken: string[] = [];
47+
for (const id of socialProviderList) {
48+
const factory = (socialProviderFactories as any)[id];
49+
try {
50+
const provider = factory(PROBE_OPTIONS as any);
51+
probed.push({ id: provider.id, accountIssuer: provider.accountIssuer });
52+
} catch (e) {
53+
broken.push(`${id}: ${(e as Error)?.message}`);
54+
}
55+
}
56+
// A factory this probe can no longer construct is a provider the gate stops
57+
// covering — fail loudly rather than shrink the checked set in silence.
58+
expect(broken, 'social provider factories the parity probe could not construct').toEqual([]);
59+
return probed;
60+
}
61+
62+
/** What better-auth will key an account by, per `resolveOAuthAccountKey`. */
63+
function issuerBetterAuthWillUse(provider: ProbedProvider): string | 'per-login' {
64+
const declared = provider.accountIssuer;
65+
if (declared === undefined) return oauthIssuerFor(provider.id);
66+
if (typeof declared === 'string') return declared;
67+
return 'per-login';
68+
}
69+
70+
/** Minimal ObjectQL stand-in — one account row, `update` patches in place. */
71+
function makeQl(row: Record<string, any>) {
72+
const tables: Record<string, any[]> = { sys_account: [row] };
73+
return {
74+
tables,
75+
update: async (object: string, data: any) => {
76+
const target = (tables[object] ?? []).find((r) => r.id === data.id);
77+
if (target) Object.assign(target, data);
78+
return target;
79+
},
80+
find: async (object: string, query: any) => {
81+
const where = query?.where ?? {};
82+
return (tables[object] ?? []).filter((r) =>
83+
Object.entries(where).every(([field, value]) =>
84+
value === null ? r[field] == null : r[field] === value,
85+
),
86+
);
87+
},
88+
};
89+
}
90+
91+
describe('account issuer parity — the backfill stamps what sign-in looks up', () => {
92+
it('covers every social provider better-auth ships', () => {
93+
const probed = instantiateAll();
94+
expect(probed.length).toBe(socialProviderList.length);
95+
});
96+
97+
it('agrees with better-auth on the issuer for every provider that has a fixed one', async () => {
98+
const mismatches: Array<{ provider: string; betterAuth: string; backfill: string | null }> = [];
99+
100+
for (const provider of instantiateAll()) {
101+
const expected = issuerBetterAuthWillUse(provider);
102+
if (expected === 'per-login') continue;
103+
104+
const ql = makeQl({
105+
id: 'a1',
106+
provider_id: provider.id,
107+
account_id: 'subject-1',
108+
issuer: null,
109+
});
110+
await backfillAccountIssuer(ql, { socialProviders: [provider] });
111+
112+
const stamped = ql.tables.sys_account[0].issuer ?? null;
113+
if (stamped !== expected) {
114+
mismatches.push({ provider: provider.id, betterAuth: expected, backfill: stamped });
115+
}
116+
}
117+
118+
expect(mismatches, 'providers whose backfilled issuer differs from the one sign-in resolves').toEqual([]);
119+
});
120+
121+
it('never invents an issuer for a provider that resolves one per login', async () => {
122+
for (const provider of instantiateAll()) {
123+
if (issuerBetterAuthWillUse(provider) !== 'per-login') continue;
124+
125+
const ql = makeQl({
126+
id: 'a1',
127+
provider_id: provider.id,
128+
account_id: 'subject-1',
129+
issuer: null,
130+
});
131+
const res = await backfillAccountIssuer(ql, { socialProviders: [provider] });
132+
133+
expect(ql.tables.sys_account[0].issuer, `${provider.id} must stay unstamped`).toBeNull();
134+
expect(res.unresolved).toEqual([{ providerId: provider.id, count: 1 }]);
135+
}
136+
});
137+
138+
it('repairs a synthetic stamp on every provider that declares a real issuer', async () => {
139+
const declaring = instantiateAll().filter((p) => typeof p.accountIssuer === 'string');
140+
// Guards the guard: if better-auth ever ships none of these, the repair
141+
// case above proves nothing and this gate has quietly stopped testing.
142+
expect(declaring.length).toBeGreaterThan(0);
143+
144+
for (const provider of declaring) {
145+
const ql = makeQl({
146+
id: 'a1',
147+
provider_id: provider.id,
148+
account_id: 'subject-1',
149+
issuer: oauthIssuerFor(provider.id),
150+
});
151+
const res = await backfillAccountIssuer(ql, { socialProviders: [provider] });
152+
153+
expect(res.repaired, `${provider.id} mis-stamp should be repaired`).toBe(1);
154+
expect(ql.tables.sys_account[0].issuer).toBe(provider.accountIssuer);
155+
}
156+
});
157+
158+
it('google — the provider this defect shipped on — resolves to its real issuer', () => {
159+
const google = (socialProviderFactories as any).google(PROBE_OPTIONS as any);
160+
expect(google.accountIssuer).toBe('https://accounts.google.com');
161+
expect(oauthIssuerFor('google')).toBe('local:oauth:google');
162+
});
163+
});

packages/plugins/plugin-auth/src/auth-plugin.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
type AuthManagerOptions,
2828
} from './auth-manager.js';
2929
import { ensureDefaultOrganization } from './ensure-default-organization.js';
30+
import type { ResolvedSocialProvider } from './backfill-account-issuer.js';
3031
import { createTenancyService, type TenancyService } from './tenancy-service.js';
3132
import { backfillMemberships, type MembershipPolicy } from './reconcile-membership.js';
3233
import {
@@ -261,6 +262,35 @@ export class AuthPlugin implements Plugin {
261262
}
262263
}
263264

265+
/**
266+
* The social providers better-auth built for this runtime, straight off its
267+
* own context — each one carrying the `accountIssuer` it will key its
268+
* accounts by. Read rather than reconstructed: reconstructing it from the
269+
* configured ids is exactly the guess that mis-stamped Google links.
270+
*
271+
* Returns `undefined` when the instance cannot be reached (auth not built
272+
* yet, or a host-supplied instance that exposes no context) — the backfill
273+
* then falls back to the id-derived issuers and reports what it cannot
274+
* resolve, which is the pre-existing behaviour, not a new failure.
275+
*/
276+
private async resolveInstantiatedSocialProviders(
277+
ctx: PluginContext,
278+
): Promise<ResolvedSocialProvider[] | undefined> {
279+
try {
280+
const auth = await this.authManager?.getAuthInstance();
281+
const context = await (auth as any)?.$context;
282+
const providers = context?.socialProviders;
283+
if (!Array.isArray(providers)) return undefined;
284+
return providers.filter((p: any) => typeof p?.id === 'string' && p.id);
285+
} catch (e) {
286+
ctx.logger.warn?.(
287+
'[auth] could not read better-auth\'s instantiated social providers — account issuers fall back to the id-derived values',
288+
{ error: (e as Error)?.message },
289+
);
290+
return undefined;
291+
}
292+
}
293+
264294
async init(ctx: PluginContext): Promise<void> {
265295
ctx.logger.info('Initializing Auth Plugin...');
266296

@@ -684,14 +714,21 @@ export class AuthPlugin implements Plugin {
684714
// better-auth 1.7 resolves every account by (issuer, providerAccountId).
685715
// Rows written before the upgrade have no issuer and are therefore
686716
// invisible to sign-in, so stamp them once at boot. Idempotent: a database
687-
// whose rows already carry an issuer costs one empty query.
717+
// whose rows already carry the right issuer costs one empty query.
718+
//
719+
// The providers are handed over as better-auth INSTANTIATED them, because
720+
// the issuer is theirs to declare and only they know it — Google names
721+
// `https://accounts.google.com`, GitHub names nothing and takes the
722+
// synthetic fallback. Deriving it from the configured ids instead is what
723+
// stamped Google links with a value sign-in never looks them up under.
688724
ctx.hook('kernel:ready', async () => {
689725
try {
690726
const ql = ctx.getService<IDataEngine>('objectql');
691727
if (!ql) return;
692728
const { backfillAccountIssuer } = await import('./backfill-account-issuer.js');
693729
await backfillAccountIssuer(ql, {
694730
logger: ctx.logger,
731+
socialProviders: await this.resolveInstantiatedSocialProviders(ctx),
695732
socialProviderIds: Object.keys(this.configuredSocialProviders ?? {}),
696733
oidcProviderIssuers: Object.fromEntries(
697734
(this.options.oidcProviders ?? [])

packages/plugins/plugin-auth/src/backfill-account-issuer.test.ts

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('backfillAccountIssuer (better-auth 1.7 account identity)', () => {
4848
expect(CREDENTIAL_ISSUER).toBe('local:credential');
4949
});
5050

51-
it('stamps configured social providers with the synthetic local:oauth issuer', async () => {
51+
it('stamps a provider that declares no issuer with the synthetic local:oauth one', async () => {
5252
const ql = makeQl({
5353
sys_account: [{ id: 'a1', provider_id: 'github', account_id: '4242', issuer: null }],
5454
});
@@ -60,6 +60,36 @@ describe('backfillAccountIssuer (better-auth 1.7 account identity)', () => {
6060
expect(oauthIssuerFor('github')).toBe('local:oauth:github');
6161
});
6262

63+
it('stamps a provider that declares its own issuer with THAT issuer', async () => {
64+
const ql = makeQl({
65+
sys_account: [{ id: 'a1', provider_id: 'google', account_id: 'sub-1', issuer: null }],
66+
});
67+
68+
const res = await backfillAccountIssuer(ql, {
69+
socialProviders: [{ id: 'google', accountIssuer: 'https://accounts.google.com' }],
70+
});
71+
72+
expect(res.stamped).toBe(1);
73+
expect(ql.tables.sys_account[0].issuer).toBe('https://accounts.google.com');
74+
});
75+
76+
it('leaves a per-login issuer underivable rather than synthesizing one', async () => {
77+
// Microsoft Entra reads the tenant's `iss` off each login's profile, so
78+
// there is no boot-time answer — and a guess would be the whole bug.
79+
const ql = makeQl({
80+
sys_account: [{ id: 'a1', provider_id: 'microsoft', account_id: 'oid-1', issuer: null }],
81+
});
82+
83+
const res = await backfillAccountIssuer(ql, {
84+
socialProviders: [{ id: 'microsoft', accountIssuer: ({ profile }: any) => profile.iss }],
85+
socialProviderIds: ['microsoft'],
86+
});
87+
88+
expect(res).toMatchObject({ scanned: 1, stamped: 0, repaired: 0 });
89+
expect(res.unresolved).toEqual([{ providerId: 'microsoft', count: 1 }]);
90+
expect(ql.tables.sys_account[0].issuer).toBeNull();
91+
});
92+
6393
it('uses the registered SSO provider\'s real issuer for federated accounts', async () => {
6494
const ql = makeQl({
6595
sys_account: [{ id: 'a1', provider_id: 'okta-prod', account_id: '00u1', issuer: null }],
@@ -145,3 +175,96 @@ describe('backfillAccountIssuer (better-auth 1.7 account identity)', () => {
145175
await expect(backfillAccountIssuer({} as any)).resolves.toMatchObject({ scanned: 0, stamped: 0 });
146176
});
147177
});
178+
179+
/**
180+
* The shipped defect: an earlier pass stamped `local:oauth:google` on links
181+
* better-auth resolves under `https://accounts.google.com`. The row went
182+
* invisible at sign-in, the callback fell through to "link this provider", and
183+
* the insert hit the `(provider_id, account_id)` unique index the invisible row
184+
* was holding — `?error=unable_to_link_account`.
185+
*/
186+
describe('backfillAccountIssuer — repairing a mis-stamped synthetic issuer', () => {
187+
it('re-stamps a synthetic issuer with the one its provider declares', async () => {
188+
const log = logger();
189+
const ql = makeQl({
190+
sys_account: [
191+
{ id: 'a1', provider_id: 'google', account_id: 'sub-1', issuer: 'local:oauth:google' },
192+
],
193+
});
194+
195+
const res = await backfillAccountIssuer(ql, {
196+
logger: log,
197+
socialProviders: [{ id: 'google', accountIssuer: 'https://accounts.google.com' }],
198+
});
199+
200+
expect(res).toMatchObject({ scanned: 1, stamped: 0, repaired: 1, unresolved: [] });
201+
expect(ql.tables.sys_account[0].issuer).toBe('https://accounts.google.com');
202+
expect(log.info).toHaveBeenCalled();
203+
});
204+
205+
it('is idempotent — the repaired row is not touched again', async () => {
206+
const ql = makeQl({
207+
sys_account: [
208+
{ id: 'a1', provider_id: 'google', account_id: 'sub-1', issuer: 'local:oauth:google' },
209+
],
210+
});
211+
const opts = { socialProviders: [{ id: 'google', accountIssuer: 'https://accounts.google.com' }] };
212+
213+
await backfillAccountIssuer(ql, opts);
214+
ql.update.mockClear();
215+
const second = await backfillAccountIssuer(ql, opts);
216+
217+
expect(second).toMatchObject({ scanned: 0, repaired: 0 });
218+
expect(ql.update).not.toHaveBeenCalled();
219+
});
220+
221+
it('leaves the synthetic issuer alone when it IS what the provider mints', async () => {
222+
const ql = makeQl({
223+
sys_account: [
224+
{ id: 'a1', provider_id: 'github', account_id: '4242', issuer: 'local:oauth:github' },
225+
],
226+
});
227+
228+
const res = await backfillAccountIssuer(ql, { socialProviders: [{ id: 'github' }] });
229+
230+
expect(res).toMatchObject({ scanned: 0, stamped: 0, repaired: 0 });
231+
expect(ql.tables.sys_account[0].issuer).toBe('local:oauth:github');
232+
});
233+
234+
it('rewrites ONLY the synthetic value — a row already holding a real issuer stands', async () => {
235+
const ql = makeQl({
236+
sys_account: [
237+
{ id: 'a1', provider_id: 'google', account_id: 'sub-1', issuer: 'https://accounts.google.com' },
238+
{ id: 'a2', provider_id: 'google', account_id: 'sub-2', issuer: 'https://some-other.example' },
239+
],
240+
});
241+
242+
const res = await backfillAccountIssuer(ql, {
243+
socialProviders: [{ id: 'google', accountIssuer: 'https://accounts.google.com' }],
244+
});
245+
246+
expect(res).toMatchObject({ scanned: 0, repaired: 0 });
247+
expect(ql.tables.sys_account[1].issuer).toBe('https://some-other.example');
248+
});
249+
250+
it('reports a repair the database refuses instead of counting it', async () => {
251+
// A duplicate row can already occupy (issuer, account_id) on a deployment
252+
// that acquired one before the unique index existed.
253+
const log = logger();
254+
const ql = makeQl({
255+
sys_account: [
256+
{ id: 'a1', provider_id: 'google', account_id: 'sub-1', issuer: 'local:oauth:google' },
257+
],
258+
});
259+
ql.update.mockRejectedValueOnce(new Error('unique constraint'));
260+
261+
const res = await backfillAccountIssuer(ql, {
262+
logger: log,
263+
socialProviders: [{ id: 'google', accountIssuer: 'https://accounts.google.com' }],
264+
});
265+
266+
expect(res).toMatchObject({ scanned: 1, repaired: 0 });
267+
expect(res.unresolved).toEqual([{ providerId: 'google', count: 1 }]);
268+
expect(log.warn).toHaveBeenCalled();
269+
});
270+
});

0 commit comments

Comments
 (0)