From ec9a1e22f7a38563469e09e49f2d661b3099808e Mon Sep 17 00:00:00 2001 From: Giuseppe Digilio Date: Tue, 30 Jul 2024 20:19:18 +0200 Subject: [PATCH] [DURACOM-288] Provide a setting to use a different REST url during SSR execution --- server.ts | 9 +- src/app/app.config.ts | 6 + .../dspace-rest.interceptor.spec.ts | 194 ++++++++++++++++++ .../dspace-rest/dspace-rest.interceptor.ts | 52 +++++ .../server-hard-redirect.service.spec.ts | 3 +- .../services/server-hard-redirect.service.ts | 19 +- src/app/thumbnail/thumbnail.component.ts | 28 ++- src/config/server-config.interface.ts | 2 + src/modules/app/browser-init.service.ts | 23 ++- src/modules/app/server-init.service.ts | 11 +- 10 files changed, 319 insertions(+), 28 deletions(-) create mode 100644 src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts create mode 100644 src/app/core/dspace-rest/dspace-rest.interceptor.ts diff --git a/server.ts b/server.ts index 1276621e9d..127a179e7a 100644 --- a/server.ts +++ b/server.ts @@ -81,6 +81,9 @@ let anonymousCache: LRU; // extend environment with app config for server extendEnvironmentWithAppConfig(environment, appConfig); +// The REST server base URL +const REST_BASE_URL = environment.rest.ssrBaseUrl || environment.rest.baseUrl; + // The Express app is exported so that it can be used by serverless Functions. export function app() { @@ -156,7 +159,7 @@ export function app() { * Proxy the sitemaps */ router.use('/sitemap**', createProxyMiddleware({ - target: `${environment.rest.baseUrl}/sitemaps`, + target: `${REST_BASE_URL}/sitemaps`, pathRewrite: path => path.replace(environment.ui.nameSpace, '/'), changeOrigin: true, })); @@ -165,7 +168,7 @@ export function app() { * Proxy the linksets */ router.use('/signposting**', createProxyMiddleware({ - target: `${environment.rest.baseUrl}`, + target: `${REST_BASE_URL}`, pathRewrite: path => path.replace(environment.ui.nameSpace, '/'), changeOrigin: true, })); @@ -623,7 +626,7 @@ function start() { * The callback function to serve health check requests */ function healthCheck(req, res) { - const baseUrl = `${environment.rest.baseUrl}${environment.actuators.endpointPath}`; + const baseUrl = `${REST_BASE_URL}${environment.actuators.endpointPath}`; axios.get(baseUrl) .then((response) => { res.status(response.status).send(response.data); diff --git a/src/app/app.config.ts b/src/app/app.config.ts index 61a04c8adb..77b29206cb 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -54,6 +54,7 @@ import { } from './app-routes'; import { BROWSE_BY_DECORATOR_MAP } from './browse-by/browse-by-switcher/browse-by-decorator'; import { AuthInterceptor } from './core/auth/auth.interceptor'; +import { DspaceRestInterceptor } from './core/dspace-rest/dspace-rest.interceptor'; import { LocaleInterceptor } from './core/locale/locale.interceptor'; import { LogInterceptor } from './core/log/log.interceptor'; import { @@ -148,6 +149,11 @@ export const commonAppConfig: ApplicationConfig = { useClass: LogInterceptor, multi: true, }, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, // register the dynamic matcher used by form. MUST be provided by the app module ...DYNAMIC_MATCHER_PROVIDERS, provideCore(), diff --git a/src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts b/src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts new file mode 100644 index 0000000000..4a47ffe9fd --- /dev/null +++ b/src/app/core/dspace-rest/dspace-rest.interceptor.spec.ts @@ -0,0 +1,194 @@ +import { + HTTP_INTERCEPTORS, + HttpClient, +} from '@angular/common/http'; +import { + HttpClientTestingModule, + HttpTestingController, +} from '@angular/common/http/testing'; +import { PLATFORM_ID } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { + APP_CONFIG, + AppConfig, +} from '../../../config/app-config.interface'; +import { DspaceRestInterceptor } from './dspace-rest.interceptor'; +import { DspaceRestService } from './dspace-rest.service'; + +describe('DspaceRestInterceptor', () => { + let httpMock: HttpTestingController; + let httpClient: HttpClient; + const appConfig: Partial = { + rest: { + ssl: false, + host: 'localhost', + port: 8080, + nameSpace: '/server', + baseUrl: 'http://api.example.com/server', + }, + }; + const appConfigWithSSR: Partial = { + rest: { + ssl: false, + host: 'localhost', + port: 8080, + nameSpace: '/server', + baseUrl: 'http://api.example.com/server', + ssrBaseUrl: 'http://ssr.example.com/server', + }, + }; + + describe('When SSR base URL is not set ', () => { + describe('and it\'s in the browser', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfig }, + { provide: PLATFORM_ID, useValue: 'browser' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should not modify the request', () => { + const url = 'http://api.example.com/server/items'; + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.url).toBe(url); + req.flush({}); + httpMock.verify(); + }); + }); + + describe('and it\'s in SSR mode', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfig }, + { provide: PLATFORM_ID, useValue: 'server' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should not replace the base URL', () => { + const url = 'http://api.example.com/server/items'; + + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.url).toBe(url); + req.flush({}); + httpMock.verify(); + }); + }); + }); + + describe('When SSR base URL is set ', () => { + describe('and it\'s in the browser', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfigWithSSR }, + { provide: PLATFORM_ID, useValue: 'browser' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should not modify the request', () => { + const url = 'http://api.example.com/server/items'; + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.url).toBe(url); + req.flush({}); + httpMock.verify(); + }); + }); + + describe('and it\'s in SSR mode', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DspaceRestService, + { + provide: HTTP_INTERCEPTORS, + useClass: DspaceRestInterceptor, + multi: true, + }, + { provide: APP_CONFIG, useValue: appConfigWithSSR }, + { provide: PLATFORM_ID, useValue: 'server' }, + ], + }); + + httpMock = TestBed.inject(HttpTestingController); + httpClient = TestBed.inject(HttpClient); + }); + + it('should replace the base URL', () => { + const url = 'http://api.example.com/server/items'; + const ssrBaseUrl = appConfigWithSSR.rest.ssrBaseUrl; + + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(ssrBaseUrl + '/items'); + expect(req.request.url).toBe(ssrBaseUrl + '/items'); + req.flush({}); + httpMock.verify(); + }); + + it('should not replace any query param containing the base URL', () => { + const url = 'http://api.example.com/server/items?url=http://api.example.com/server/item/1'; + const ssrBaseUrl = appConfigWithSSR.rest.ssrBaseUrl; + + httpClient.get(url).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const req = httpMock.expectOne(ssrBaseUrl + '/items?url=http://api.example.com/server/item/1'); + expect(req.request.url).toBe(ssrBaseUrl + '/items?url=http://api.example.com/server/item/1'); + req.flush({}); + httpMock.verify(); + }); + }); + }); +}); diff --git a/src/app/core/dspace-rest/dspace-rest.interceptor.ts b/src/app/core/dspace-rest/dspace-rest.interceptor.ts new file mode 100644 index 0000000000..efd2c12b5d --- /dev/null +++ b/src/app/core/dspace-rest/dspace-rest.interceptor.ts @@ -0,0 +1,52 @@ +import { isPlatformBrowser } from '@angular/common'; +import { + HttpEvent, + HttpHandler, + HttpInterceptor, + HttpRequest, +} from '@angular/common/http'; +import { + Inject, + Injectable, + PLATFORM_ID, +} from '@angular/core'; +import { Observable } from 'rxjs'; + +import { + APP_CONFIG, + AppConfig, +} from '../../../config/app-config.interface'; +import { isEmpty } from '../../shared/empty.util'; + +@Injectable() +/** + * This Interceptor is used to use the configured base URL for the request made during SSR execution + */ +export class DspaceRestInterceptor implements HttpInterceptor { + + /** + * Contains the configured application base URL + * @protected + */ + protected baseUrl: string; + protected ssrBaseUrl: string; + + constructor( + @Inject(APP_CONFIG) protected appConfig: AppConfig, + @Inject(PLATFORM_ID) private platformId: string, + ) { + this.baseUrl = this.appConfig.rest.baseUrl; + this.ssrBaseUrl = this.appConfig.rest.ssrBaseUrl; + } + + intercept(request: HttpRequest, next: HttpHandler): Observable> { + if (isPlatformBrowser(this.platformId) || isEmpty(this.ssrBaseUrl) || this.baseUrl === this.ssrBaseUrl) { + return next.handle(request); + } + + // Different SSR Base URL specified so replace it in the current request url + const url = request.url.replace(this.baseUrl, this.ssrBaseUrl); + const newRequest: HttpRequest = request.clone({ url }); + return next.handle(newRequest); + } +} diff --git a/src/app/core/services/server-hard-redirect.service.spec.ts b/src/app/core/services/server-hard-redirect.service.spec.ts index 27777ed7ba..d3247dd0dd 100644 --- a/src/app/core/services/server-hard-redirect.service.spec.ts +++ b/src/app/core/services/server-hard-redirect.service.spec.ts @@ -1,5 +1,6 @@ import { TestBed } from '@angular/core/testing'; +import { environment } from '../../../environments/environment.test'; import { ServerHardRedirectService } from './server-hard-redirect.service'; describe('ServerHardRedirectService', () => { @@ -7,7 +8,7 @@ describe('ServerHardRedirectService', () => { const mockRequest = jasmine.createSpyObj(['get']); const mockResponse = jasmine.createSpyObj(['redirect', 'end']); - const service: ServerHardRedirectService = new ServerHardRedirectService(mockRequest, mockResponse); + const service: ServerHardRedirectService = new ServerHardRedirectService(environment, mockRequest, mockResponse); const origin = 'https://test-host.com:4000'; beforeEach(() => { diff --git a/src/app/core/services/server-hard-redirect.service.ts b/src/app/core/services/server-hard-redirect.service.ts index e1ded1568c..1592d9bf1c 100644 --- a/src/app/core/services/server-hard-redirect.service.ts +++ b/src/app/core/services/server-hard-redirect.service.ts @@ -7,10 +7,15 @@ import { Response, } from 'express'; +import { + APP_CONFIG, + AppConfig, +} from '../../../config/app-config.interface'; import { REQUEST, RESPONSE, } from '../../../express.tokens'; +import { isNotEmpty } from '../../shared/empty.util'; import { HardRedirectService } from './hard-redirect.service'; /** @@ -20,6 +25,7 @@ import { HardRedirectService } from './hard-redirect.service'; export class ServerHardRedirectService extends HardRedirectService { constructor( + @Inject(APP_CONFIG) protected appConfig: AppConfig, @Inject(REQUEST) protected req: Request, @Inject(RESPONSE) protected res: Response, ) { @@ -35,17 +41,22 @@ export class ServerHardRedirectService extends HardRedirectService { * optional HTTP status code to use for redirect (default = 302, which is a temporary redirect) */ redirect(url: string, statusCode?: number) { - if (url === this.req.url) { return; } + let redirectUrl = url; + // If redirect url contains SSR base url then replace with public base url + if (isNotEmpty(this.appConfig.rest.ssrBaseUrl) && this.appConfig.rest.baseUrl !== this.appConfig.rest.ssrBaseUrl) { + redirectUrl = url.replace(this.appConfig.rest.ssrBaseUrl, this.appConfig.rest.baseUrl); + } + if (this.res.finished) { const req: any = this.req; req._r_count = (req._r_count || 0) + 1; console.warn('Attempted to redirect on a finished response. From', - this.req.url, 'to', url); + this.req.url, 'to', redirectUrl); if (req._r_count > 10) { console.error('Detected a redirection loop. killing the nodejs process'); @@ -59,9 +70,9 @@ export class ServerHardRedirectService extends HardRedirectService { status = 302; } - console.log(`Redirecting from ${this.req.url} to ${url} with ${status}`); + console.info(`Redirecting from ${this.req.url} to ${redirectUrl} with ${status}`); - this.res.redirect(status, url); + this.res.redirect(status, redirectUrl); this.res.end(); // I haven't found a way to correctly stop Angular rendering. // So we just let it end its work, though we have already closed diff --git a/src/app/thumbnail/thumbnail.component.ts b/src/app/thumbnail/thumbnail.component.ts index cc583c3998..7b22dde4cd 100644 --- a/src/app/thumbnail/thumbnail.component.ts +++ b/src/app/thumbnail/thumbnail.component.ts @@ -1,8 +1,13 @@ -import { CommonModule } from '@angular/common'; +import { + CommonModule, + isPlatformBrowser, +} from '@angular/common'; import { Component, + Inject, Input, OnChanges, + PLATFORM_ID, SimpleChanges, } from '@angular/core'; import { TranslateModule } from '@ngx-translate/core'; @@ -76,6 +81,7 @@ export class ThumbnailComponent implements OnChanges { isLoading = true; constructor( + @Inject(PLATFORM_ID) private platformID: any, protected auth: AuthService, protected authorizationService: AuthorizationDataService, protected fileService: FileService, @@ -87,16 +93,18 @@ export class ThumbnailComponent implements OnChanges { * Use a default image if no actual image is available. */ ngOnChanges(changes: SimpleChanges): void { - if (hasNoValue(this.thumbnail)) { - this.setSrc(this.defaultImage); - return; - } + if (isPlatformBrowser(this.platformID)) { + if (hasNoValue(this.thumbnail)) { + this.setSrc(this.defaultImage); + return; + } - const src = this.contentHref; - if (hasValue(src)) { - this.setSrc(src); - } else { - this.setSrc(this.defaultImage); + const src = this.contentHref; + if (hasValue(src)) { + this.setSrc(src); + } else { + this.setSrc(this.defaultImage); + } } } diff --git a/src/config/server-config.interface.ts b/src/config/server-config.interface.ts index 8797ea3d60..cdf23cd146 100644 --- a/src/config/server-config.interface.ts +++ b/src/config/server-config.interface.ts @@ -6,4 +6,6 @@ export class ServerConfig implements Config { public port: number; public nameSpace: string; public baseUrl?: string; + public ssrBaseUrl?: string; + public hasSsrBaseUrl?: boolean; } diff --git a/src/modules/app/browser-init.service.ts b/src/modules/app/browser-init.service.ts index 525067da3a..b68b4b426a 100644 --- a/src/modules/app/browser-init.service.ts +++ b/src/modules/app/browser-init.service.ts @@ -144,15 +144,20 @@ export class BrowserInitService extends InitService { * @private */ private async loadAppState(): Promise { - const state = this.transferState.get(InitService.NGRX_STATE, null); - this.transferState.remove(InitService.NGRX_STATE); - this.store.dispatch(new StoreAction(StoreActionTypes.REHYDRATE, state)); - return lastValueFrom( - this.store.select(coreSelector).pipe( - find((core: any) => isNotEmpty(core)), - map(() => true), - ), - ); + // The app state can be transferred only when SSR and CSR are using the same base url for the REST API + if (!this.appConfig.rest.hasSsrBaseUrl) { + const state = this.transferState.get(InitService.NGRX_STATE, null); + this.transferState.remove(InitService.NGRX_STATE); + this.store.dispatch(new StoreAction(StoreActionTypes.REHYDRATE, state)); + return lastValueFrom( + this.store.select(coreSelector).pipe( + find((core: any) => isNotEmpty(core)), + map(() => true), + ), + ); + } else { + return Promise.resolve(true); + } } private trackAuthTokenExpiration(): void { diff --git a/src/modules/app/server-init.service.ts b/src/modules/app/server-init.service.ts index c4aa4ffba3..851c2c24c0 100644 --- a/src/modules/app/server-init.service.ts +++ b/src/modules/app/server-init.service.ts @@ -21,6 +21,7 @@ import { LocaleService } from '../../app/core/locale/locale.service'; import { HeadTagService } from '../../app/core/metadata/head-tag.service'; import { CorrelationIdService } from '../../app/correlation-id/correlation-id.service'; import { InitService } from '../../app/init.service'; +import { isNotEmpty } from '../../app/shared/empty.util'; import { MenuService } from '../../app/shared/menu/menu.service'; import { ThemeService } from '../../app/shared/theme-support/theme.service'; import { Angulartics2DSpace } from '../../app/statistics/angulartics/dspace-provider'; @@ -100,6 +101,14 @@ export class ServerInitService extends InitService { } private saveAppConfigForCSR(): void { - this.transferState.set(APP_CONFIG_STATE, environment as AppConfig); + if (isNotEmpty(environment.rest.ssrBaseUrl) && environment.rest.baseUrl !== environment.rest.ssrBaseUrl) { + // Avoid to transfer ssrBaseUrl in order to prevent security issues + const config: AppConfig = Object.assign({}, environment as AppConfig, { + rest: Object.assign({}, environment.rest, { ssrBaseUrl: '', hasSsrBaseUrl: true }), + }); + this.transferState.set(APP_CONFIG_STATE, config); + } else { + this.transferState.set(APP_CONFIG_STATE, environment as AppConfig); + } } }