From 6317c1f473ce8d5d82bf6174f2fae70ad727c520 Mon Sep 17 00:00:00 2001 From: Matus Kasak Date: Thu, 6 Aug 2026 12:10:32 +0200 Subject: [PATCH] JCU/fix(security): stop advertising Express and mark cookies Secure on HTTPS Two of the three findings in M7 live in this repo: * Every response carried `X-Powered-By: Express`. That is fingerprinting material for an attacker and useful to nobody else, so the header is now disabled on the SSR server. * Cookies written by the UI - `XSRF-TOKEN`, `dsLanguage`, the Orejime consent cookie, `dsAccessibility`, `dsCorrelationId`, the redirect and impersonation cookies - were written without the `Secure` attribute, so they are sent back over plaintext if the user is ever downgraded to HTTP. ClientCookieService now adds `Secure` whenever the page itself was loaded over HTTPS. The decision is taken from the page protocol rather than from `ui.ssl`, because TLS is usually terminated by a reverse proxy and `ui.ssl` is false on HTTPS-only sites; reading the protocol also keeps `http://localhost` development working. The third finding (missing HSTS / CSP / nosniff / Referrer-Policy / Permissions-Policy headers) belongs to the nginx in front of the app and cannot be fixed from here - a ready-to-apply snippet is in the PR description. Fixes the in-repo part of M7 from dataquest-dev/dspace-customers#853. Co-Authored-By: Claude Opus 5 (1M context) --- server.ts | 5 ++ .../services/client-cookie.service.spec.ts | 60 +++++++++++++++++++ .../core/services/client-cookie.service.ts | 36 ++++++++++- 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 src/app/core/services/client-cookie.service.spec.ts diff --git a/server.ts b/server.ts index 568e41b3a71..cb7cd96581a 100644 --- a/server.ts +++ b/server.ts @@ -98,6 +98,11 @@ export function app() { */ const server = express(); + // Don't advertise the server stack to every client. "X-Powered-By: Express" is pure + // fingerprinting material: it tells an attacker which exploits are worth trying and is of + // no use to anyone else. + server.disable('x-powered-by'); + // Tell Express to trust X-FORWARDED-* headers from proxies // See https://expressjs.com/en/guide/behind-proxies.html server.set('trust proxy', environment.ui.useProxies); diff --git a/src/app/core/services/client-cookie.service.spec.ts b/src/app/core/services/client-cookie.service.spec.ts new file mode 100644 index 00000000000..63d2fcd48f0 --- /dev/null +++ b/src/app/core/services/client-cookie.service.spec.ts @@ -0,0 +1,60 @@ +import { DOCUMENT } from '@angular/common'; +import { TestBed } from '@angular/core/testing'; +import Cookies from 'js-cookie'; + +import { ClientCookieService } from './client-cookie.service'; + +describe('ClientCookieService', () => { + + /** + * Builds the service around a document that claims to be served over the given protocol. + */ + function serviceOn(protocol: string): ClientCookieService { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + ClientCookieService, + { provide: DOCUMENT, useValue: { location: { protocol } } }, + ], + }); + return TestBed.inject(ClientCookieService); + } + + beforeEach(() => { + spyOn(Cookies, 'set'); + }); + + describe('when the page is served over HTTPS', () => { + it('should mark the cookie as Secure', () => { + serviceOn('https:').set('XSRF-TOKEN', 'a-token'); + + expect(Cookies.set).toHaveBeenCalledWith('XSRF-TOKEN', 'a-token', jasmine.objectContaining({ secure: true })); + }); + + it('should keep the caller\'s other attributes', () => { + serviceOn('https:').set('some-cookie', 'value', { expires: 7 }); + + expect(Cookies.set).toHaveBeenCalledWith('some-cookie', 'value', { expires: 7, secure: true }); + }); + }); + + describe('when the page is served over plain HTTP', () => { + it('should not mark the cookie as Secure, so local development keeps working', () => { + serviceOn('http:').set('XSRF-TOKEN', 'a-token'); + + expect(Cookies.set).toHaveBeenCalledWith('XSRF-TOKEN', 'a-token', jasmine.objectContaining({ secure: false })); + }); + }); + + it('should let an explicit secure attribute win', () => { + serviceOn('https:').set('some-cookie', 'value', { secure: false }); + + expect(Cookies.set).toHaveBeenCalledWith('some-cookie', 'value', { secure: false }); + }); + + it('should still serialize non-string values as JSON', () => { + serviceOn('https:').set('some-cookie', { a: 1 }); + + expect(Cookies.set).toHaveBeenCalledWith('some-cookie', '{"a":1}', jasmine.objectContaining({ secure: true })); + }); +}); diff --git a/src/app/core/services/client-cookie.service.ts b/src/app/core/services/client-cookie.service.ts index 9ea0d7c5b18..1200c6d43a3 100644 --- a/src/app/core/services/client-cookie.service.ts +++ b/src/app/core/services/client-cookie.service.ts @@ -1,4 +1,8 @@ -import { Injectable } from '@angular/core'; +import { DOCUMENT } from '@angular/common'; +import { + Inject, + Injectable, +} from '@angular/core'; import Cookies from 'js-cookie'; import { @@ -9,9 +13,13 @@ import { @Injectable() export class ClientCookieService extends CookieService implements ICookieService { + constructor(@Inject(DOCUMENT) protected document: Document) { + super(); + } + public set(name: string, value: any, options?: Cookies.CookieAttributes): void { const toStore = typeof value === 'string' ? value : JSON.stringify(value); - Cookies.set(name, toStore, options); + Cookies.set(name, toStore, this.withSecureFlag(options)); this.updateSource(); } @@ -20,6 +28,30 @@ export class ClientCookieService extends CookieService implements ICookieService this.updateSource(); } + /** + * Marks every cookie we write as `Secure` whenever the page itself was loaded over HTTPS, so the + * browser never sends it back over a plaintext connection. Without this, cookies such as + * `XSRF-TOKEN` are written without the attribute and travel in the clear if the user is ever + * downgraded to HTTP. + * + * The decision is taken from the protocol of the current page rather than from `ui.ssl`, because + * in a typical deployment TLS is terminated by a reverse proxy and the UI server itself is + * configured as plain HTTP — `ui.ssl` would be `false` on a site that is HTTPS-only. Reading the + * page protocol also keeps local development over `http://localhost` working, where a `Secure` + * cookie is not guaranteed to be accepted. + * + * An explicit `secure` in {@link Cookies.CookieAttributes} always wins. + */ + private withSecureFlag(options?: Cookies.CookieAttributes): Cookies.CookieAttributes { + if (options?.secure !== undefined) { + return options; + } + return { + ...options, + secure: this.document?.location?.protocol === 'https:', + }; + } + public get(name: string): any { const raw = Cookies.get(name); if (raw === undefined) {