fix(sso): forward RFC 9207 iss parameter in OIDC callback (#1785)

* fix(sso): forward RFC 9207 iss parameter in OIDC callback

The Custom OIDC callback only forwarded code and state from the query
string to the token exchange, silently dropping the iss parameter that
issuer-identification-aware providers (Keycloak 22+, and others) add
to the redirect. openid-client rejects the exchange as an invalid
response once discovery advertises support for that parameter, so
login failed for any such provider.

* fix(sso): forward RFC 9207 iss parameter in OIDC callback

The Custom OIDC callback only forwarded code and state from the query
string to the token exchange, silently dropping the iss parameter that
issuer-identification-aware providers add to the redirect. openid-client
rejects the exchange as an invalid response once discovery advertises
support for that parameter (confirmed on Keycloak 26), so login failed
for any such provider.

Also logs the underlying openid-client error cause on callback failure
instead of only the generic message it collapses specific validation
errors into, since that cause carries the actual diagnosis.

* refactor(sso): dedupe iss-forwarding test setup

Extracts the shared callback-with-stubbed-service setup used by both
new regression tests into one helper.
This commit is contained in:
Anso
2026-08-07 23:04:27 -04:00
committed by GitHub
parent 89b16b97d1
commit e084ad424c
3 changed files with 103 additions and 7 deletions
+81 -1
View File
@@ -1,4 +1,4 @@
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest';
import { describe, it, expect, beforeAll, afterAll, vi, type MockInstance } from 'vitest';
import { setupTestDb, cleanupTestDb, TEST_JWT_SECRET } from './helpers/setupTestDb';
import supertest from 'supertest';
import jwt from 'jsonwebtoken';
@@ -110,6 +110,86 @@ describe('SSO OIDC Callback', () => {
expect(res.status).toBe(302);
expect(res.headers.location).toContain('User');
});
// Drives a callback request with a valid state cookie, returning the stubbed
// handleOIDCCallback spy so the caller can assert on the params it received.
async function callbackWithStubbedService(provider: string, query: Record<string, string>): Promise<MockInstance> {
const { SSOService } = await import('../services/SSOService');
const { CryptoService } = await import('../services/CryptoService');
const stateCookie = CryptoService.getInstance().encrypt(JSON.stringify({
state: 'test-state',
codeVerifier: 'test-verifier',
provider,
}));
const spy = vi
.spyOn(SSOService.getInstance(), 'handleOIDCCallback')
.mockResolvedValue({ success: false, error: 'stubbed for iss-forwarding assertion' });
await supertest(app)
.get(`/api/auth/sso/oidc/${provider}/callback`)
.query(query)
.set('Cookie', `sencho_sso_state=${stateCookie}`);
return spy;
}
it('forwards the RFC 9207 iss query parameter to SSOService.handleOIDCCallback', async () => {
const spy = await callbackWithStubbedService('oidc_custom', {
code: 'test-code',
state: 'test-state',
iss: 'https://idp.example.com/realms/master',
});
expect(spy).toHaveBeenCalledWith(
'oidc_custom',
expect.any(String),
expect.objectContaining({ code: 'test-code', state: 'test-state', iss: 'https://idp.example.com/realms/master' }),
'test-state',
'test-verifier',
);
spy.mockRestore();
});
it('omits iss from the forwarded params when the provider does not send one', async () => {
const spy = await callbackWithStubbedService('oidc_google', { code: 'test-code', state: 'test-state' });
expect(spy).toHaveBeenCalledWith(
'oidc_google',
expect.any(String),
expect.objectContaining({ code: 'test-code', state: 'test-state', iss: undefined }),
'test-state',
'test-verifier',
);
spy.mockRestore();
});
});
describe('SSOService.buildTokenExchangeUrl', () => {
it('sets the iss query parameter when provided', async () => {
const { SSOService } = await import('../services/SSOService');
const url = SSOService.getInstance().buildTokenExchangeUrl(
'http://sencho.example.com/api/auth/sso/oidc/oidc_custom/callback',
{ code: 'test-code', state: 'test-state', iss: 'https://idp.example.com/realms/master' },
);
expect(url.searchParams.get('code')).toBe('test-code');
expect(url.searchParams.get('state')).toBe('test-state');
expect(url.searchParams.get('iss')).toBe('https://idp.example.com/realms/master');
});
it('omits the iss query parameter when not provided', async () => {
const { SSOService } = await import('../services/SSOService');
const url = SSOService.getInstance().buildTokenExchangeUrl(
'http://sencho.example.com/api/auth/sso/oidc/oidc_google/callback',
{ code: 'test-code', state: 'test-state' },
);
expect(url.searchParams.get('code')).toBe('test-code');
expect(url.searchParams.get('state')).toBe('test-state');
expect(url.searchParams.has('iss')).toBe(false);
});
});
describe('SSO User Provisioning', () => {