Merge pull request DSpace#3063 from tdonohue/port_new_csrf_fixes_to_7x

[Port dspace-7_x] Ensures CSRF token is initialized prior to first modifying (non-GET) request

Author: tdonohue

.github/workflows/build.yml

@@ -33,7 +33,7 @@ jobs:
       #CHROME_VERSION: "90.0.4430.212-1"
       # Bump Node heap size (OOM in CI after upgrading to Angular 15)
       NODE_OPTIONS: '--max-old-space-size=4096'
-      # Project name to use when running docker-compose prior to e2e tests
+      # Project name to use when running "docker compose" prior to e2e tests
       COMPOSE_PROJECT_NAME: 'ci'
     strategy:
       # Create a matrix of Node versions to test against (in parallel)
@@ -108,12 +108,12 @@ jobs:
           path: 'coverage/dspace-angular/lcov.info'
           retention-days: 14
 
-      # Using docker-compose start backend using CI configuration
+      # Using "docker compose" start backend using CI configuration
       # and load assetstore from a cached copy
       - name: Start DSpace REST Backend via Docker (for e2e tests)
         run: |
-          docker-compose -f ./docker/docker-compose-ci.yml up -d
-          docker-compose -f ./docker/cli.yml -f ./docker/cli.assetstore.yml run --rm dspace-cli
+          docker compose -f ./docker/docker-compose-ci.yml up -d
+          docker compose -f ./docker/cli.yml -f ./docker/cli.assetstore.yml run --rm dspace-cli
           docker container ls
 
       # Run integration tests via Cypress.io
@@ -182,7 +182,7 @@ jobs:
         run: kill -9 $(lsof -t -i:4000)
 
       - name: Shutdown Docker containers
-        run: docker-compose -f ./docker/docker-compose-ci.yml down
+        run: docker compose -f ./docker/docker-compose-ci.yml down
 
   # Codecov upload is a separate job in order to allow us to restart this separate from the entire build/test
   # job above. This is necessary because Codecov uploads seem to randomly fail at times.

src/app/access-control/group-registry/group-form/group-form.component.spec.ts

@@ -23,6 +23,7 @@ import { DSpaceObject } from '../../../core/shared/dspace-object.model';
 import { HALEndpointService } from '../../../core/shared/hal-endpoint.service';
 import { PageInfo } from '../../../core/shared/page-info.model';
 import { UUIDService } from '../../../core/shared/uuid.service';
+import { XSRFService } from '../../../core/xsrf/xsrf.service';
 import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { GroupMock, GroupMock2 } from '../../../shared/testing/group-mock';
