diff --git a/charts/ccd-admin-web/Chart.yaml b/charts/ccd-admin-web/Chart.yaml index 28b3521cc..222f9aed5 100644 --- a/charts/ccd-admin-web/Chart.yaml +++ b/charts/ccd-admin-web/Chart.yaml @@ -2,7 +2,7 @@ description: Helm chart for the HMCTS CCD Admin Web name: ccd-admin-web apiVersion: v2 home: https://github.com/hmcts/ccd-admin-web -version: 2.2.15 +version: 2.2.16 maintainers: - name: HMCTS CCD Dev Team email: ccd-devops@HMCTS.NET diff --git a/charts/ccd-admin-web/values.aat.template.yaml b/charts/ccd-admin-web/values.aat.template.yaml index fdcf5f66d..217052e86 100644 --- a/charts/ccd-admin-web/values.aat.template.yaml +++ b/charts/ccd-admin-web/values.aat.template.yaml @@ -1,3 +1,5 @@ nodejs: image: ${IMAGE_NAME} - ingressHost: ${SERVICE_FQDN} \ No newline at end of file + ingressHost: ${SERVICE_FQDN} + environment: + IDAM_OAUTH2_REDIRECT_URI_ALLOWLIST: "${SERVICE_FQDN}" \ No newline at end of file diff --git a/charts/ccd-admin-web/values.preview.template.yaml b/charts/ccd-admin-web/values.preview.template.yaml index 6d155517a..407f14a70 100644 --- a/charts/ccd-admin-web/values.preview.template.yaml +++ b/charts/ccd-admin-web/values.preview.template.yaml @@ -2,6 +2,7 @@ nodejs: image: ${IMAGE_NAME} ingressHost: ${SERVICE_FQDN} environment: + IDAM_OAUTH2_REDIRECT_URI_ALLOWLIST: "${SERVICE_FQDN}" ADMINWEB_LOGIN_URL: https://idam-web-public.aat.platform.hmcts.net ADMINWEB_IMPORT_URL: http://ccd-definition-store-api-aat.service.core-compute-aat.internal/import ADMINWEB_JURISDICTIONS_URL: http://ccd-definition-store-api-aat.service.core-compute-aat.internal/api/data/jurisdictions diff --git a/charts/ccd-admin-web/values.yaml b/charts/ccd-admin-web/values.yaml index 159761f42..ca0f36025 100644 --- a/charts/ccd-admin-web/values.yaml +++ b/charts/ccd-admin-web/values.yaml @@ -30,6 +30,7 @@ nodejs: IDAM_BASE_URL: https://idam-api.{{ .Values.global.environment }}.platform.hmcts.net IDAM_OAUTH2_TOKEN_ENDPOINT: https://idam-api.{{ .Values.global.environment }}.platform.hmcts.net/oauth2/token IDAM_OAUTH2_LOGOUT_ENDPOINT: https://idam-api.{{ .Values.global.environment }}.platform.hmcts.net/session/:token + IDAM_OAUTH2_REDIRECT_URI_ALLOWLIST: "ccd-admin-web.{{ .Values.global.environment }}.platform.hmcts.net" ADMINWEB_LOGIN_URL: "https://idam-web-public.{{ .Values.global.environment }}.platform.hmcts.net/login" ADMINWEB_IMPORT_URL: "http://ccd-definition-store-api-{{ .Values.global.environment }}.service.core-compute-{{ .Values.global.environment }}.internal/import" diff --git a/config/custom-environment-variables.yaml b/config/custom-environment-variables.yaml index 2b4dcae30..8ba14283d 100644 --- a/config/custom-environment-variables.yaml +++ b/config/custom-environment-variables.yaml @@ -14,6 +14,7 @@ idam: token_endpoint: IDAM_OAUTH2_TOKEN_ENDPOINT logout_endpoint: IDAM_OAUTH2_LOGOUT_ENDPOINT client_id: IDAM_OAUTH2_CLIENT_ID + redirect_uri_allowlist: IDAM_OAUTH2_REDIRECT_URI_ALLOWLIST adminWeb: login_url: ADMINWEB_LOGIN_URL import_url: ADMINWEB_IMPORT_URL diff --git a/config/default.yaml b/config/default.yaml index 8c11b47e7..f5fba68a7 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -23,6 +23,7 @@ idam: token_endpoint: http://localhost:5000/oauth2/token logout_endpoint: http://localhost:5000/session/:token client_id: ccd_admin + redirect_uri_allowlist: "localhost,127.0.0.1" adminWeb: login_url: https://localhost:3501/login import_url: http://localhost:4451/import diff --git a/src/main/oauth2/access-token-request.ts b/src/main/oauth2/access-token-request.ts index 1a1060ba6..30ccbd01a 100644 --- a/src/main/oauth2/access-token-request.ts +++ b/src/main/oauth2/access-token-request.ts @@ -1,13 +1,31 @@ import * as fetch from "node-fetch"; -import { format } from "url"; +import { format, URL } from "url"; import { get } from "config"; import { Logger } from "@hmcts/nodejs-logging"; +const ERROR_INVALID_REDIRECT_URI = { + code: "INVALID_REDIRECT_URI", + error: "Bad Request", + message: "Redirect URI is not permitted", + status: 400, +}; + const completeRedirectURI = (uri) => { - if (!uri.startsWith("http")) { - return `https://${uri}`; + let parsedUrl; + try { + const fullUri = uri.startsWith("http") ? uri : `https://${uri}`; + parsedUrl = new URL(fullUri); + } catch (e) { + const error = Object.assign({}, ERROR_INVALID_REDIRECT_URI, { cause: e }); + throw error; + } + const allowedHosts = (get("idam.oauth2.redirect_uri_allowlist") as string) + .split(",") + .map((h) => h.trim()); + if (!allowedHosts.includes(parsedUrl.hostname)) { + throw ERROR_INVALID_REDIRECT_URI; } - return uri; + return parsedUrl.href; }; export function accessTokenRequest(request) { diff --git a/src/test/oauth2/access-token-request.spec.ts b/src/test/oauth2/access-token-request.spec.ts index 93d0bf542..5ef4cfaba 100644 --- a/src/test/oauth2/access-token-request.spec.ts +++ b/src/test/oauth2/access-token-request.spec.ts @@ -15,7 +15,10 @@ describe("Access Token Request", () => { const TOKEN_ENDPOINT = "http://localhost:1234/oauth2/token"; const REDIRECT_URN = "localhost/redirect/to"; const REDIRECT_URL = "https://localhost/redirect/to"; + const REDIRECT_URL_WITH_PORT = "https://localhost:3501/redirect/to"; + const DISALLOWED_URI = "https://attacker.com/steal"; const AUTH_CODE = "xyz789"; + const REDIRECT_ALLOWLIST = "localhost,127.0.0.1"; const REQUEST = sinonExpressMock.mockReq({ query: { @@ -29,6 +32,12 @@ describe("Access Token Request", () => { redirect_uri: REDIRECT_URL, }, }); + const REQUEST_WITH_PORT = sinonExpressMock.mockReq({ + query: { + code: AUTH_CODE, + redirect_uri: REDIRECT_URL_WITH_PORT, + }, + }); const RESPONSE = { body: { access_token: "q1w2e3r4t5y6", @@ -46,6 +55,7 @@ describe("Access Token Request", () => { config = { get: sinon.stub(), }; + config.get.withArgs("idam.oauth2.redirect_uri_allowlist").returns(REDIRECT_ALLOWLIST); fetch = fetchMock.sandbox().post(`begin:${TOKEN_ENDPOINT}`, RESPONSE); accessTokenRequest = proxyquire("../../main/oauth2/access-token-request", { @@ -89,4 +99,36 @@ describe("Access Token Request", () => { }) .catch((error) => done(new Error(error))); }); + + it("should allow redirect URIs with an allowed hostname and port", (done) => { + config.get.withArgs("idam.oauth2.client_id").returns(CLIENT_ID); + config.get.withArgs("secrets.ccd.ccd-admin-web-oauth2-client-secret").returns(CLIENT_SECRET); + config.get.withArgs("idam.oauth2.token_endpoint").returns(TOKEN_ENDPOINT); + + accessTokenRequest(REQUEST_WITH_PORT) + .then(() => { + expect(fetch.called()).to.be.true; + const requestedUrl = url.parse(fetch.lastUrl(), true); + expect(requestedUrl.query.redirect_uri).to.equal(REDIRECT_URL_WITH_PORT); + done(); + }) + .catch((error) => done(new Error(error))); + }); + + it("should reject redirect URIs with disallowed hosts", async () => { + config.get.withArgs("idam.oauth2.client_id").returns(CLIENT_ID); + config.get.withArgs("secrets.ccd.ccd-admin-web-oauth2-client-secret").returns(CLIENT_SECRET); + config.get.withArgs("idam.oauth2.token_endpoint").returns(TOKEN_ENDPOINT); + const REQUEST_DISALLOWED = sinonExpressMock.mockReq({ + query: { code: AUTH_CODE, redirect_uri: DISALLOWED_URI }, + }); + try { + await accessTokenRequest(REQUEST_DISALLOWED); + expect.fail("Expected error to be thrown"); + } catch (error) { + expect(error.status).to.equal(400); + expect(error.error).to.equal("Bad Request"); + expect(error.message).to.equal("Redirect URI is not permitted"); + } + }); });