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) {