@@ -211,6 +212,7 @@ describe('GroupFormComponent', () => {
         { provide: HttpClient, useValue: {} },
         { provide: ObjectCacheService, useValue: {} },
         { provide: UUIDService, useValue: {} },
+        { provide: XSRFService, useValue: {} },
         { provide: Store, useValue: {} },
         { provide: RemoteDataBuildService, useValue: {} },
         { provide: HALEndpointService, useValue: {} },

src/app/admin/admin-registries/bitstream-formats/bitstream-formats.component.spec.ts

@@ -15,6 +15,7 @@ import { NotificationsService } from '../../../shared/notifications/notification
 import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub';
 import { BitstreamFormat } from '../../../core/shared/bitstream-format.model';
 import { BitstreamFormatSupportLevel } from '../../../core/shared/bitstream-format-support-level';
+import { XSRFService } from '../../../core/xsrf/xsrf.service';
 import { cold, getTestScheduler, hot } from 'jasmine-marbles';
 import { TestScheduler } from 'rxjs/testing';
 import {
@@ -108,7 +109,8 @@ describe('BitstreamFormatsComponent', () => {
         { provide: BitstreamFormatDataService, useValue: bitstreamFormatService },
         { provide: HostWindowService, useValue: new HostWindowServiceStub(0) },
         { provide: NotificationsService, useValue: notificationsServiceStub },
-        { provide: PaginationService, useValue: paginationService }
+        { provide: PaginationService, useValue: paginationService },
+        { provide: XSRFService, useValue: {} },
       ]
     }).compileComponents();
   };

src/app/admin/admin-workflow-page/admin-workflow-search-results/admin-workflow-search-result-list-element/workflow-item/workflow-item-search-result-admin-workflow-list-element.component.spec.ts

@@ -9,6 +9,7 @@ import { CollectionElementLinkType } from '../../../../../shared/object-collecti
 import { ViewMode } from '../../../../../core/shared/view-mode.model';
 import { RouterTestingModule } from '@angular/router/testing';
 import { WorkflowItem } from '../../../../../core/submission/models/workflowitem.model';
+import { XSRFService } from '../../../../../core/xsrf/xsrf.service';
 import {
   WorkflowItemSearchResultAdminWorkflowListElementComponent
 } from './workflow-item-search-result-admin-workflow-list-element.component';
@@ -58,7 +59,8 @@ describe('WorkflowItemSearchResultAdminWorkflowListElementComponent', () => {
           { provide: TruncatableService, useValue: mockTruncatableService },
           { provide: LinkService, useValue: linkService },
           { provide: DSONameService, useClass: DSONameServiceMock },
-          { provide: APP_CONFIG, useValue: environment }
+          { provide: APP_CONFIG, useValue: environment },
+          { provide: XSRFService, useValue: {} },
         ],
         schemas: [NO_ERRORS_SCHEMA]
       })

src/app/core/data/request.effects.ts

@@ -1,14 +1,15 @@
 import { Injectable, Injector } from '@angular/core';
 
 import { Actions, createEffect, ofType } from '@ngrx/effects';
-import { catchError, filter, map, mergeMap, take } from 'rxjs/operators';
+import { catchError, filter, map, mergeMap, take, withLatestFrom } from 'rxjs/operators';
 
 import { hasValue, isNotEmpty } from '../../shared/empty.util';
 import { StoreActionTypes } from '../../store.actions';
 import { getClassForType } from '../cache/builders/build-decorators';
 import { RawRestResponse } from '../dspace-rest/raw-rest-response.model';
 import { DspaceRestService } from '../dspace-rest/dspace-rest.service';
 import { DSpaceSerializer } from '../dspace-rest/dspace.serializer';
+import { XSRFService } from '../xsrf/xsrf.service';
 import {
   RequestActionTypes,
   RequestErrorAction,
@@ -19,6 +20,7 @@ import {
 import { RequestService } from './request.service';
 import { ParsedResponse } from '../cache/response.models';
 import { RequestError } from './request-error.model';
+import { RestRequestMethod } from './rest-request-method';
 import { RestRequestWithResponseParser } from './rest-request-with-response-parser.model';
 import { RequestEntry } from './request-entry.model';
 
@@ -33,7 +35,11 @@ export class RequestEffects {
       );
     }),
     filter((entry: RequestEntry) => hasValue(entry)),
