diff --git a/.changeset/oidc-grant-binding-per-client.md b/.changeset/oidc-grant-binding-per-client.md new file mode 100644 index 00000000..061c5e9d --- /dev/null +++ b/.changeset/oidc-grant-binding-per-client.md @@ -0,0 +1,5 @@ +--- +'seamless-auth-api': patch +--- + +Bind provider lookups and signatures to the grant for the asking client, even when one session has signed in to several clients. The check used to read one grant for the session and compare its client, so a session signed in to two apps could be refused for one of them. diff --git a/src/services/oidcPayloadStore.ts b/src/services/oidcPayloadStore.ts index 3cf8815a..da548d03 100644 --- a/src/services/oidcPayloadStore.ts +++ b/src/services/oidcPayloadStore.ts @@ -134,10 +134,13 @@ async function findOne( export async function hasOidcGrantForSession(sessionId: string, clientId: string) { if (!isUuid(sessionId)) return false; + // One first-party session can sign in to several clients, so the client is part of the + // query rather than checked on whichever grant comes back first. const grant = await OidcPayload.findOne({ where: { model: 'Grant', sessionId, + payload: { clientId }, expiresAt: { [Op.gt]: new Date() }, } as WhereOptions, }); diff --git a/tests/postgres/oidcProvider.spec.ts b/tests/postgres/oidcProvider.spec.ts index 9789ca9e..f952e114 100644 --- a/tests/postgres/oidcProvider.spec.ts +++ b/tests/postgres/oidcProvider.spec.ts @@ -136,6 +136,31 @@ describe('grant binding', () => { expect((await findGrantBoundSession(session.id, 'app'))?.id).toBe(session.id); expect(await findGrantBoundSession(session.id, 'other')).toBeNull(); }); + + it('binds every client a session signed in to, not just one of them', async () => { + const { User } = await import('../../src/models/users.js'); + const { Session } = await import('../../src/models/sessions.js'); + const user = await User.create({ email: `${randomUUID()}@example.com` } as never); + const session = await Session.create({ + userId: user.id, + mode: 'server', + expiresAt: new Date(Date.now() + 3_600_000), + idleExpiresAt: new Date(Date.now() + 3_600_000), + }); + + for (const clientId of ['first', 'second', 'third']) { + await upsertOidcPayload('Grant', randomUUID(), { + payload: { clientId, accountId: user.id }, + expiresIn: 600, + sessionId: session.id, + }); + } + + for (const clientId of ['first', 'second', 'third']) { + expect((await findGrantBoundSession(session.id, clientId))?.id).toBe(session.id); + } + expect(await findGrantBoundSession(session.id, 'other')).toBeNull(); + }); }); describe('oidc_clients', () => { diff --git a/tests/unit/services/oidcPayloadStore.spec.ts b/tests/unit/services/oidcPayloadStore.spec.ts index 83e3ee6c..4d8dcffe 100644 --- a/tests/unit/services/oidcPayloadStore.spec.ts +++ b/tests/unit/services/oidcPayloadStore.spec.ts @@ -158,7 +158,7 @@ describe('hasOidcGrantForSession', () => { vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({ payload: { clientId: 'app' } } as never); expect(await hasOidcGrantForSession(SESSION, 'app')).toBe(true); expect(vi.mocked(OidcPayload.findOne).mock.calls[0][0]).toMatchObject({ - where: { model: 'Grant', sessionId: SESSION }, + where: { model: 'Grant', sessionId: SESSION, payload: { clientId: 'app' } }, }); vi.mocked(OidcPayload.findOne).mockResolvedValueOnce({