diff --git a/docs/Configuration.md b/docs/Configuration.md index 4be21d046d..f7ab4988fd 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -63,3 +63,70 @@ Angulartics can be configured to work with a number of other services besides Go In order to start using one of these services, select it from the [Angulartics Providers page](https://angulartics.github.io/angulartics2/#providers), and follow the instructions on how to configure it. The Google Analytics script was added in [`main.browser.ts`](https://github.com/DSpace/dspace-angular/blob/ff04760f4af91ac3e7add5e7424a46cb2439e874/src/main.browser.ts#L33) instead of the `` tag in `index.html` to ensure events get sent when the page is shown in a client's browser, and not when it's rendered on the universal server. Likely you'll want to do the same when adding a new service. + +## SEO when hosting REST Api and UI on different servers + +Indexers such as Google Scholar require that files are hosted on the same domain as the page that links them. In DSpace 7, Bitstreams are served from the REST server. So if you use different servers for the REST api and the UI you'll want to ensure that Bitstream downloads are proxied through the UI server. + +In order to achieve this we'll need to do two things: +- **Proxy the Bitstream downloads through the UI server.** You'll need to put a webserver such as httpd or nginx in front of the UI server in order to achieve this. [Below](#apache-http-server-config) you'll find a section explaining how to do it in httpd. +- **Update the URLs for Bitstream downloads to match the UI server.** This can be done using a setting in the UI environment file. + +### UI config +If you set the property `rewriteDownloadUrls` to `true` in your `environment.prod.ts` file, the [origin](https://developer.mozilla.org/en-US/docs/Glossary/Origin) of any download URL will be replaced by the origin of the UI. This will also happen for the `citation_pdf_url` `` tag on Item pages. + +The app will determine the UI origin currently in use, so the external UI URL doesn't need to be configured anywhere and rewrites will still work if you host the UI from multiple domains. + +### Apache HTTP Server config + +#### Basics +In order to be able to host bitstreams from the UI Server you'll need to enable mod_proxy and add the following to the httpd config of your UI server: + +``` +ProxyPassMatch "/server/api/core/bitstreams/([^/]+)/content" "http://rest.api/server/api/core/bitstreams/$1/content" +ProxyPassReverse "/server/api/core/bitstreams/([^/]+)/content" "http://rest.api/server/api/core/bitstreams/$1/content" +``` + +Replace http://rest.api in with the correct origin for your REST server. + +The `ProxyPassMatch` line forwards all requests matching the regular expression for a bitstream download URL to the corresponding path on the REST server + +The `ProxyPassReverse` ensures that if the REST server were to return redirect response, httpd would also swap out its hostname for the hostname of the UI before forwarding the response to the client. + +#### Using HTTPS +If your REST server uses https, you'll need to enable mod_ssl and ensure `SSLProxyEngine on` is part of your UI server's httpd config as well + +If the UI hostname doesn't match the CN in the SSL certificate of the REST server (which is likely if they're on different domains), you'll also need to add the following lines + +``` +SSLProxyCheckPeerCN off +SSLProxyCheckPeerName off +``` +These are two names for [the same directive](https://httpd.apache.org/docs/trunk/mod/mod_ssl.html#sslproxycheckpeername) that have been used for various versions of httpd, old versions need the former, then some in-between versions need both, and newer versions only need the latter. Keeping them both doesn't harm anything. + +So the entire config becomes: + +``` +SSLProxyEngine on +SSLProxyCheckPeerCN off +SSLProxyCheckPeerName off +ProxyPassMatch "/server/api/core/bitstreams/([^/]+)/content" "https://rest.api/server/api/core/bitstreams/$1/content" +ProxyPassReverse "/server/api/core/bitstreams/([^/]+)/content" "https://rest.api/server/api/core/bitstreams/$1/content" +``` + +If you don't want httpd to verify the certificate of the REST server, you can also turn all checks off with the following config: + +``` +SSLProxyEngine on +SSLProxyVerify none +SSLProxyCheckPeerCN off +SSLProxyCheckPeerName off +SSLProxyCheckPeerExpire off +ProxyPassMatch "/server/api/core/bitstreams/([^/]+)/content" "https://rest.api/server/api/core/bitstreams/$1/content" +ProxyPassReverse "/server/api/core/bitstreams/([^/]+)/content" "https://rest.api/server/api/core/bitstreams/$1/content" +``` + + + + + diff --git a/scripts/set-env.ts b/scripts/set-env.ts index 5570b77218..0aa106538c 100644 --- a/scripts/set-env.ts +++ b/scripts/set-env.ts @@ -57,8 +57,8 @@ function generateEnvironmentFile(file: GlobalConfig): void { // TODO remove workaround in beta 5 if (file.rest.nameSpace.match("(.*)/api/?$") !== null) { - const newValue = getNameSpace(file.rest.nameSpace); - console.log(colors.white.bgMagenta.bold(`The rest.nameSpace property in your environment file or in your DSPACE_REST_NAMESPACE environment variable ends with '/api'.\nThis is deprecated. As '/api' isn't configurable on the rest side, it shouldn't be repeated in every environment file.\nPlease change the rest nameSpace to '${newValue}'`)); + file.rest.nameSpace = getNameSpace(file.rest.nameSpace); + console.log(colors.white.bgMagenta.bold(`The rest.nameSpace property in your environment file or in your DSPACE_REST_NAMESPACE environment variable ends with '/api'.\nThis is deprecated. As '/api' isn't configurable on the rest side, it shouldn't be repeated in every environment file.\nPlease change the rest nameSpace to '${file.rest.nameSpace}'`)); } const contents = `export const environment = ` + JSON.stringify(file); diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 74c4522bbd..28fe8e1acc 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -52,10 +52,13 @@ import { UUIDService } from '../shared/uuid.service'; import { MetadataService } from './metadata.service'; import { environment } from '../../../environments/environment'; import { storeModuleConfig } from '../../app.reducer'; +import { HardRedirectService } from '../services/hard-redirect.service'; +import { URLCombiner } from '../url-combiner/url-combiner'; /* tslint:disable:max-classes-per-file */ @Component({ - template: `` + template: ` + ` }) class TestComponent { constructor(private metadata: MetadataService) { @@ -170,6 +173,7 @@ describe('MetadataService', () => { Title, // tslint:disable-next-line:no-empty { provide: ItemDataService, useValue: { findById: () => {} } }, + { provide: HardRedirectService, useValue: { rewriteDownloadURL: (a) => a, getRequestOrigin: () => environment.ui.baseUrl }}, BrowseService, MetadataService ], @@ -208,7 +212,7 @@ describe('MetadataService', () => { tick(); expect(tagStore.get('citation_dissertation_name')[0].content).toEqual('Test PowerPoint Document'); expect(tagStore.get('citation_dissertation_institution')[0].content).toEqual('Mock Publisher'); - expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual([environment.ui.baseUrl, router.url].join('')); + expect(tagStore.get('citation_abstract_html_url')[0].content).toEqual(new URLCombiner(environment.ui.baseUrl, router.url).toString()); expect(tagStore.get('citation_pdf_url')[0].content).toEqual('https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreams/99b00f3c-1cc6-4689-8158-91965bee6b28/content'); })); diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index 02d2b0c86b..90171bac10 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -1,4 +1,4 @@ -import { Inject, Injectable } from '@angular/core'; +import { Injectable } from '@angular/core'; import { Meta, MetaDefinition, Title } from '@angular/platform-browser'; import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; @@ -20,7 +20,8 @@ import { Bitstream } from '../shared/bitstream.model'; import { DSpaceObject } from '../shared/dspace-object.model'; import { Item } from '../shared/item.model'; import { getFirstSucceededRemoteDataPayload, getFirstSucceededRemoteListPayload } from '../shared/operators'; -import { environment } from '../../../environments/environment'; +import { HardRedirectService } from '../services/hard-redirect.service'; +import { URLCombiner } from '../url-combiner/url-combiner'; @Injectable() export class MetadataService { @@ -39,6 +40,7 @@ export class MetadataService { private dsoNameService: DSONameService, private bitstreamDataService: BitstreamDataService, private bitstreamFormatDataService: BitstreamFormatDataService, + private redirectService: HardRedirectService ) { // TODO: determine what open graph meta tags are needed and whether // the differ per route. potentially add image based on DSpaceObject @@ -254,7 +256,7 @@ export class MetadataService { */ private setCitationAbstractUrlTag(): void { if (this.currentObject.value instanceof Item) { - const value = [environment.ui.baseUrl, this.router.url].join(''); + const value = new URLCombiner(this.redirectService.getRequestOrigin(), this.router.url).toString(); this.addMetaTag('citation_abstract_html_url', value); } } @@ -279,7 +281,8 @@ export class MetadataService { getFirstSucceededRemoteDataPayload() ).subscribe((format: BitstreamFormat) => { if (format.mimetype === 'application/pdf') { - this.addMetaTag('citation_pdf_url', bitstream._links.content.href); + const rewrittenURL= this.redirectService.rewriteDownloadURL(bitstream._links.content.href); + this.addMetaTag('citation_pdf_url', rewrittenURL); } }); } 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 9d4c5df9a2..64518579aa 100644 --- a/src/app/core/services/browser-hard-redirect.service.spec.ts +++ b/src/app/core/services/browser-hard-redirect.service.spec.ts @@ -2,11 +2,12 @@ import {TestBed} from '@angular/core/testing'; import {BrowserHardRedirectService} from './browser-hard-redirect.service'; describe('BrowserHardRedirectService', () => { - + const origin = 'test origin'; const mockLocation = { href: undefined, pathname: '/pathname', search: '/search', + origin } as Location; const service: BrowserHardRedirectService = new BrowserHardRedirectService(mockLocation); @@ -38,4 +39,12 @@ describe('BrowserHardRedirectService', () => { expect(service.getCurrentRoute()).toEqual(mockLocation.pathname + mockLocation.search); }); }); + + describe('when requesting the origin', () => { + + it('should return the location origin', () => { + expect(service.getRequestOrigin()).toEqual(origin); + }); + }); + }); diff --git a/src/app/core/services/browser-hard-redirect.service.ts b/src/app/core/services/browser-hard-redirect.service.ts index 0d14b6b834..4b7424bee2 100644 --- a/src/app/core/services/browser-hard-redirect.service.ts +++ b/src/app/core/services/browser-hard-redirect.service.ts @@ -11,11 +11,12 @@ export function locationProvider(): Location { * Service for performing hard redirects within the browser app module */ @Injectable() -export class BrowserHardRedirectService implements HardRedirectService { +export class BrowserHardRedirectService extends HardRedirectService { constructor( @Inject(LocationToken) protected location: Location, ) { + super(); } /** @@ -32,4 +33,11 @@ export class BrowserHardRedirectService implements HardRedirectService { getCurrentRoute() { return this.location.pathname + this.location.search; } + + /** + * Get the hostname of the request + */ + getRequestOrigin() { + return this.location.origin; + } } diff --git a/src/app/core/services/hard-redirect.service.spec.ts b/src/app/core/services/hard-redirect.service.spec.ts new file mode 100644 index 0000000000..f7e2ba8955 --- /dev/null +++ b/src/app/core/services/hard-redirect.service.spec.ts @@ -0,0 +1,57 @@ +import { HardRedirectService } from './hard-redirect.service'; +import { environment } from '../../../environments/environment'; +import { TestBed } from '@angular/core/testing'; +import { Injectable } from '@angular/core'; + +const requestOrigin = 'http://dspace-angular-ui.dspace.com'; + +describe('HardRedirectService', () => { + let service: TestHardRedirectService; + + beforeEach(() => { + TestBed.configureTestingModule({ providers: [TestHardRedirectService] }); + service = TestBed.get(TestHardRedirectService); + }); + + describe('when calling rewriteDownloadURL', () => { + let originalValue; + const relativePath = '/test/url/path'; + const testURL = environment.rest.baseUrl + relativePath; + beforeEach(() => { + originalValue = environment.rewriteDownloadUrls; + }); + + it('it should return the same url when rewriteDownloadURL is false', () => { + environment.rewriteDownloadUrls = false; + expect(service.rewriteDownloadURL(testURL)).toEqual(testURL); + }); + + it('it should replace part of the url when rewriteDownloadURL is true', () => { + environment.rewriteDownloadUrls = true; + expect(service.rewriteDownloadURL(testURL)).toEqual(requestOrigin + environment.rest.nameSpace + relativePath); + }); + + afterEach(() => { + environment.rewriteDownloadUrls = originalValue; + }) + }); +}); + +@Injectable() +class TestHardRedirectService extends HardRedirectService { + constructor() { + super(); + } + + redirect(url: string) { + return undefined; + } + + getCurrentRoute() { + return undefined; + } + + getRequestOrigin() { + return requestOrigin; + } +} diff --git a/src/app/core/services/hard-redirect.service.ts b/src/app/core/services/hard-redirect.service.ts index 09757a1250..a09521dae5 100644 --- a/src/app/core/services/hard-redirect.service.ts +++ b/src/app/core/services/hard-redirect.service.ts @@ -1,4 +1,6 @@ import { Injectable } from '@angular/core'; +import { environment } from '../../../environments/environment'; +import { URLCombiner } from '../url-combiner/url-combiner'; /** * Service to take care of hard redirects @@ -19,4 +21,20 @@ export abstract class HardRedirectService { * e.g. /search?page=1&query=open%20access&f.dateIssued.min=1980&f.dateIssued.max=2020 */ abstract getCurrentRoute(); + + /** + * Get the hostname of the request + */ + abstract getRequestOrigin(); + + public rewriteDownloadURL(originalUrl: string): string { + if (environment.rewriteDownloadUrls) { + const hostName = this.getRequestOrigin(); + const namespace = environment.rest.nameSpace; + const rewrittenUrl = new URLCombiner(hostName, namespace).toString(); + return originalUrl.replace(environment.rest.baseUrl, rewrittenUrl); + } else { + return originalUrl; + } + } } 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 2d09c21eb9..dc89517468 100644 --- a/src/app/core/services/server-hard-redirect.service.spec.ts +++ b/src/app/core/services/server-hard-redirect.service.spec.ts @@ -7,8 +7,13 @@ describe('ServerHardRedirectService', () => { const mockResponse = jasmine.createSpyObj(['redirect', 'end']); const service: ServerHardRedirectService = new ServerHardRedirectService(mockRequest, mockResponse); + const origin = 'test-host'; beforeEach(() => { + mockRequest.headers = { + host: 'test-host', + }; + TestBed.configureTestingModule({}); }); @@ -40,4 +45,12 @@ describe('ServerHardRedirectService', () => { expect(service.getCurrentRoute()).toEqual(mockRequest.originalUrl); }); }); + + describe('when requesting the origin', () => { + + it('should return the location origin', () => { + expect(service.getRequestOrigin()).toEqual(origin); + }); + }); + }); diff --git a/src/app/core/services/server-hard-redirect.service.ts b/src/app/core/services/server-hard-redirect.service.ts index 79755d2dc9..65b404ca6c 100644 --- a/src/app/core/services/server-hard-redirect.service.ts +++ b/src/app/core/services/server-hard-redirect.service.ts @@ -7,12 +7,13 @@ import { HardRedirectService } from './hard-redirect.service'; * Service for performing hard redirects within the server app module */ @Injectable() -export class ServerHardRedirectService implements HardRedirectService { +export class ServerHardRedirectService extends HardRedirectService { constructor( @Inject(REQUEST) protected req: Request, @Inject(RESPONSE) protected res: Response, ) { + super(); } /** @@ -59,4 +60,11 @@ export class ServerHardRedirectService implements HardRedirectService { getCurrentRoute() { return this.req.originalUrl; } + + /** + * Get the hostname of the request + */ + getRequestOrigin() { + return this.req.headers.host; + } } diff --git a/src/app/shared/file-download-link/file-download-link.component.spec.ts b/src/app/shared/file-download-link/file-download-link.component.spec.ts index ac1751d43d..56702787d3 100644 --- a/src/app/shared/file-download-link/file-download-link.component.spec.ts +++ b/src/app/shared/file-download-link/file-download-link.component.spec.ts @@ -3,6 +3,7 @@ import { FileDownloadLinkComponent } from './file-download-link.component'; import { AuthService } from '../../core/auth/auth.service'; import { FileService } from '../../core/shared/file.service'; import { of as observableOf } from 'rxjs'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; describe('FileDownloadLinkComponent', () => { let component: FileDownloadLinkComponent; @@ -23,13 +24,14 @@ describe('FileDownloadLinkComponent', () => { beforeEach(async(() => { init(); TestBed.configureTestingModule({ - declarations: [ FileDownloadLinkComponent ], + declarations: [FileDownloadLinkComponent], providers: [ { provide: AuthService, useValue: authService }, - { provide: FileService, useValue: fileService } + { provide: FileService, useValue: fileService }, + { provide: HardRedirectService, useValue: { rewriteDownloadURL: (a) => a } }, ] }) - .compileComponents(); + .compileComponents(); })); beforeEach(() => { diff --git a/src/app/shared/file-download-link/file-download-link.component.ts b/src/app/shared/file-download-link/file-download-link.component.ts index 9df7c191ff..71283642c9 100644 --- a/src/app/shared/file-download-link/file-download-link.component.ts +++ b/src/app/shared/file-download-link/file-download-link.component.ts @@ -2,6 +2,7 @@ import { Component, Input, OnInit } from '@angular/core'; import { FileService } from '../../core/shared/file.service'; import { Observable } from 'rxjs/internal/Observable'; import { AuthService } from '../../core/auth/auth.service'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; @Component({ selector: 'ds-file-download-link', @@ -30,10 +31,13 @@ export class FileDownloadLinkComponent implements OnInit { isAuthenticated$: Observable; constructor(private fileService: FileService, - private authService: AuthService) { } + private authService: AuthService, + private redirectService: HardRedirectService) { + } ngOnInit() { this.isAuthenticated$ = this.authService.isAuthenticated(); + this.href = this.redirectService.rewriteDownloadURL(this.href); } /** @@ -44,5 +48,4 @@ export class FileDownloadLinkComponent implements OnInit { this.fileService.downloadFile(this.href); return false; } - } diff --git a/src/config/global-config.interface.ts b/src/config/global-config.interface.ts index acef3404eb..07ee4ca444 100644 --- a/src/config/global-config.interface.ts +++ b/src/config/global-config.interface.ts @@ -31,4 +31,5 @@ export interface GlobalConfig extends Config { item: ItemPageConfig; collection: CollectionPageConfig; theme: Theme; + rewriteDownloadUrls: boolean; } diff --git a/src/environments/environment.common.ts b/src/environments/environment.common.ts index 32ae2f54b0..32e6097976 100644 --- a/src/environments/environment.common.ts +++ b/src/environments/environment.common.ts @@ -216,4 +216,6 @@ export const environment: GlobalConfig = { theme: { name: 'default', }, + // Whether the UI should rewrite file download URLs to match its domain. Only necessary to enable when running UI and REST API on separate domains + rewriteDownloadUrls: false, };