From 716dd1de12c5761c4ab6bae613fd6fc125099a30 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 03:29:58 +0000 Subject: [PATCH 1/3] Initial plan From e16b19867a8bb5cc3e7ef9c0cb5668e4962df32b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 03:53:07 +0000 Subject: [PATCH 2/3] Add Corppass PAR (Pushed Authorization Request) feature with feature flag --- shared/constants/feature-flags.ts | 1 + src/app/config/features/spcp-myinfo.config.ts | 7 + src/app/modules/core/core.errors.ts | 1 + .../public-form/public-form.controller.ts | 17 +- .../spcp/__tests__/spcp.oidc.client.spec.ts | 212 ++++++++++++++++++ .../spcp/__tests__/spcp.test.constants.ts | 13 ++ .../modules/spcp/spcp.oidc.client.errors.ts | 9 + src/app/modules/spcp/spcp.oidc.client.ts | 109 ++++++++- .../modules/spcp/spcp.oidc.service/index.ts | 1 + .../spcp.oidc.service/spcp.oidc.service.cp.ts | 32 +++ 10 files changed, 398 insertions(+), 4 deletions(-) diff --git a/shared/constants/feature-flags.ts b/shared/constants/feature-flags.ts index 732ab38d10..3e6421c4cd 100644 --- a/shared/constants/feature-flags.ts +++ b/shared/constants/feature-flags.ts @@ -28,4 +28,5 @@ export const featureFlags = { singpassMrf: 'singpass-mrf' as const, enableSaveDraftButtonFloating: 'enable-save-draft-button-floating' as const, enableSaveDraftButtonHeader: 'enable-save-draft-button-header' as const, + enableCorppassPar: 'enable-corppass-par' as const, } diff --git a/src/app/config/features/spcp-myinfo.config.ts b/src/app/config/features/spcp-myinfo.config.ts index d0403bd174..e1b11c733b 100644 --- a/src/app/config/features/spcp-myinfo.config.ts +++ b/src/app/config/features/spcp-myinfo.config.ts @@ -29,6 +29,7 @@ type ISpcpConfig = { spOidcRpJwksSecretPath: string cpOidcNdiDiscoveryEndpoint: string cpOidcNdiJwksEndpoint: string + cpOidcNdiParEndpoint: string cpOidcRpClientId: string cpOidcRpRedirectUrl: string cpOidcRpJwksPublic: string @@ -215,6 +216,12 @@ const spcpMyInfoSchema: Schema = { default: null, env: 'CP_OIDC_NDI_JWKS_ENDPOINT', }, + cpOidcNdiParEndpoint: { + doc: "NDI's Corppass OIDC PAR (Pushed Authorization Request) Endpoint", + format: String, + default: '', + env: 'CP_OIDC_NDI_PAR_ENDPOINT', + }, cpOidcRpClientId: { doc: "The Relying Party's Corppass Client ID as registered with NDI", format: String, diff --git a/src/app/modules/core/core.errors.ts b/src/app/modules/core/core.errors.ts index 9fbf44c864..55d8f54480 100644 --- a/src/app/modules/core/core.errors.ts +++ b/src/app/modules/core/core.errors.ts @@ -127,6 +127,7 @@ export enum ErrorCodes { SPCP_OIDC_MISSING_ID_TOKEN = 110207, SPCP_OIDC_INVALID_VERIFICATION_KEY = 110208, SPCP_OIDC_EXCHANGE_AUTH_TOKEN = 110209, + SPCP_OIDC_CREATE_PAR_REQUEST = 110210, // [110300 - 110399] MyInfo Errors (/modules/myinfo) MYINFO_CIRCUIT_BREAKER = 110300, MYINFO_FETCH = 110301, diff --git a/src/app/modules/form/public-form/public-form.controller.ts b/src/app/modules/form/public-form/public-form.controller.ts index 7e665df71f..07872bea4e 100644 --- a/src/app/modules/form/public-form/public-form.controller.ts +++ b/src/app/modules/form/public-form/public-form.controller.ts @@ -54,6 +54,7 @@ import { SgidService } from '../../sgid/sgid.service' import { validateSgidForm } from '../../sgid/sgid.util' import { InvalidJwtError, VerifyJwtError } from '../../spcp/spcp.errors' import { getOidcService } from '../../spcp/spcp.oidc.service' +import { CpOidcServiceClass } from '../../spcp/spcp.oidc.service/spcp.oidc.service.cp' import { getRedirectTargetSpcpOidc, validateSpcpForm, @@ -701,10 +702,20 @@ export const _handleFormAuthRedirect: ControllerHandler< isPersistentLogin, encodedQuery, ) - return getOidcService(FormAuthType.CP).createRedirectUrl( - target, - form.esrvcId, + const cpOidcService = getOidcService( + FormAuthType.CP, + ) as CpOidcServiceClass + // Use PAR-based redirect when the feature flag is enabled + const useCorppassPar = req.growthbook?.isOn( + featureFlags.enableCorppassPar, ) + if (useCorppassPar) { + return cpOidcService.createRedirectUrlWithPar( + target, + form.esrvcId, + ) + } + return cpOidcService.createRedirectUrl(target, form.esrvcId) }) } case FormAuthType.SGID: diff --git a/src/app/modules/spcp/__tests__/spcp.oidc.client.spec.ts b/src/app/modules/spcp/__tests__/spcp.oidc.client.spec.ts index 6e9c7696ab..902cbf219b 100644 --- a/src/app/modules/spcp/__tests__/spcp.oidc.client.spec.ts +++ b/src/app/modules/spcp/__tests__/spcp.oidc.client.spec.ts @@ -13,6 +13,7 @@ import { SpcpOidcBaseClientCache } from '../spcp.oidc.client.cache' import { CreateAuthorisationUrlError, CreateJwtError, + CreateParRequestError, ExchangeAuthTokenError, GetDecryptionKeyError, GetSigningKeyError, @@ -32,9 +33,11 @@ import { import { CP_OIDC_NDI_DISCOVERY_ENDPOINT, CP_OIDC_NDI_JWKS_ENDPOINT, + CP_OIDC_NDI_PAR_ENDPOINT, CP_OIDC_RP_CLIENT_ID, CP_OIDC_RP_REDIRECT_URL, cpOidcClientConfig, + cpOidcClientConfigWithPar, SP_OIDC_NDI_DISCOVERY_ENDPOINT, SP_OIDC_NDI_JWKS_ENDPOINT, SP_OIDC_RP_CLIENT_ID, @@ -2203,6 +2206,215 @@ describe('CpOidcClient', () => { }) }) + describe('createAuthorisationUrlWithPar', () => { + it('should throw CreateAuthorisationUrlError if state parameter is empty', async () => { + // Arrange + jest + .spyOn(SpcpOidcBaseClientCache.prototype, 'refresh') + .mockResolvedValueOnce('ok' as unknown as Refresh) + + const MOCK_EMPTY_STATE = '' + const MOCK_ESRVCID = 'esrvcId' + + // Act + + const cpOidcClient = new CpOidcClient(cpOidcClientConfigWithPar) + const tryCreateUrl = cpOidcClient.createAuthorisationUrlWithPar( + MOCK_EMPTY_STATE, + MOCK_ESRVCID, + ) + + // Assert + await expect(tryCreateUrl).rejects.toThrow(CreateAuthorisationUrlError) + await expect(tryCreateUrl).rejects.toThrow('Empty state') + }) + + it('should throw CreateAuthorisationUrlError if esrvcId parameter is empty', async () => { + // Arrange + jest + .spyOn(SpcpOidcBaseClientCache.prototype, 'refresh') + .mockResolvedValueOnce('ok' as unknown as Refresh) + + const MOCK_STATE = 'state' + const MOCK_EMPTY_ESRVCID = '' + + // Act + + const cpOidcClient = new CpOidcClient(cpOidcClientConfigWithPar) + const tryCreateUrl = cpOidcClient.createAuthorisationUrlWithPar( + MOCK_STATE, + MOCK_EMPTY_ESRVCID, + ) + + // Assert + await expect(tryCreateUrl).rejects.toThrow(CreateAuthorisationUrlError) + await expect(tryCreateUrl).rejects.toThrow('Empty esrvcId') + }) + + it('should throw CreateParRequestError if PAR endpoint is not configured', async () => { + // Arrange + jest + .spyOn(SpcpOidcBaseClientCache.prototype, 'refresh') + .mockResolvedValueOnce('ok' as unknown as Refresh) + + const MOCK_STATE = 'state' + const MOCK_ESRVCID = 'esrvcId' + + // Act + // Use the config without PAR endpoint + const cpOidcClient = new CpOidcClient(cpOidcClientConfig) + const tryCreateUrl = cpOidcClient.createAuthorisationUrlWithPar( + MOCK_STATE, + MOCK_ESRVCID, + ) + + // Assert + await expect(tryCreateUrl).rejects.toThrow(CreateParRequestError) + await expect(tryCreateUrl).rejects.toThrow('PAR endpoint not configured') + }) + + it('should correctly POST to PAR endpoint and return authorisation URL with request_uri', async () => { + // Arrange + jest + .spyOn(SpcpOidcBaseClientCache.prototype, 'refresh') + .mockResolvedValueOnce('ok' as unknown as Refresh) + + const MOCK_STATE = 'state' + const MOCK_ESRVCID = 'esrvcId' + const MOCK_REQUEST_URI = 'urn:ietf:params:oauth:request_uri:12345' + const MOCK_AUTH_ENDPOINT = 'https://corppass.example.com/authorize' + + jest + .spyOn(CpOidcClient.prototype, 'getBaseClientFromCache') + .mockResolvedValueOnce({ + issuer: { + metadata: { + authorization_endpoint: MOCK_AUTH_ENDPOINT, + issuer: 'https://corppass.example.com', + }, + }, + } as unknown as BaseClient) + + jest + .spyOn(CpOidcClient.prototype, 'createJWT') + .mockResolvedValueOnce('mockJwt') + + const axiosSpy = jest.spyOn(axios, 'post').mockResolvedValueOnce({ + data: { + request_uri: MOCK_REQUEST_URI, + expires_in: 90, + }, + }) + + // Act + const cpOidcClient = new CpOidcClient(cpOidcClientConfigWithPar) + const result = await cpOidcClient.createAuthorisationUrlWithPar( + MOCK_STATE, + MOCK_ESRVCID, + ) + + // Assert + expect(axiosSpy).toHaveBeenCalledOnce() + expect(axiosSpy).toHaveBeenCalledWith( + CP_OIDC_NDI_PAR_ENDPOINT, + expect.stringContaining('esrvcID'), + expect.objectContaining({ + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + }), + ) + expect(result).toContain(MOCK_AUTH_ENDPOINT) + expect(result).toContain('request_uri=') + expect(result).toContain(encodeURIComponent(MOCK_REQUEST_URI)) + }) + + it('should throw CreateParRequestError if PAR response is missing request_uri', async () => { + // Arrange + jest + .spyOn(SpcpOidcBaseClientCache.prototype, 'refresh') + .mockResolvedValueOnce('ok' as unknown as Refresh) + + const MOCK_STATE = 'state' + const MOCK_ESRVCID = 'esrvcId' + + jest + .spyOn(CpOidcClient.prototype, 'getBaseClientFromCache') + .mockResolvedValueOnce({ + issuer: { + metadata: { + authorization_endpoint: 'https://corppass.example.com/authorize', + issuer: 'https://corppass.example.com', + }, + }, + } as unknown as BaseClient) + + jest + .spyOn(CpOidcClient.prototype, 'createJWT') + .mockResolvedValueOnce('mockJwt') + + jest.spyOn(axios, 'post').mockResolvedValueOnce({ + data: { + expires_in: 90, + // Missing request_uri + }, + }) + + // Act + const cpOidcClient = new CpOidcClient(cpOidcClientConfigWithPar) + const tryCreateUrl = cpOidcClient.createAuthorisationUrlWithPar( + MOCK_STATE, + MOCK_ESRVCID, + ) + + // Assert + await expect(tryCreateUrl).rejects.toThrow(CreateParRequestError) + await expect(tryCreateUrl).rejects.toThrow( + 'PAR response missing request_uri', + ) + }) + + it('should throw CreateParRequestError if PAR request fails', async () => { + // Arrange + jest + .spyOn(SpcpOidcBaseClientCache.prototype, 'refresh') + .mockResolvedValueOnce('ok' as unknown as Refresh) + + const MOCK_STATE = 'state' + const MOCK_ESRVCID = 'esrvcId' + + jest + .spyOn(CpOidcClient.prototype, 'getBaseClientFromCache') + .mockResolvedValueOnce({ + issuer: { + metadata: { + authorization_endpoint: 'https://corppass.example.com/authorize', + issuer: 'https://corppass.example.com', + }, + }, + } as unknown as BaseClient) + + jest + .spyOn(CpOidcClient.prototype, 'createJWT') + .mockResolvedValueOnce('mockJwt') + + jest + .spyOn(axios, 'post') + .mockRejectedValueOnce(new Error('Network error')) + + // Act + const cpOidcClient = new CpOidcClient(cpOidcClientConfigWithPar) + const tryCreateUrl = cpOidcClient.createAuthorisationUrlWithPar( + MOCK_STATE, + MOCK_ESRVCID, + ) + + // Assert + await expect(tryCreateUrl).rejects.toThrow(CreateParRequestError) + await expect(tryCreateUrl).rejects.toThrow('PAR request failed') + }) + }) + describe('getDecryptionKey', () => { it('should return GetDecryptionKeyError if jwe is empty', () => { // Arrange diff --git a/src/app/modules/spcp/__tests__/spcp.test.constants.ts b/src/app/modules/spcp/__tests__/spcp.test.constants.ts index 5768fd237f..6e94513af4 100644 --- a/src/app/modules/spcp/__tests__/spcp.test.constants.ts +++ b/src/app/modules/spcp/__tests__/spcp.test.constants.ts @@ -148,6 +148,7 @@ export const SP_OIDC_RP_REDIRECT_URL = 'spOidcRpRedirectUrl' export const CP_OIDC_NDI_DISCOVERY_ENDPOINT = 'cpOidcNdiDiscoveryEndpoint' export const CP_OIDC_NDI_JWKS_ENDPOINT = 'cpOidcNdiJwksEndpoint' +export const CP_OIDC_NDI_PAR_ENDPOINT = 'https://corppass.example.com/par' export const CP_OIDC_RP_CLIENT_ID = 'cpOidcRpClientId' export const CP_OIDC_RP_REDIRECT_URL = 'cpOidcRpRedirectUrl' @@ -198,3 +199,15 @@ export const cpOidcClientConfig: SpcpOidcClientConstructorParams = { rpSecretJwks: TEST_CP_RP_SECRET_JWKS, rpPublicJwks: TEST_CP_RP_PUBLIC_JWKS, } + +export const cpOidcClientConfigWithPar: SpcpOidcClientConstructorParams & { + parEndpoint: string +} = { + ndiDiscoveryEndpoint: CP_OIDC_NDI_DISCOVERY_ENDPOINT, + ndiJwksEndpoint: CP_OIDC_NDI_JWKS_ENDPOINT, + rpClientId: CP_OIDC_RP_CLIENT_ID, + rpRedirectUrl: CP_OIDC_RP_REDIRECT_URL, + rpSecretJwks: TEST_CP_RP_SECRET_JWKS, + rpPublicJwks: TEST_CP_RP_PUBLIC_JWKS, + parEndpoint: CP_OIDC_NDI_PAR_ENDPOINT, +} diff --git a/src/app/modules/spcp/spcp.oidc.client.errors.ts b/src/app/modules/spcp/spcp.oidc.client.errors.ts index 9b78be43c8..02a15c0888 100644 --- a/src/app/modules/spcp/spcp.oidc.client.errors.ts +++ b/src/app/modules/spcp/spcp.oidc.client.errors.ts @@ -9,6 +9,15 @@ export class CreateAuthorisationUrlError extends ApplicationError { } } +/** + * Error while creating Pushed Authorisation Request + */ +export class CreateParRequestError extends ApplicationError { + constructor(message = 'Error while creating Pushed Authorisation Request') { + super(message, undefined, ErrorCodes.SPCP_OIDC_CREATE_PAR_REQUEST) + } +} + /** * Failed to create JWT */ diff --git a/src/app/modules/spcp/spcp.oidc.client.ts b/src/app/modules/spcp/spcp.oidc.client.ts index 45c67a544f..eefefe0c85 100644 --- a/src/app/modules/spcp/spcp.oidc.client.ts +++ b/src/app/modules/spcp/spcp.oidc.client.ts @@ -19,6 +19,7 @@ import { SpcpOidcBaseClientCache } from './spcp.oidc.client.cache' import { CreateAuthorisationUrlError, CreateJwtError, + CreateParRequestError, ExchangeAuthTokenError, GetDecryptionKeyError, GetSigningKeyError, @@ -63,6 +64,14 @@ export abstract class SpcpOidcBaseClient { */ _spcpOidcBaseClientCache: SpcpOidcBaseClientCache + /** + * Protected getter for the redirect URL + * @returns RP redirect URL + */ + protected get rpRedirectUrl(): string { + return this.#rpRedirectUrl + } + /** * Constructor for client * @param config @@ -512,8 +521,106 @@ export class CpOidcClient extends SpcpOidcBaseClient { authType = FormAuthType.CP eServiceIdKey = 'esrvcID' - constructor(params: SpcpOidcClientConstructorParams) { + #parEndpoint: string + + constructor( + params: SpcpOidcClientConstructorParams & { parEndpoint?: string }, + ) { super(params) + this.#parEndpoint = params.parEndpoint || '' + } + + /** + * Method to generate url to CP login page for authorisation using Pushed Authorization Request (PAR) + * POSTs authorization parameters to PAR endpoint and returns a URL with request_uri + * @param state - contains formId, remember me, and stored queryId + * @param esrvcId - eServiceId + * @return authorisation url with request_uri from PAR response + * @throws CreateParRequestError if PAR request fails + * @throws CreateAuthorisationUrlError if state or esrvcId is undefined + */ + async createAuthorisationUrlWithPar( + state: string, + esrvcId: string, + ): Promise { + if (!state) { + throw new CreateAuthorisationUrlError( + 'Empty state, failed to create redirect url.', + ) + } + if (!esrvcId) { + throw new CreateAuthorisationUrlError( + 'Empty esrvcId, failed to create redirect url.', + ) + } + if (!this.#parEndpoint) { + throw new CreateParRequestError( + 'PAR endpoint not configured, failed to create redirect url.', + ) + } + + const baseClient = await this.getBaseClientFromCache() + const nonce = ulid() + + // Create client assertion for PAR request + const clientAssertion = await this.createJWT( + { + iss: this.rpClientId, + aud: baseClient.issuer.metadata.issuer, + sub: this.rpClientId, + }, + '60s', + ) + + // Construct PAR request body + const parBody = new URLSearchParams({ + scope: 'openid', + response_type: 'code', + client_id: this.rpClientId, + redirect_uri: this.rpRedirectUrl, + state: state, + nonce: nonce, + [this.eServiceIdKey]: esrvcId, + client_assertion_type: + 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer', + client_assertion: clientAssertion, + }).toString() + + try { + const { data } = await axios.post<{ + request_uri: string + expires_in: number + }>(this.#parEndpoint, parBody, { + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + }) + + if (!data.request_uri) { + throw new CreateParRequestError('PAR response missing request_uri') + } + + // Construct authorization URL with request_uri from PAR response + const authorizationEndpoint = + baseClient.issuer.metadata.authorization_endpoint + + if (!authorizationEndpoint) { + throw new CreateParRequestError( + 'Authorization endpoint not found in issuer metadata', + ) + } + + const authorisationUrl = `${authorizationEndpoint}?client_id=${encodeURIComponent(this.rpClientId)}&request_uri=${encodeURIComponent(data.request_uri)}` + + return authorisationUrl + } catch (err) { + if (err instanceof CreateParRequestError) { + throw err + } + throw new CreateParRequestError( + `PAR request failed: ${err instanceof Error ? err.message : 'Unknown error'}`, + ) + } } /** diff --git a/src/app/modules/spcp/spcp.oidc.service/index.ts b/src/app/modules/spcp/spcp.oidc.service/index.ts index 16e1867874..c4302b8008 100644 --- a/src/app/modules/spcp/spcp.oidc.service/index.ts +++ b/src/app/modules/spcp/spcp.oidc.service/index.ts @@ -41,6 +41,7 @@ const cpOidcClient = new CpOidcClient({ preIacFilePath: spcpMyInfoConfig.cpOidcRpJwksSecretPath, postIacJsonString: spcpMyInfoConfig.cpOidcRpJwksSecret, }), + parEndpoint: spcpMyInfoConfig.cpOidcNdiParEndpoint, }) const cpOidcProps = { diff --git a/src/app/modules/spcp/spcp.oidc.service/spcp.oidc.service.cp.ts b/src/app/modules/spcp/spcp.oidc.service/spcp.oidc.service.cp.ts index 9a26ac810e..21eef981b3 100644 --- a/src/app/modules/spcp/spcp.oidc.service/spcp.oidc.service.cp.ts +++ b/src/app/modules/spcp/spcp.oidc.service/spcp.oidc.service.cp.ts @@ -3,6 +3,7 @@ import { err, errAsync, ok, okAsync, Result, ResultAsync } from 'neverthrow' import { FormAuthType } from '../../../../../shared/types' import { createLoggerWithLabel } from '../../../config/logger' import { + CreateRedirectUrlError, ExchangeAuthTokenError, InvalidIdTokenError, InvalidJwtError, @@ -42,6 +43,37 @@ export class CpOidcServiceClass extends SpcpOidcServiceClass { this.oidcProps = oidcProps } + /** + * Create the URL to which the client should be redirected for Corppass OIDC login using PAR. + * @param state - contains formId, remember me, and stored queryId + * @param esrvcId CP OIDC e-service ID + * @returns okAsync(redirectUrl) + * @returns errAsync(CreateRedirectUrlError) + */ + createRedirectUrlWithPar( + state: string, + esrvcId: string, + ): ResultAsync { + const logMeta = { + action: 'createRedirectUrlWithPar', + state, + esrvcId, + authType: this.authType, + } + + return ResultAsync.fromPromise( + this.oidcClient.createAuthorisationUrlWithPar(state, esrvcId), + (error) => { + logger.error({ + message: 'Error while creating redirect URL with PAR', + meta: logMeta, + error, + }) + return new CreateRedirectUrlError() + }, + ) + } + /** * Verifies a Corppass JWT and extracts its payload. * @param jwt The contents of the JWT cookie From 7894f871f7123f37b69a811c36569ba7df2a1df3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 03:56:52 +0000 Subject: [PATCH 3/3] Address code review feedback - use URLSearchParams and fix error types --- src/app/modules/spcp/spcp.oidc.client.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/app/modules/spcp/spcp.oidc.client.ts b/src/app/modules/spcp/spcp.oidc.client.ts index eefefe0c85..db19c0b336 100644 --- a/src/app/modules/spcp/spcp.oidc.client.ts +++ b/src/app/modules/spcp/spcp.oidc.client.ts @@ -605,16 +605,24 @@ export class CpOidcClient extends SpcpOidcBaseClient { baseClient.issuer.metadata.authorization_endpoint if (!authorizationEndpoint) { - throw new CreateParRequestError( + throw new CreateAuthorisationUrlError( 'Authorization endpoint not found in issuer metadata', ) } - const authorisationUrl = `${authorizationEndpoint}?client_id=${encodeURIComponent(this.rpClientId)}&request_uri=${encodeURIComponent(data.request_uri)}` + const authorisationUrlParams = new URLSearchParams({ + client_id: this.rpClientId, + request_uri: data.request_uri, + }) + + const authorisationUrl = `${authorizationEndpoint}?${authorisationUrlParams.toString()}` return authorisationUrl } catch (err) { - if (err instanceof CreateParRequestError) { + if ( + err instanceof CreateParRequestError || + err instanceof CreateAuthorisationUrlError + ) { throw err } throw new CreateParRequestError(