-    map((entry: RequestEntry) => entry.request),
+    withLatestFrom(this.xsrfService.tokenInitialized$),
+    // If it's a GET request, or we have an XSRF token, dispatch it immediately
+    // Otherwise wait for the XSRF token first
+    filter(([entry, tokenInitialized]: [RequestEntry, boolean]) => entry.request.method === RestRequestMethod.GET || tokenInitialized === true),
+    map(([entry, tokenInitialized]: [RequestEntry, boolean]) => entry.request),
     mergeMap((request: RestRequestWithResponseParser) => {
       let body = request.body;
       if (isNotEmpty(request.body) && !request.isMultipart) {
@@ -73,7 +79,8 @@ export class RequestEffects {
     private actions$: Actions,
     private restApi: DspaceRestService,
     private injector: Injector,
-    protected requestService: RequestService
+    protected requestService: RequestService,
+    protected xsrfService: XSRFService,
   ) { }
 
 }

src/app/core/data/request.service.spec.ts

@@ -80,11 +80,11 @@ describe('RequestService', () => {
     store = TestBed.inject(Store);
     mockStore = store as MockStore<CoreState>;
     mockStore.setState(initialState);
+
     service = new RequestService(
       objectCache,
       uuidService,
       store,
-      undefined
     );
     serviceAsAny = service as any;
   });

src/app/core/data/request.service.ts

@@ -8,7 +8,7 @@ import cloneDeep from 'lodash/cloneDeep';
 import { hasValue, isEmpty, isNotEmpty, hasNoValue } from '../../shared/empty.util';
 import { ObjectCacheEntry } from '../cache/object-cache.reducer';
 import { ObjectCacheService } from '../cache/object-cache.service';
-import { IndexState, MetaIndexState } from '../index/index.reducer';
+import { IndexState } from '../index/index.reducer';
 import { requestIndexSelector, getUrlWithoutEmbedParams } from '../index/index.selectors';
 import { UUIDService } from '../shared/uuid.service';
 import {
@@ -136,8 +136,7 @@ export class RequestService {
 
   constructor(private objectCache: ObjectCacheService,
               private uuidService: UUIDService,
-              private store: Store<CoreState>,
-              private indexStore: Store<MetaIndexState>) {
+              private store: Store<CoreState>) {
   }
 
   generateRequestId(): string {

src/app/core/xsrf/browser-xsrf.service.spec.ts

@@ -0,0 +1,58 @@
+import { HttpClient } from '@angular/common/http';
+import {
+  HttpClientTestingModule,
+  HttpTestingController,
+} from '@angular/common/http/testing';
+import { TestBed } from '@angular/core/testing';
+
+import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
+import { BrowserXSRFService } from './browser-xsrf.service';
+
+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 GET to the csrf endpoint`, (done: DoneFn) => {
+      service.initXSRFToken(httpClient)();
+
+      const req = httpTestingController.expectOne({
+        url: endpointURL,
+        method: 'GET',
+      });
+
+      req.flush({});
+      httpTestingController.verify();
+      expect().nothing();
+      done();
+    });
+
+    describe(`when the GET succeeds`, () => {
+      it(`should set tokenInitialized$ to true`, (done: DoneFn) => {
+        service.initXSRFToken(httpClient)();
+
+        const req = httpTestingController.expectOne(endpointURL);
+
+        req.flush({});
+        httpTestingController.verify();
+
+        expect(service.tokenInitialized$.getValue()).toBeTrue();
+        done();
+      });
+    });
+
+  });
+});

src/app/core/xsrf/browser-xsrf.service.ts

@@ -0,0 +1,30 @@
+import { HttpClient } from '@angular/common/http';
+import { Injectable } from '@angular/core';
+import { take } from 'rxjs/operators';
+
+import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
+import { XSRFService } from './xsrf.service';
+
+/**
+ * Browser (CSR) Service to obtain a new CSRF/XSRF token when needed by our RequestService
+ * to perform a modify request (e.g. POST/PUT/DELETE).
+ * NOTE: This is primarily necessary before the *first* modifying request, as the CSRF
+ * token may not yet be initialized.
+ */
+@Injectable()
+export class BrowserXSRFService extends XSRFService {
+  initXSRFToken(httpClient: HttpClient): () => Promise<any> {
+    return () => new Promise<void>((resolve) => {
+      // Force a new token to be created by calling the CSRF endpoint
+      httpClient.get(new RESTURLCombiner('/security/csrf').toString(), undefined).pipe(
+        take(1),
+      ).subscribe(() => {
+        // Once token is returned, set tokenInitialized to true.
+        this.tokenInitialized$.next(true);
+      });
+
+      // return immediately, the rest of the app doesn't need to wait for this to finish
+      resolve();
+    });
+  }
+}

src/app/core/xsrf/server-xsrf.service.spec.ts

@@ -0,0 +1,33 @@
+import { HttpClient } from '@angular/common/http';
+
+import { ServerXSRFService } from './server-xsrf.service';
+
+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();
+      });
+    });
+  });
+});