diff --git a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts index 0c8c64a470..02de06f415 100644 --- a/src/app/access-control/group-registry/group-form/group-form.component.spec.ts +++ b/src/app/access-control/group-registry/group-form/group-form.component.spec.ts @@ -53,6 +53,7 @@ import { HALEndpointService } from '../../../core/shared/hal-endpoint.service'; import { NoContent } from '../../../core/shared/NoContent.model'; import { PageInfo } from '../../../core/shared/page-info.model'; import { UUIDService } from '../../../core/shared/uuid.service'; +import { XSRFService } from '../../../core/xsrf/xsrf.service'; import { AlertComponent } from '../../../shared/alert/alert.component'; import { ContextHelpDirective } from '../../../shared/context-help.directive'; import { FormBuilderService } from '../../../shared/form/builder/form-builder.service'; @@ -244,6 +245,7 @@ describe('GroupFormComponent', () => { { provide: HttpClient, useValue: {} }, { provide: ObjectCacheService, useValue: {} }, { provide: UUIDService, useValue: {} }, + { provide: XSRFService, useValue: {} }, { provide: Store, useValue: {} }, { provide: RemoteDataBuildService, useValue: {} }, { provide: HALEndpointService, useValue: {} }, diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index 76e70e8a6d..92442854ba 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -16,6 +16,7 @@ import { getTestScheduler, } from 'jasmine-marbles'; import { + BehaviorSubject, EMPTY, Observable, of as observableOf, @@ -32,6 +33,7 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { coreReducers } from '../core.reducers'; import { CoreState } from '../core-state.model'; import { UUIDService } from '../shared/uuid.service'; +import { XSRFService } from '../xsrf/xsrf.service'; import { RequestConfigureAction, RequestExecuteAction, @@ -59,6 +61,7 @@ describe('RequestService', () => { let uuidService: UUIDService; let store: Store; let mockStore: MockStore; + let xsrfService: XSRFService; const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb'; const testHref = 'https://rest.api/endpoint/selfLink'; @@ -104,10 +107,15 @@ describe('RequestService', () => { store = TestBed.inject(Store); mockStore = store as MockStore; mockStore.setState(initialState); + xsrfService = { + tokenInitialized$: new BehaviorSubject(false), + } as XSRFService; + service = new RequestService( objectCache, uuidService, store, + xsrfService, undefined, ); serviceAsAny = service as any; diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 9bd262b1ad..6ce37d3545 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -42,6 +42,7 @@ import { requestIndexSelector, } from '../index/index.selectors'; import { UUIDService } from '../shared/uuid.service'; +import { XSRFService } from '../xsrf/xsrf.service'; import { RequestConfigureAction, RequestExecuteAction, @@ -168,6 +169,7 @@ export class RequestService { constructor(private objectCache: ObjectCacheService, private uuidService: UUIDService, private store: Store, + protected xsrfService: XSRFService, private indexStore: Store) { } @@ -450,7 +452,17 @@ export class RequestService { */ private dispatchRequest(request: RestRequest) { this.store.dispatch(new RequestConfigureAction(request)); - this.store.dispatch(new RequestExecuteAction(request.uuid)); + // If it's a GET request, or we have an XSRF token, dispatch it immediately + if (request.method === RestRequestMethod.GET || this.xsrfService.tokenInitialized$.getValue() === true) { + this.store.dispatch(new RequestExecuteAction(request.uuid)); + } else { + // Otherwise wait for the XSRF token first + this.xsrfService.tokenInitialized$.pipe( + find((hasInitialized: boolean) => hasInitialized === true), + ).subscribe(() => { + this.store.dispatch(new RequestExecuteAction(request.uuid)); + }); + } } /** diff --git a/src/app/core/xsrf/browser-xsrf.service.spec.ts b/src/app/core/xsrf/browser-xsrf.service.spec.ts new file mode 100644 index 0000000000..378df0e46b --- /dev/null +++ b/src/app/core/xsrf/browser-xsrf.service.spec.ts @@ -0,0 +1,64 @@ +import { BrowserXSRFService } from './browser-xsrf.service'; +import { HttpClient } from '@angular/common/http'; +import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; +import { TestBed } from '@angular/core/testing'; +import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; + +describe(`BrowserXSRFService`, () => { + let service: BrowserXSRFService; + let httpClient: HttpClient; + let httpTestingController: HttpTestingController; + + const endpointURL = new RESTURLCombiner('/security/csrf').toString(); + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [ HttpClientTestingModule ], + providers: [ BrowserXSRFService ] + }); + httpClient = TestBed.inject(HttpClient); + httpTestingController = TestBed.inject(HttpTestingController); + service = TestBed.inject(BrowserXSRFService); + }); + + describe(`initXSRFToken`, () => { + it(`should perform a POST to the csrf endpoint`, () => { + service.initXSRFToken(httpClient)(); + + const req = httpTestingController.expectOne({ + url: endpointURL, + method: 'POST' + }); + + req.flush({}); + httpTestingController.verify(); + }); + + describe(`when the POST succeeds`, () => { + it(`should set tokenInitialized$ to true`, () => { + service.initXSRFToken(httpClient)(); + + const req = httpTestingController.expectOne(endpointURL); + + req.flush({}); + httpTestingController.verify(); + + expect(service.tokenInitialized$.getValue()).toBeTrue(); + }); + }); + + describe(`when the POST fails`, () => { + it(`should set tokenInitialized$ to true`, () => { + service.initXSRFToken(httpClient)(); + + const req = httpTestingController.expectOne(endpointURL); + + req.error(new ErrorEvent('415')); + httpTestingController.verify(); + + expect(service.tokenInitialized$.getValue()).toBeTrue(); + }); + }); + + }); +}); diff --git a/src/app/core/xsrf/browser-xsrf.service.ts b/src/app/core/xsrf/browser-xsrf.service.ts new file mode 100644 index 0000000000..271e8dca47 --- /dev/null +++ b/src/app/core/xsrf/browser-xsrf.service.ts @@ -0,0 +1,26 @@ +import { HttpClient } from '@angular/common/http'; +import { Injectable } from '@angular/core'; +import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; +import { take, catchError } from 'rxjs/operators'; +import { of as observableOf } from 'rxjs'; +import { XSRFService } from './xsrf.service'; + +@Injectable() +export class BrowserXSRFService extends XSRFService { + initXSRFToken(httpClient: HttpClient): () => Promise { + return () => new Promise((resolve) => { + httpClient.post(new RESTURLCombiner('/security/csrf').toString(), undefined).pipe( + // errors are to be expected if the token and the cookie don't match, that's what we're + // trying to fix for future requests, so just emit any observable to end up in the + // subscribe + catchError(() => observableOf(null)), + take(1), + ).subscribe(() => { + this.tokenInitialized$.next(true); + }); + + // return immediately, the rest of the app doesn't need to wait for this to finish + resolve(); + }); + } +} diff --git a/src/app/core/xsrf/server-xsrf.service.spec.ts b/src/app/core/xsrf/server-xsrf.service.spec.ts new file mode 100644 index 0000000000..b2ace67dd0 --- /dev/null +++ b/src/app/core/xsrf/server-xsrf.service.spec.ts @@ -0,0 +1,32 @@ +import { ServerXSRFService } from './server-xsrf.service'; +import { HttpClient } from '@angular/common/http'; + +describe(`ServerXSRFService`, () => { + let service: ServerXSRFService; + let httpClient: HttpClient; + + beforeEach(() => { + httpClient = jasmine.createSpyObj(['post', 'get', 'request']); + service = new ServerXSRFService(); + }); + + describe(`initXSRFToken`, () => { + it(`shouldn't perform any requests`, (done: DoneFn) => { + service.initXSRFToken(httpClient)().then(() => { + for (const prop in httpClient) { + if (httpClient.hasOwnProperty(prop)) { + expect(httpClient[prop]).not.toHaveBeenCalled(); + } + } + done(); + }); + }); + + it(`should leave tokenInitialized$ on false`, (done: DoneFn) => { + service.initXSRFToken(httpClient)().then(() => { + expect(service.tokenInitialized$.getValue()).toBeFalse(); + done(); + }); + }); + }); +}); diff --git a/src/app/core/xsrf/server-xsrf.service.ts b/src/app/core/xsrf/server-xsrf.service.ts new file mode 100644 index 0000000000..1577893f95 --- /dev/null +++ b/src/app/core/xsrf/server-xsrf.service.ts @@ -0,0 +1,14 @@ +import { HttpClient } from '@angular/common/http'; +import { Injectable } from '@angular/core'; +import { XSRFService } from './xsrf.service'; + +@Injectable() +export class ServerXSRFService extends XSRFService { + initXSRFToken(httpClient: HttpClient): () => Promise { + return () => new Promise((resolve) => { + // return immediately, and keep tokenInitialized$ false. The server side can make only GET + // requests, since it can never get a valid XSRF cookie + resolve(); + }); + } +} diff --git a/src/app/core/xsrf/xsrf.service.spec.ts b/src/app/core/xsrf/xsrf.service.spec.ts new file mode 100644 index 0000000000..a7c5c01cb7 --- /dev/null +++ b/src/app/core/xsrf/xsrf.service.spec.ts @@ -0,0 +1,20 @@ +import { XSRFService } from './xsrf.service'; +import { HttpClient } from '@angular/common/http'; + +class XSRFServiceImpl extends XSRFService { + initXSRFToken(httpClient: HttpClient): () => Promise { + return () => null; + } +} + +describe(`XSRFService`, () => { + let service: XSRFService; + + beforeEach(() => { + service = new XSRFServiceImpl(); + }); + + it(`should start with tokenInitialized$.hasValue() === false`, () => { + expect(service.tokenInitialized$.getValue()).toBeFalse(); + }); +}); diff --git a/src/app/core/xsrf/xsrf.service.ts b/src/app/core/xsrf/xsrf.service.ts new file mode 100644 index 0000000000..fb8dfe74b3 --- /dev/null +++ b/src/app/core/xsrf/xsrf.service.ts @@ -0,0 +1,10 @@ +import { HttpClient } from '@angular/common/http'; +import { Injectable } from '@angular/core'; +import { BehaviorSubject } from 'rxjs'; + +@Injectable() +export abstract class XSRFService { + public tokenInitialized$: BehaviorSubject = new BehaviorSubject(false); + + abstract initXSRFToken(httpClient: HttpClient): () => Promise; +} diff --git a/src/modules/app/browser-app.module.ts b/src/modules/app/browser-app.module.ts index cb8431beb8..48b40dcddb 100644 --- a/src/modules/app/browser-app.module.ts +++ b/src/modules/app/browser-app.module.ts @@ -2,7 +2,10 @@ import { HttpClient, HttpClientModule, } from '@angular/common/http'; -import { NgModule } from '@angular/core'; +import { + APP_INITIALIZER, + NgModule, +} from '@angular/core'; import { BrowserModule, BrowserTransferStateModule, @@ -48,13 +51,15 @@ import { ClientCookieService } from '../../app/core/services/client-cookie.servi import { CookieService } from '../../app/core/services/cookie.service'; import { HardRedirectService } from '../../app/core/services/hard-redirect.service'; import { ReferrerService } from '../../app/core/services/referrer.service'; +import { BrowserXSRFService } from '../../app/core/xsrf/browser-xsrf.service'; +import { XSRFService } from '../../app/core/xsrf/xsrf.service'; import { BrowserKlaroService } from '../../app/shared/cookies/browser-klaro.service'; import { KlaroService } from '../../app/shared/cookies/klaro.service'; import { MissingTranslationHelper } from '../../app/shared/translate/missing-translation.helper'; import { GoogleAnalyticsService } from '../../app/statistics/google-analytics.service'; import { SubmissionService } from '../../app/submission/submission.service'; import { TranslateBrowserLoader } from '../../ngx-translate-loaders/translate-browser.loader'; -import { BrowserInitService } from './browser-init.service'; +import { BrowserInitService } from './browser-init.service' export const REQ_KEY = makeStateKey('req'); @@ -98,6 +103,16 @@ export function getRequest(transferState: TransferState): any { useFactory: getRequest, deps: [TransferState], }, + { + provide: APP_INITIALIZER, + useFactory: (xsrfService: XSRFService, httpClient: HttpClient) => xsrfService.initXSRFToken(httpClient), + deps: [ XSRFService, HttpClient ], + multi: true, + }, + { + provide: XSRFService, + useClass: BrowserXSRFService, + }, { provide: AuthService, useClass: AuthService, diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index 4de5687d04..20db68c671 100644 --- a/src/modules/app/server-app.module.ts +++ b/src/modules/app/server-app.module.ts @@ -46,6 +46,8 @@ import { ServerReferrerService } from '../../app/core/services/server.referrer.s import { ServerCookieService } from '../../app/core/services/server-cookie.service'; import { ServerHardRedirectService } from '../../app/core/services/server-hard-redirect.service'; import { ServerXhrService } from '../../app/core/services/server-xhr.service'; +import { ServerXSRFService } from '../../app/core/xsrf/server-xsrf.service'; +import { XSRFService } from '../../app/core/xsrf/xsrf.service'; import { AngularticsProviderMock } from '../../app/shared/mocks/angulartics-provider.service.mock'; import { Angulartics2Mock } from '../../app/shared/mocks/angulartics2.service.mock'; import { Angulartics2DSpace } from '../../app/statistics/angulartics/dspace-provider'; @@ -112,6 +114,10 @@ export function createTranslateLoader(transferState: TransferState) { provide: AuthRequestService, useClass: ServerAuthRequestService, }, + { + provide: XSRFService, + useClass: ServerXSRFService, + }, { provide: LocaleService, useClass: ServerLocaleService,