chore(core): Add browser id on the oidc issue cookie (#18642)

This commit is contained in:
Guillaume Jacquart
2025-08-22 09:30:48 +02:00
committed by GitHub
parent ca8629ef30
commit cf337de83d
2 changed files with 16 additions and 11 deletions

View File

@@ -2,12 +2,13 @@ import type { User } from '@n8n/db';
import { type Request, type Response } from 'express'; import { type Request, type Response } from 'express';
import { mock } from 'jest-mock-extended'; import { mock } from 'jest-mock-extended';
import type { AuthService } from '@/auth/auth.service';
import type { UrlService } from '@/services/url.service';
import type { OidcService } from '../../oidc.service.ee'; import type { OidcService } from '../../oidc.service.ee';
import { OidcController } from '../oidc.controller.ee'; import { OidcController } from '../oidc.controller.ee';
import type { AuthService } from '@/auth/auth.service';
import type { AuthlessRequest } from '@/requests';
import type { UrlService } from '@/services/url.service';
const authService = mock<AuthService>(); const authService = mock<AuthService>();
const oidcService = mock<OidcService>(); const oidcService = mock<OidcService>();
const urlService = mock<UrlService>(); const urlService = mock<UrlService>();
@@ -33,8 +34,9 @@ describe('OidcController', () => {
describe('callbackHandler', () => { describe('callbackHandler', () => {
test('Should issue cookie with MFA flag set to true on successful OIDC login', async () => { test('Should issue cookie with MFA flag set to true on successful OIDC login', async () => {
const req = mock<Request>({ const req = mock<AuthlessRequest>({
originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value', originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value',
browserId: 'browser-id-123',
}); });
const res = mock<Response>(); const res = mock<Response>();
@@ -51,16 +53,17 @@ describe('OidcController', () => {
expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl); expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl);
// Verify that issueCookie was called with MFA flag set to true // Verify that issueCookie was called with MFA flag set to true
expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true); expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, req.browserId);
// Verify redirect to home page // Verify redirect to home page
expect(res.redirect).toHaveBeenCalledWith('/'); expect(res.redirect).toHaveBeenCalledWith('/');
}); });
test('Should handle callback URL with different query parameters', async () => { test('Should handle callback URL with different query parameters', async () => {
const req = mock<Request>({ const req = mock<AuthlessRequest>({
originalUrl: originalUrl:
'/sso/oidc/callback?code=different_code&state=different_state&session_state=session123', '/sso/oidc/callback?code=different_code&state=different_state&session_state=session123',
browserId: 'browser-id-123',
}); });
const res = mock<Response>(); const res = mock<Response>();
@@ -73,13 +76,14 @@ describe('OidcController', () => {
await controller.callbackHandler(req, res); await controller.callbackHandler(req, res);
expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl); expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl);
expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true); expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, req.browserId);
expect(res.redirect).toHaveBeenCalledWith('/'); expect(res.redirect).toHaveBeenCalledWith('/');
}); });
test('Should handle callback URL with no query parameters', async () => { test('Should handle callback URL with no query parameters', async () => {
const req = mock<Request>({ const req = mock<AuthlessRequest>({
originalUrl: '/sso/oidc/callback', originalUrl: '/sso/oidc/callback',
browserId: undefined,
}); });
const res = mock<Response>(); const res = mock<Response>();
@@ -90,7 +94,7 @@ describe('OidcController', () => {
await controller.callbackHandler(req, res); await controller.callbackHandler(req, res);
expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl); expect(oidcService.loginUser).toHaveBeenCalledWith(expectedCallbackUrl);
expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true); expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, undefined);
expect(res.redirect).toHaveBeenCalledWith('/'); expect(res.redirect).toHaveBeenCalledWith('/');
}); });

View File

@@ -8,6 +8,7 @@ import { UrlService } from '@/services/url.service';
import { OIDC_CLIENT_SECRET_REDACTED_VALUE } from '../constants'; import { OIDC_CLIENT_SECRET_REDACTED_VALUE } from '../constants';
import { OidcService } from '../oidc.service.ee'; import { OidcService } from '../oidc.service.ee';
import { AuthlessRequest } from '@/requests';
@RestController('/sso/oidc') @RestController('/sso/oidc')
export class OidcController { export class OidcController {
@@ -51,13 +52,13 @@ export class OidcController {
@Get('/callback', { skipAuth: true }) @Get('/callback', { skipAuth: true })
@Licensed('feat:oidc') @Licensed('feat:oidc')
async callbackHandler(req: Request, res: Response) { async callbackHandler(req: AuthlessRequest, res: Response) {
const fullUrl = `${this.urlService.getInstanceBaseUrl()}${req.originalUrl}`; const fullUrl = `${this.urlService.getInstanceBaseUrl()}${req.originalUrl}`;
const callbackUrl = new URL(fullUrl); const callbackUrl = new URL(fullUrl);
const user = await this.oidcService.loginUser(callbackUrl); const user = await this.oidcService.loginUser(callbackUrl);
this.authService.issueCookie(res, user, true); this.authService.issueCookie(res, user, true, req.browserId);
res.redirect('/'); res.redirect('/');
} }