Skip to content

Commit

Permalink
Merge pull request #1588 from govuk-one-login/AUT-2733/fix-server-sen…
Browse files Browse the repository at this point in the history 
…t-consent-cookie

AUT-2733: Fix server sent consent cookie
  • Loading branch information
@gtvj
gtvj authored May 7, 2024
2 parents 0e5065c + 08b5805 commit b47b1c8
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 79 deletions.
31 changes: 11 additions & 20 deletions src/components/authorize/authorize-controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import {
import { getNextPathAndUpdateJourney, ERROR_CODES } from "../common/constants";
import { BadRequestError, QueryParamsError } from "../../utils/error";
import { ExpressRouteFunc } from "../../types";
import {
CookieConsentModel,
CookieConsentServiceInterface,
} from "../common/cookie-consent/types";
import { CookieConsentServiceInterface } from "../common/cookie-consent/types";
import { cookieConsentService } from "../common/cookie-consent/cookie-consent-service";
import { sanitize } from "../../utils/strings";
import { USER_JOURNEY_EVENTS } from "../common/state-machine/state-machine";
Expand All @@ -31,21 +28,9 @@ import {
import { logger } from "../../utils/logger";
import { Claims } from "./claims-config";

function createConsentCookie(
res: Response,
consentCookieValue: CookieConsentModel
) {
res.cookie(COOKIES_PREFERENCES_SET, consentCookieValue.value, {
expires: consentCookieValue.expiry,
secure: true,
httpOnly: true,
domain: res.locals.analyticsCookieDomain,
});
}

export function authorizeGet(
authService: AuthorizeServiceInterface = authorizeService(),
cookieService: CookieConsentServiceInterface = cookieConsentService(),
cookiesConsentService: CookieConsentServiceInterface = cookieConsentService(),
kmsService: KmsDecryptionServiceInterface = new KmsDecryptionService(),
jwtService: JwtServiceInterface = new JwtService()
): ExpressRouteFunc {
Expand Down Expand Up @@ -164,9 +149,15 @@ export function authorizeGet(

if (req.session.client.cookieConsentEnabled && cookieConsent) {
const consentCookieValue =
cookieService.createConsentCookieValue(cookieConsent);

createConsentCookie(res, consentCookieValue);
cookiesConsentService.createConsentCookieValue(cookieConsent);

res.cookie(COOKIES_PREFERENCES_SET, consentCookieValue.value, {
expires: consentCookieValue.expires,
secure: true,
httpOnly: false,
domain: res.locals.analyticsCookieDomain,
encode: String,
});

if (
startAuthResponse.data.user.gaCrossDomainTrackingId &&
Expand Down
69 changes: 48 additions & 21 deletions src/components/authorize/tests/authorize-controller.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { authorizeGet } from "../authorize-controller";
import { CookieConsentServiceInterface } from "../../common/cookie-consent/types";
import {
COOKIE_CONSENT,
COOKIES_PREFERENCES_SET,
OIDC_PROMPT,
PATH_NAMES,
} from "../../../app.constants";
Expand All @@ -22,6 +23,7 @@ import { createmockclaims } from "./test-data";
import { Claims } from "../claims-config";
import { getOrchToAuthExpectedClientId } from "../../../config";
import { createMockRequest } from "../../../../test/helpers/mock-request-helper";
import { createMockCookieConsentService } from "../../../../test/helpers/mock-cookie-consent-service-helper";

describe("authorize controller", () => {
let req: RequestOutput;
Expand Down Expand Up @@ -55,11 +57,6 @@ describe("authorize controller", () => {
success: true,
});

fakeCookieConsentService = {
getCookieConsent: sinon.fake(),
createConsentCookieValue: sinon.fake(),
};

fakeKmsDecryptionService = {
decrypt: sinon.fake.returns(Promise.resolve("jwt")),
};
Expand Down Expand Up @@ -88,18 +85,24 @@ describe("authorize controller", () => {
});

it("should redirect to /sign-in-or-create page with cookie preferences set", async () => {
req.body.cookie_preferences = "true";

authServiceResponseData.data.user = {
cookieConsent: COOKIE_CONSENT.ACCEPT,
};

fakeAuthorizeService = mockAuthService(authServiceResponseData);

const fakeCookieConsentService: CookieConsentServiceInterface = {
getCookieConsent: sinon.fake(),
createConsentCookieValue: sinon.fake.returns({
value: JSON.stringify("cookieValue"),
expiry: "cookieExpires",
}),
} as unknown as CookieConsentServiceInterface;
const fakeCookieConsentService = createMockCookieConsentService(
req.body.cookie_preferences
);

const consentCookieValue =
fakeCookieConsentService.createConsentCookieValue(
req.body.cookie_preferences === "true"
? COOKIE_CONSENT.ACCEPT
: COOKIE_CONSENT.REJECT
);

await authorizeGet(
fakeAuthorizeService,
Expand All @@ -108,7 +111,20 @@ describe("authorize controller", () => {
fakeJwtService
)(req as Request, res as Response);

expect(res.cookie).to.have.been.called;
expect(res.cookie).to.have.been.calledWith(
COOKIES_PREFERENCES_SET,
consentCookieValue.value,
sinon.match({
expires: sinon.match((date: Date) => {
const expectedExpires = new Date(Date.now());
expectedExpires.setFullYear(expectedExpires.getFullYear() + 1);
return Math.abs(date.getTime() - expectedExpires.getTime()) < 1000;
}),
secure: true,
httpOnly: false,
encode: String,
})
);
expect(res.redirect).to.have.calledWith(PATH_NAMES.SIGN_IN_OR_CREATE);
});

Expand Down Expand Up @@ -270,13 +286,16 @@ describe("authorize controller", () => {
};
fakeAuthorizeService = mockAuthService(authServiceResponseData);

const fakeCookieConsentService: CookieConsentServiceInterface = {
getCookieConsent: sinon.fake(),
createConsentCookieValue: sinon.fake.returns({
value: JSON.stringify("cookieValue"),
expiry: "cookieExpires",
}),
} as unknown as CookieConsentServiceInterface;
const fakeCookieConsentService = createMockCookieConsentService(
req.body.cookie_preferences
);

const consentCookieValue =
fakeCookieConsentService.createConsentCookieValue(
req.body.cookie_preferences === "true"
? COOKIE_CONSENT.ACCEPT
: COOKIE_CONSENT.REJECT
);

await authorizeGet(
fakeAuthorizeService,
Expand All @@ -285,7 +304,15 @@ describe("authorize controller", () => {
fakeJwtService
)(req as Request, res as Response);

expect(res.cookie).to.have.been.called;
expect(res.cookie).to.have.been.calledWith(
COOKIES_PREFERENCES_SET,
consentCookieValue.value,
sinon.match({
secure: true,
httpOnly: false,
encode: String,
})
);
expect(res.redirect).to.have.calledWith(
`${PATH_NAMES.SIGN_IN_OR_CREATE}?_ga=${gaTrackingId}`
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/common/cookie-consent/cookie-consent-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function cookieConsentService(): CookieConsentServiceInterface {
cookieExpires.setFullYear(cookieExpires.getFullYear() - 1);
}

return { value: JSON.stringify(cookieValue), expiry: cookieExpires };
return { value: JSON.stringify(cookieValue), expires: cookieExpires };
};

return {
Expand Down
6 changes: 3 additions & 3 deletions src/components/common/cookie-consent/tests/cookie-consent-service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("cookie consent service", () => {
COOKIE_CONSENT.NOT_ENGAGED
);
expect(result.value).to.have.be.equal("{}");
expect(result.expiry.getFullYear()).to.have.be.equal(
expect(result.expires.getFullYear()).to.have.be.equal(
new Date().getFullYear() - 1
);
});
Expand All @@ -46,7 +46,7 @@ describe("cookie consent service", () => {
COOKIE_CONSENT.ACCEPT
);
expect(result.value).to.have.be.equal('{"analytics":true}');
expect(result.expiry.getFullYear()).to.have.be.equal(
expect(result.expires.getFullYear()).to.have.be.equal(
new Date().getFullYear() + 1
);
});
Expand All @@ -56,7 +56,7 @@ describe("cookie consent service", () => {
COOKIE_CONSENT.REJECT
);
expect(result.value).to.have.be.equal('{"analytics":false}');
expect(result.expiry.getFullYear()).to.have.be.equal(
expect(result.expires.getFullYear()).to.have.be.equal(
new Date().getFullYear() + 1
);
});
Expand Down
2 changes: 1 addition & 1 deletion src/components/common/cookie-consent/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ export interface CookieConsentServiceInterface {

export interface CookieConsentModel {
value: string;
expiry: Date;
expires: Date;
}
21 changes: 7 additions & 14 deletions src/components/common/cookies/cookies-controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
COOKIES_PREFERENCES_SET,
COOKIE_CONSENT,
} from "../../../app.constants";
import { CookieConsentModel } from "../cookie-consent/types";

const cookieService = cookieConsentService();

Expand All @@ -27,22 +26,16 @@ export function cookiesPost(req: Request, res: Response): void {
consentValue === "true" ? COOKIE_CONSENT.ACCEPT : COOKIE_CONSENT.REJECT
);

createConsentCookie(res, consentCookieValue);
res.cookie(COOKIES_PREFERENCES_SET, consentCookieValue.value, {
expires: consentCookieValue.expires,
secure: true,
httpOnly: false,
domain: res.locals.analyticsCookieDomain,
encode: String,
});

res.locals.backUrl = req.body.originalReferer;
res.locals.analyticsConsent = consentValue === "true";
res.locals.updated = true;
res.render("common/cookies/index.njk");
}

function createConsentCookie(
res: Response,
consentCookieValue: CookieConsentModel
) {
res.cookie(COOKIES_PREFERENCES_SET, consentCookieValue.value, {
expires: consentCookieValue.expiry,
secure: true,
httpOnly: true,
domain: res.locals.analyticsCookieDomain,
});
}
101 changes: 82 additions & 19 deletions src/components/common/cookies/tests/cookies-controller.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ import { sinon } from "../../../../../test/utils/test-utils";
import { Request, Response } from "express";

import { cookiesGet, cookiesPost } from "../cookies-controller";
import { COOKIES_PREFERENCES_SET, PATH_NAMES } from "../../../../app.constants";
import {
COOKIE_CONSENT,
COOKIES_PREFERENCES_SET,
PATH_NAMES,
} from "../../../../app.constants";
import { mockResponse, RequestOutput, ResponseOutput } from "mock-req-res";
import { createMockCookieConsentService } from "../../../../../test/helpers/mock-cookie-consent-service-helper";
import { createMockRequest } from "../../../../../test/helpers/mock-request-helper";

describe("cookies controller", () => {
Expand All @@ -33,30 +38,88 @@ describe("cookies controller", () => {
expect(res.locals.backUrl).to.equal("/last-page");
});
});

describe("cookiesPost", () => {
it("should save analytics preferences as yes and render cookies page", () => {
req.body.cookie_preferences = "true";
req.body.originalReferer = "/page-before-1";
describe("where the user has consented to cookies", () => {
it("should call res.cookie with the expected arguments and flags", () => {
req.body.cookie_preferences = "true";

cookiesPost(req as Request, res as Response);
const fakeCookieConsentService = createMockCookieConsentService(
req.body.cookie_preferences
);

expect(res.render).to.have.been.calledWith("common/cookies/index.njk");
expect(res.locals.analyticsConsent).to.equal(true);
expect(res.locals.updated).to.equal(true);
expect(res.locals.backUrl).to.equal("/page-before-1");
expect(res.cookie).to.have.been.calledWith(COOKIES_PREFERENCES_SET);
const consentCookieValue =
fakeCookieConsentService.createConsentCookieValue(
req.body.cookie_preferences === "true"
? COOKIE_CONSENT.ACCEPT
: COOKIE_CONSENT.REJECT
);

cookiesPost(req as Request, res as Response);

expect(res.cookie).to.have.been.calledWith(
COOKIES_PREFERENCES_SET,
consentCookieValue.value,
sinon.match({
secure: true,
httpOnly: false,
encode: String,
})
);
});

it("should render the page", () => {
req.body.cookie_preferences = "true";
req.body.originalReferer = "/page-before-1";

cookiesPost(req as Request, res as Response);

expect(res.render).to.have.been.calledWith("common/cookies/index.njk");
expect(res.locals.analyticsConsent).to.equal(true);
expect(res.locals.updated).to.equal(true);
expect(res.locals.backUrl).to.equal("/page-before-1");
});
});
it("should save analytics preferences as no and render cookies page", () => {
req.body.cookie_preferences = "false";
req.body.originalReferer = "/page-before-2";

cookiesPost(req as Request, res as Response);
describe("where the user has not consented to cookies", () => {
it("should call res.cookie with the expected arguments and flags", () => {
req.body.cookie_preferences = "false";

expect(res.render).to.have.been.calledWith("common/cookies/index.njk");
expect(res.locals.analyticsConsent).to.equal(false);
expect(res.locals.updated).to.equal(true);
expect(res.locals.backUrl).to.equal("/page-before-2");
expect(res.cookie).to.have.been.calledWith(COOKIES_PREFERENCES_SET);
const fakeCookieConsentService = createMockCookieConsentService(
req.body.cookie_preferences
);

const consentCookieValue =
fakeCookieConsentService.createConsentCookieValue(
req.body.cookie_preferences === "true"
? COOKIE_CONSENT.ACCEPT
: COOKIE_CONSENT.REJECT
);

cookiesPost(req as Request, res as Response);

expect(res.cookie).to.have.been.calledWith(
COOKIES_PREFERENCES_SET,
consentCookieValue.value,
sinon.match({
secure: true,
httpOnly: false,
encode: String,
})
);
});

it("should render the page", () => {
req.body.cookie_preferences = "false";
req.body.originalReferer = "/page-before-2";

cookiesPost(req as Request, res as Response);

expect(res.render).to.have.been.calledWith("common/cookies/index.njk");
expect(res.locals.analyticsConsent).to.equal(false);
expect(res.locals.updated).to.equal(true);
expect(res.locals.backUrl).to.equal("/page-before-2");
});
});
});
});
19 changes: 19 additions & 0 deletions test/helpers/mock-cookie-consent-service-helper.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { sinon } from "../utils/test-utils";
import { CookieConsentServiceInterface } from "../../src/components/common/cookie-consent/types";

export function createMockCookieConsentService(
userCookieConsentPreference: string
): CookieConsentServiceInterface {
const expiryDate: Date = new Date();
expiryDate.setFullYear(expiryDate.getFullYear() + 1);

return {
getCookieConsent: sinon.fake(),
createConsentCookieValue: sinon.fake.returns({
value: JSON.stringify({
analytics: userCookieConsentPreference === "true",
}),
expires: expiryDate,
}),
};
}

0 comments on commit b47b1c8

Please sign in to comment.