From 241816e8369b2b0dde3dbee603ff1947ec2ff0d5 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 22 Sep 2022 12:39:58 +0200 Subject: [PATCH 1/2] use location.replace to ensure the browser can track the redirect in its history --- .../browser-hard-redirect.service.spec.ts | 30 ++++++++++++------- .../services/browser-hard-redirect.service.ts | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/app/core/services/browser-hard-redirect.service.spec.ts b/src/app/core/services/browser-hard-redirect.service.spec.ts index b9745906c3..1d8666d259 100644 --- a/src/app/core/services/browser-hard-redirect.service.spec.ts +++ b/src/app/core/services/browser-hard-redirect.service.spec.ts @@ -2,17 +2,25 @@ import { TestBed } from '@angular/core/testing'; import { BrowserHardRedirectService } from './browser-hard-redirect.service'; describe('BrowserHardRedirectService', () => { - const origin = 'https://test-host.com:4000'; - const mockLocation = { - href: undefined, - pathname: '/pathname', - search: '/search', - origin - } as Location; - - const service: BrowserHardRedirectService = new BrowserHardRedirectService(mockLocation); + let origin: string; + let mockLocation: Location; + let service: BrowserHardRedirectService; beforeEach(() => { + origin = 'https://test-host.com:4000'; + mockLocation = { + href: undefined, + pathname: '/pathname', + search: '/search', + origin, + replace: (url: string) => { + mockLocation.href = url; + } + } as Location; + spyOn(mockLocation, 'replace'); + + service = new BrowserHardRedirectService(mockLocation); + TestBed.configureTestingModule({}); }); @@ -28,8 +36,8 @@ describe('BrowserHardRedirectService', () => { service.redirect(redirect); }); - it('should update the location', () => { - expect(mockLocation.href).toEqual(redirect); + it('should call location.replace with the new url', () => { + expect(mockLocation.replace).toHaveBeenCalledWith(redirect); }); }); diff --git a/src/app/core/services/browser-hard-redirect.service.ts b/src/app/core/services/browser-hard-redirect.service.ts index eeb9006039..4ef9548899 100644 --- a/src/app/core/services/browser-hard-redirect.service.ts +++ b/src/app/core/services/browser-hard-redirect.service.ts @@ -24,7 +24,7 @@ export class BrowserHardRedirectService extends HardRedirectService { * @param url */ redirect(url: string) { - this.location.href = url; + this.location.replace(url); } /** From 941e71a75b991b8612d0a0f7244f9a4f71911357 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 28 Sep 2022 13:10:46 +0200 Subject: [PATCH 2/2] add useProxies config to support x-forwarded headers in express --- server.ts | 4 ++++ src/config/config.util.spec.ts | 4 ++++ src/config/default-app-config.ts | 5 ++++- src/config/ui-server-config.interface.ts | 2 ++ src/environments/environment.test.ts | 3 ++- 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/server.ts b/server.ts index 9fe03fe5b5..8f37c551f0 100644 --- a/server.ts +++ b/server.ts @@ -75,6 +75,10 @@ export function app() { */ const server = express(); + // 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); + /* * If production mode is enabled in the environment file: * - Enable Angular's production mode diff --git a/src/config/config.util.spec.ts b/src/config/config.util.spec.ts index 7896b1f5c2..2d1d8e1be7 100644 --- a/src/config/config.util.spec.ts +++ b/src/config/config.util.spec.ts @@ -10,6 +10,7 @@ describe('Config Util', () => { expect(appConfig.cache.msToLive.default).toEqual(15 * 60 * 1000); // 15 minute expect(appConfig.ui.rateLimiter.windowMs).toEqual(1 * 60 * 1000); // 1 minute expect(appConfig.ui.rateLimiter.max).toEqual(500); + expect(appConfig.ui.useProxies).toEqual(true); expect(appConfig.submission.autosave.metadata).toEqual([]); @@ -25,6 +26,8 @@ describe('Config Util', () => { }; appConfig.ui.rateLimiter = rateLimiter; + appConfig.ui.useProxies = false; + const autoSaveMetadata = [ 'dc.author', 'dc.title' @@ -44,6 +47,7 @@ describe('Config Util', () => { expect(environment.cache.msToLive.default).toEqual(msToLive); expect(environment.ui.rateLimiter.windowMs).toEqual(rateLimiter.windowMs); expect(environment.ui.rateLimiter.max).toEqual(rateLimiter.max); + expect(environment.ui.useProxies).toEqual(false); expect(environment.submission.autosave.metadata[0]).toEqual(autoSaveMetadata[0]); expect(environment.submission.autosave.metadata[1]).toEqual(autoSaveMetadata[1]); diff --git a/src/config/default-app-config.ts b/src/config/default-app-config.ts index b1b64d1c87..146c7a57c4 100644 --- a/src/config/default-app-config.ts +++ b/src/config/default-app-config.ts @@ -37,7 +37,10 @@ export class DefaultAppConfig implements AppConfig { rateLimiter: { windowMs: 1 * 60 * 1000, // 1 minute max: 500 // limit each IP to 500 requests per windowMs - } + }, + + // Trust X-FORWARDED-* headers from proxies + useProxies: true, }; // The REST API server settings diff --git a/src/config/ui-server-config.interface.ts b/src/config/ui-server-config.interface.ts index 93f90c345c..70e2fa3e26 100644 --- a/src/config/ui-server-config.interface.ts +++ b/src/config/ui-server-config.interface.ts @@ -11,4 +11,6 @@ export class UIServerConfig extends ServerConfig { max: number; }; + // Trust X-FORWARDED-* headers from proxies + useProxies: boolean; } diff --git a/src/environments/environment.test.ts b/src/environments/environment.test.ts index 6fe4dd6516..7838a351bf 100644 --- a/src/environments/environment.test.ts +++ b/src/environments/environment.test.ts @@ -25,7 +25,8 @@ export const environment: BuildConfig = { rateLimiter: { windowMs: 1 * 60 * 1000, // 1 minute max: 500 // limit each IP to 500 requests per windowMs - } + }, + useProxies: true, }, // The REST API server settings.