Skip to content

Commit 7d5e499

Browse files
artloweltdonohue
authored andcommitted
make a call to ensure a correct XSRF token before performing any non-GET requests
1 parent cbee67c commit 7d5e499

11 files changed

Lines changed: 208 additions & 3 deletions

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { DSpaceObject } from '../../../core/shared/dspace-object.model';
2323
import { HALEndpointService } from '../../../core/shared/hal-endpoint.service';
2424
import { PageInfo } from '../../../core/shared/page-info.model';
2525
import { UUIDService } from '../../../core/shared/uuid.service';
26+
import { XSRFService } from '../../../core/xsrf/xsrf.service';
2627
import { FormBuilderService } from '../../../shared/form/builder/form-builder.service';
2728
import { NotificationsService } from '../../../shared/notifications/notifications.service';
2829
import { GroupMock, GroupMock2 } from '../../../shared/testing/group-mock';
@@ -211,6 +212,7 @@ describe('GroupFormComponent', () => {
211212
{ provide: HttpClient, useValue: {} },
212213
{ provide: ObjectCacheService, useValue: {} },
213214
{ provide: UUIDService, useValue: {} },
215+
{ provide: XSRFService, useValue: {} },
214216
{ provide: Store, useValue: {} },
215217
{ provide: RemoteDataBuildService, useValue: {} },
216218
{ provide: HALEndpointService, useValue: {} },

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { Store, StoreModule } from '@ngrx/store';
22
import { cold, getTestScheduler } from 'jasmine-marbles';
3-
import { EMPTY, Observable, of as observableOf } from 'rxjs';
3+
import { BehaviorSubject, EMPTY, Observable, of as observableOf } from 'rxjs';
44
import { TestScheduler } from 'rxjs/testing';
55

66
import { getMockObjectCacheService } from '../../shared/mocks/object-cache.service.mock';
77
import { defaultUUID, getMockUUIDService } from '../../shared/mocks/uuid.service.mock';
88
import { ObjectCacheService } from '../cache/object-cache.service';
99
import { coreReducers} from '../core.reducers';
1010
import { UUIDService } from '../shared/uuid.service';
11+
import { XSRFService } from '../xsrf/xsrf.service';
1112
import { RequestConfigureAction, RequestExecuteAction, RequestStaleAction } from './request.actions';
1213
import {
1314
DeleteRequest,
@@ -35,6 +36,7 @@ describe('RequestService', () => {
3536
let uuidService: UUIDService;
3637
let store: Store<CoreState>;
3738
let mockStore: MockStore<CoreState>;
39+
let xsrfService: XSRFService;
3840

3941
const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb';
4042
const testHref = 'https://rest.api/endpoint/selfLink';
@@ -80,10 +82,15 @@ describe('RequestService', () => {
8082
store = TestBed.inject(Store);
8183
mockStore = store as MockStore<CoreState>;
8284
mockStore.setState(initialState);
85+
xsrfService = {
86+
tokenInitialized$: new BehaviorSubject(false),
87+
} as XSRFService;
88+
8389
service = new RequestService(
8490
objectCache,
8591
uuidService,
8692
store,
93+
xsrfService,
8794
undefined
8895
);
8996
serviceAsAny = service as any;

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { ObjectCacheService } from '../cache/object-cache.service';
1111
import { IndexState, MetaIndexState } from '../index/index.reducer';
1212
import { requestIndexSelector, getUrlWithoutEmbedParams } from '../index/index.selectors';
1313
import { UUIDService } from '../shared/uuid.service';
14+
import { XSRFService } from '../xsrf/xsrf.service';
1415
import {
1516
RequestConfigureAction,
1617
RequestExecuteAction,
@@ -137,6 +138,7 @@ export class RequestService {
137138
constructor(private objectCache: ObjectCacheService,
138139
private uuidService: UUIDService,
139140
private store: Store<CoreState>,
141+
protected xsrfService: XSRFService,
140142
private indexStore: Store<MetaIndexState>) {
141143
}
142144

@@ -419,7 +421,17 @@ export class RequestService {
419421
*/
420422
private dispatchRequest(request: RestRequest) {
421423
this.store.dispatch(new RequestConfigureAction(request));
422-
this.store.dispatch(new RequestExecuteAction(request.uuid));
424+
// If it's a GET request, or we have an XSRF token, dispatch it immediately
425+
if (request.method === RestRequestMethod.GET || this.xsrfService.tokenInitialized$.getValue() === true) {
426+
this.store.dispatch(new RequestExecuteAction(request.uuid));
427+
} else {
428+
// Otherwise wait for the XSRF token first
429+
this.xsrfService.tokenInitialized$.pipe(
430+
find((hasInitialized: boolean) => hasInitialized === true),
431+
).subscribe(() => {
432+
this.store.dispatch(new RequestExecuteAction(request.uuid));
433+
});
434+
}
423435
}
424436

425437
/**
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { BrowserXSRFService } from './browser-xsrf.service';
2+
import { HttpClient } from '@angular/common/http';
3+
import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
4+
import { TestBed } from '@angular/core/testing';
5+
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
6+
7+
describe(`BrowserXSRFService`, () => {
8+
let service: BrowserXSRFService;
9+
let httpClient: HttpClient;
10+
let httpTestingController: HttpTestingController;
11+
12+
const endpointURL = new RESTURLCombiner('/security/csrf').toString();
13+
14+
beforeEach(() => {
15+
TestBed.configureTestingModule({
16+
imports: [ HttpClientTestingModule ],
17+
providers: [ BrowserXSRFService ]
18+
});
19+
httpClient = TestBed.inject(HttpClient);
20+
httpTestingController = TestBed.inject(HttpTestingController);
21+
service = TestBed.inject(BrowserXSRFService);
22+
});
23+
24+
describe(`initXSRFToken`, () => {
25+
it(`should perform a POST to the csrf endpoint`, () => {
26+
service.initXSRFToken(httpClient)();
27+
28+
const req = httpTestingController.expectOne({
29+
url: endpointURL,
30+
method: 'POST'
31+
});
32+
33+
req.flush({});
34+
httpTestingController.verify();
35+
});
36+
37+
describe(`when the POST succeeds`, () => {
38+
it(`should set tokenInitialized$ to true`, () => {
39+
service.initXSRFToken(httpClient)();
40+
41+
const req = httpTestingController.expectOne(endpointURL);
42+
43+
req.flush({});
44+
httpTestingController.verify();
45+
46+
expect(service.tokenInitialized$.getValue()).toBeTrue();
47+
});
48+
});
49+
50+
describe(`when the POST fails`, () => {
51+
it(`should set tokenInitialized$ to true`, () => {
52+
service.initXSRFToken(httpClient)();
53+
54+
const req = httpTestingController.expectOne(endpointURL);
55+
56+
req.error(new ErrorEvent('415'));
57+
httpTestingController.verify();
58+
59+
expect(service.tokenInitialized$.getValue()).toBeTrue();
60+
});
61+
});
62+
63+
});
64+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { HttpClient } from '@angular/common/http';
2+
import { Injectable } from '@angular/core';
3+
import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
4+
import { take, catchError } from 'rxjs/operators';
5+
import { of as observableOf } from 'rxjs';
6+
import { XSRFService } from './xsrf.service';
7+
8+
@Injectable()
9+
export class BrowserXSRFService extends XSRFService {
10+
initXSRFToken(httpClient: HttpClient): () => Promise<any> {
11+
return () => new Promise((resolve) => {
12+
httpClient.post(new RESTURLCombiner('/security/csrf').toString(), undefined).pipe(
13+
// errors are to be expected if the token and the cookie don't match, that's what we're
14+
// trying to fix for future requests, so just emit any observable to end up in the
15+
// subscribe
16+
catchError(() => observableOf(null)),
17+
take(1),
18+
).subscribe(() => {
19+
this.tokenInitialized$.next(true);
20+
});
21+
22+
// return immediately, the rest of the app doesn't need to wait for this to finish
23+
resolve();
24+
});
25+
}
26+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { ServerXSRFService } from './server-xsrf.service';
2+
import { HttpClient } from '@angular/common/http';
3+
4+
describe(`ServerXSRFService`, () => {
5+
let service: ServerXSRFService;
6+
let httpClient: HttpClient;
7+
8+
beforeEach(() => {
9+
httpClient = jasmine.createSpyObj(['post', 'get', 'request']);
10+
service = new ServerXSRFService();
11+
});
12+
13+
describe(`initXSRFToken`, () => {
14+
it(`shouldn't perform any requests`, (done: DoneFn) => {
15+
service.initXSRFToken(httpClient)().then(() => {
16+
for (const prop in httpClient) {
17+
if (httpClient.hasOwnProperty(prop)) {
18+
expect(httpClient[prop]).not.toHaveBeenCalled();
19+
}
20+
}
21+
done();
22+
});
23+
});
24+
25+
it(`should leave tokenInitialized$ on false`, (done: DoneFn) => {
26+
service.initXSRFToken(httpClient)().then(() => {
27+
expect(service.tokenInitialized$.getValue()).toBeFalse();
28+
done();
29+
});
30+
});
31+
});
32+
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { HttpClient } from '@angular/common/http';
2+
import { Injectable } from '@angular/core';
3+
import { XSRFService } from './xsrf.service';
4+
5+
@Injectable()
6+
export class ServerXSRFService extends XSRFService {
7+
initXSRFToken(httpClient: HttpClient): () => Promise<any> {
8+
return () => new Promise((resolve) => {
9+
// return immediately, and keep tokenInitialized$ false. The server side can make only GET
10+
// requests, since it can never get a valid XSRF cookie
11+
resolve();
12+
});
13+
}
14+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { XSRFService } from './xsrf.service';
2+
import { HttpClient } from '@angular/common/http';
3+
4+
class XSRFServiceImpl extends XSRFService {
5+
initXSRFToken(httpClient: HttpClient): () => Promise<any> {
6+
return () => null;
7+
}
8+
}
9+
10+
describe(`XSRFService`, () => {
11+
let service: XSRFService;
12+
13+
beforeEach(() => {
14+
service = new XSRFServiceImpl();
15+
});
16+
17+
it(`should start with tokenInitialized$.hasValue() === false`, () => {
18+
expect(service.tokenInitialized$.getValue()).toBeFalse();
19+
});
20+
});

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { HttpClient } from '@angular/common/http';
2+
import { Injectable } from '@angular/core';
3+
import { BehaviorSubject } from 'rxjs';
4+
5+
@Injectable()
6+
export abstract class XSRFService {
7+
public tokenInitialized$: BehaviorSubject<boolean> = new BehaviorSubject(false);
8+
9+
abstract initXSRFToken(httpClient: HttpClient): () => Promise<any>;
10+
}

src/modules/app/browser-app.module.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { HttpClient, HttpClientModule } from '@angular/common/http';
2-
import { NgModule } from '@angular/core';
2+
import { APP_INITIALIZER, NgModule } from '@angular/core';
33
import { BrowserModule, BrowserTransferStateModule, makeStateKey, TransferState } from '@angular/platform-browser';
44
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
55
import { REQUEST } from '@nguniversal/express-engine/tokens';
@@ -32,6 +32,8 @@ import { AuthRequestService } from '../../app/core/auth/auth-request.service';
3232
import { BrowserAuthRequestService } from '../../app/core/auth/browser-auth-request.service';
3333
import { BrowserInitService } from './browser-init.service';
3434
import { ReferrerService } from '../../app/core/services/referrer.service';
35+
import { BrowserXSRFService } from '../../app/core/xsrf/browser-xsrf.service';
36+
import { XSRFService } from '../../app/core/xsrf/xsrf.service';
3537
import { BrowserReferrerService } from '../../app/core/services/browser.referrer.service';
3638

3739
export const REQ_KEY = makeStateKey<string>('req');
@@ -73,6 +75,16 @@ export function getRequest(transferState: TransferState): any {
7375
useFactory: getRequest,
7476
deps: [TransferState]
7577
},
78+
{
79+
provide: APP_INITIALIZER,
80+
useFactory: (xsrfService: XSRFService, httpClient: HttpClient) => xsrfService.initXSRFToken(httpClient),
81+
deps: [ XSRFService, HttpClient ],
82+
multi: true,
83+
},
84+
{
85+
provide: XSRFService,
86+
useClass: BrowserXSRFService,
87+
},
7688
{
7789
provide: AuthService,
7890
useClass: AuthService

0 commit comments

Comments
 (0)