Skip to content

Commit 0692ba7

Browse files
authored
🐛 fix: fix oidc accountId mismatch (lobehub#10058)
* chore: adjust oidc login and consent page mobile style * fix: acccount mismatch error * test: add oidc service test case
1 parent 39d91a8 commit 0692ba7

File tree

4 files changed

+256
-4
lines changed

4 files changed

+256
-4
lines changed

src/app/[variants]/oauth/consent/[uid]/Consent/index.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,9 @@ const useStyles = createStyles(({ css, token }) => ({
4242
background-color: transparent;
4343
`,
4444
card: css`
45-
width: 100%;
4645
max-width: 500px;
4746
border-color: ${token.colorBorderSecondary};
4847
border-radius: 12px;
49-
5048
background-color: ${token.colorBgContainer};
5149
`,
5250
connector: css`

src/app/[variants]/oauth/consent/[uid]/Login.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ const useStyles = createStyles(({ css, token, responsive }) => ({
2929
font-weight: 500;
3030
`,
3131
card: css`
32-
width: 100%;
3332
max-width: 500px;
3433
border-color: ${token.colorBorderSecondary};
3534
border-radius: 12px;
36-
3735
background: ${token.colorBgContainer};
3836
3937
${responsive.mobile} {
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
import { createContextForInteractionDetails } from '@/libs/oidc-provider/http-adapter';
4+
5+
import { OIDCService } from '.';
6+
import { getOIDCProvider } from './oidcProvider';
7+
8+
vi.mock('@/libs/oidc-provider/http-adapter', () => ({
9+
createContextForInteractionDetails: vi.fn(),
10+
}));
11+
12+
vi.mock('./oidcProvider', () => ({
13+
getOIDCProvider: vi.fn(),
14+
}));
15+
16+
const createMockProvider = () => {
17+
const grantCtor = Object.assign(
18+
vi.fn().mockImplementation((payload) => ({
19+
...payload,
20+
destroy: vi.fn(),
21+
})),
22+
{ find: vi.fn() },
23+
);
24+
25+
return {
26+
interactionDetails: vi.fn(),
27+
interactionResult: vi.fn(),
28+
interactionFinished: vi.fn(),
29+
Grant: grantCtor,
30+
Client: {
31+
find: vi.fn(),
32+
},
33+
};
34+
};
35+
36+
describe('OIDCService', () => {
37+
beforeEach(() => {
38+
vi.clearAllMocks();
39+
});
40+
41+
it('initialize should resolve provider and return a working service', async () => {
42+
const provider = createMockProvider();
43+
provider.Client.find.mockResolvedValue({ metadata: () => ({ scope: 'openid' }) });
44+
vi.mocked(getOIDCProvider).mockResolvedValue(provider as any);
45+
46+
const service = await OIDCService.initialize();
47+
await service.getClientMetadata('client-1');
48+
49+
expect(getOIDCProvider).toHaveBeenCalledTimes(1);
50+
expect(provider.Client.find).toHaveBeenCalledWith('client-1');
51+
});
52+
53+
it('getInteractionDetails should delegate to provider with context request/response', async () => {
54+
const provider = createMockProvider();
55+
provider.interactionDetails.mockResolvedValue({ prompt: 'login' });
56+
vi.mocked(createContextForInteractionDetails).mockResolvedValue({
57+
req: { id: 'req' },
58+
res: { id: 'res' },
59+
} as any);
60+
61+
const service = new OIDCService(provider as any);
62+
const result = await service.getInteractionDetails('uid-1');
63+
64+
expect(createContextForInteractionDetails).toHaveBeenCalledWith('uid-1');
65+
expect(provider.interactionDetails).toHaveBeenCalledWith({ id: 'req' }, { id: 'res' });
66+
expect(result).toEqual({ prompt: 'login' });
67+
});
68+
69+
it('getInteractionResult should return provider interaction result', async () => {
70+
const provider = createMockProvider();
71+
provider.interactionResult.mockResolvedValue({ ok: true });
72+
vi.mocked(createContextForInteractionDetails).mockResolvedValue({
73+
req: { id: 'req' },
74+
res: { id: 'res' },
75+
} as any);
76+
77+
const service = new OIDCService(provider as any);
78+
const payload = { login: true };
79+
const result = await service.getInteractionResult('uid-2', payload);
80+
81+
expect(provider.interactionResult).toHaveBeenCalledWith({ id: 'req' }, { id: 'res' }, payload);
82+
expect(result).toEqual({ ok: true });
83+
});
84+
85+
it('finishInteraction should call provider with merge option', async () => {
86+
const provider = createMockProvider();
87+
provider.interactionFinished.mockResolvedValue(undefined);
88+
vi.mocked(createContextForInteractionDetails).mockResolvedValue({
89+
req: { id: 'req' },
90+
res: { id: 'res' },
91+
} as any);
92+
93+
const service = new OIDCService(provider as any);
94+
const payload = { consent: true };
95+
await service.finishInteraction('uid-3', payload);
96+
97+
expect(provider.interactionFinished).toHaveBeenCalledWith(
98+
{ id: 'req' },
99+
{ id: 'res' },
100+
payload,
101+
{ mergeWithLastSubmission: true },
102+
);
103+
});
104+
105+
it('findOrCreateGrants should reuse existing matching grant', async () => {
106+
const provider = createMockProvider();
107+
const existingGrant = { accountId: 'account-1', clientId: 'client-1' };
108+
provider.Grant.find.mockResolvedValue(existingGrant as any);
109+
110+
const service = new OIDCService(provider as any);
111+
const grant = await service.findOrCreateGrants('account-1', 'client-1', 'grant-1');
112+
113+
expect(provider.Grant.find).toHaveBeenCalledWith('grant-1');
114+
expect(provider.Grant).not.toHaveBeenCalled();
115+
expect(grant).toBe(existingGrant);
116+
});
117+
118+
it('findOrCreateGrants should destroy mismatched grant and create a new one', async () => {
119+
const provider = createMockProvider();
120+
const staleGrant = {
121+
accountId: 'other-account',
122+
clientId: 'client-1',
123+
destroy: vi.fn().mockResolvedValue(undefined),
124+
};
125+
provider.Grant.find.mockResolvedValue(staleGrant as any);
126+
127+
const createdGrant = { accountId: 'account-2', clientId: 'client-1' };
128+
provider.Grant.mockImplementation(() => createdGrant as any);
129+
130+
const service = new OIDCService(provider as any);
131+
const grant = await service.findOrCreateGrants('account-2', 'client-1', 'grant-2');
132+
133+
expect(staleGrant.destroy).toHaveBeenCalledTimes(1);
134+
expect(provider.Grant).toHaveBeenCalledWith({ accountId: 'account-2', clientId: 'client-1' });
135+
expect(grant).toBe(createdGrant);
136+
});
137+
138+
it('findOrCreateGrants should create new grant when no existing id is provided', async () => {
139+
const provider = createMockProvider();
140+
const createdGrant = { accountId: 'account-3', clientId: 'client-3' };
141+
provider.Grant.mockImplementation(() => createdGrant as any);
142+
143+
const service = new OIDCService(provider as any);
144+
const grant = await service.findOrCreateGrants('account-3', 'client-3');
145+
146+
expect(provider.Grant.find).not.toHaveBeenCalled();
147+
expect(provider.Grant).toHaveBeenCalledWith({ accountId: 'account-3', clientId: 'client-3' });
148+
expect(grant).toBe(createdGrant);
149+
});
150+
151+
it('findOrCreateGrants should create new grant if provided id is missing in storage', async () => {
152+
const provider = createMockProvider();
153+
provider.Grant.find.mockResolvedValue(undefined);
154+
const createdGrant = { accountId: 'account-4', clientId: 'client-4' };
155+
provider.Grant.mockImplementation(() => createdGrant as any);
156+
157+
const service = new OIDCService(provider as any);
158+
const grant = await service.findOrCreateGrants('account-4', 'client-4', 'grant-missing');
159+
160+
expect(provider.Grant.find).toHaveBeenCalledWith('grant-missing');
161+
expect(provider.Grant).toHaveBeenCalledWith({ accountId: 'account-4', clientId: 'client-4' });
162+
expect(grant).toBe(createdGrant);
163+
});
164+
165+
it('findOrCreateGrants should recreate grant if client mismatch occurs', async () => {
166+
const provider = createMockProvider();
167+
const staleGrant = {
168+
accountId: 'account-5',
169+
clientId: 'other-client',
170+
destroy: vi.fn().mockResolvedValue(undefined),
171+
};
172+
provider.Grant.find.mockResolvedValue(staleGrant as any);
173+
174+
const createdGrant = { accountId: 'account-5', clientId: 'client-5' };
175+
provider.Grant.mockImplementation(() => createdGrant as any);
176+
177+
const service = new OIDCService(provider as any);
178+
const grant = await service.findOrCreateGrants(
179+
'account-5',
180+
'client-5',
181+
'grant-client-mismatch',
182+
);
183+
184+
expect(staleGrant.destroy).toHaveBeenCalledTimes(1);
185+
expect(provider.Grant).toHaveBeenCalledWith({ accountId: 'account-5', clientId: 'client-5' });
186+
expect(grant).toBe(createdGrant);
187+
});
188+
189+
it('findOrCreateGrants should continue when destroy throws during mismatch cleanup', async () => {
190+
const provider = createMockProvider();
191+
const staleGrant = {
192+
accountId: 'wrong-account',
193+
clientId: 'client-6',
194+
destroy: vi.fn().mockRejectedValue(new Error('destroy failed')),
195+
};
196+
provider.Grant.find.mockResolvedValue(staleGrant as any);
197+
198+
const createdGrant = { accountId: 'account-6', clientId: 'client-6' };
199+
provider.Grant.mockImplementation(() => createdGrant as any);
200+
201+
const service = new OIDCService(provider as any);
202+
const grant = await service.findOrCreateGrants('account-6', 'client-6', 'grant-error');
203+
204+
expect(staleGrant.destroy).toHaveBeenCalledTimes(1);
205+
expect(provider.Grant).toHaveBeenCalledWith({ accountId: 'account-6', clientId: 'client-6' });
206+
expect(grant).toBe(createdGrant);
207+
});
208+
209+
it('getClientMetadata should return metadata from client lookup', async () => {
210+
const provider = createMockProvider();
211+
provider.Client.find.mockResolvedValue({
212+
metadata: () => ({ redirect_uris: ['https://example.com/cb'] }),
213+
});
214+
215+
const service = new OIDCService(provider as any);
216+
const metadata = await service.getClientMetadata('client-5');
217+
218+
expect(provider.Client.find).toHaveBeenCalledWith('client-5');
219+
expect(metadata).toEqual({ redirect_uris: ['https://example.com/cb'] });
220+
});
221+
222+
it('getClientMetadata should return undefined when client is missing', async () => {
223+
const provider = createMockProvider();
224+
provider.Client.find.mockResolvedValue(undefined);
225+
226+
const service = new OIDCService(provider as any);
227+
const metadata = await service.getClientMetadata('client-missing');
228+
229+
expect(provider.Client.find).toHaveBeenCalledWith('client-missing');
230+
expect(metadata).toBeUndefined();
231+
});
232+
});

src/server/services/oidc/index.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,30 @@ export class OIDCService {
4141
// 如果之前的交互步骤已经关联了 Grant
4242
grant = await this.provider.Grant.find(existingGrantId);
4343
log('Found existing grantId: %s', existingGrantId);
44+
if (grant) {
45+
const accountMismatch = grant.accountId && grant.accountId !== accountId;
46+
const clientMismatch = grant.clientId && grant.clientId !== clientId;
47+
48+
if (accountMismatch || clientMismatch) {
49+
log(
50+
'Discarding stale grant %s due to mismatch (stored account=%s, client=%s; expected account=%s, client=%s)',
51+
existingGrantId,
52+
grant.accountId,
53+
grant.clientId,
54+
accountId,
55+
clientId,
56+
);
57+
try {
58+
await grant.destroy();
59+
log('Destroyed mismatched grant: %s', existingGrantId);
60+
} catch (error) {
61+
log('Failed to destroy mismatched grant %s: %O', existingGrantId, error);
62+
}
63+
grant = undefined;
64+
}
65+
} else {
66+
log('Existing grantId %s not found in storage, will create a new grant', existingGrantId);
67+
}
4468
}
4569

4670
if (!grant) {

0 commit comments

Comments
 (0)