Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import { UploaderService } from '../../shared/uploader/uploader.service';
import { HostWindowService } from '../../shared/host-window.service';
import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub';
import { UploaderComponent } from '../../shared/uploader/uploader.component';
import { HttpXsrfTokenExtractor } from '@angular/common/http';
import { CookieService } from '../../core/services/cookie.service';
import { CookieServiceMock } from '../../shared/mocks/cookie.service.mock';
import { HttpXsrfTokenExtractorMock } from '../../shared/mocks/http-xsrf-token-extractor.mock';

describe('MyDSpaceNewSubmissionComponent test', () => {

Expand Down Expand Up @@ -55,6 +59,8 @@ describe('MyDSpaceNewSubmissionComponent test', () => {
ChangeDetectorRef,
MyDSpaceNewSubmissionComponent,
UploaderService,
{ provide: HttpXsrfTokenExtractor, useValue: new HttpXsrfTokenExtractorMock('mock-token') },
{ provide: CookieService, useValue: new CookieServiceMock() },
{ provide: HostWindowService, useValue: new HostWindowServiceStub(800) },
],
schemas: [NO_ERRORS_SCHEMA]
Expand Down
20 changes: 4 additions & 16 deletions src/app/core/xsrf/xsrf.interceptor.spec.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,20 @@
import { TestBed } from '@angular/core/testing';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { HttpHeaders, HTTP_INTERCEPTORS, HttpResponse, HttpXsrfTokenExtractor, HttpErrorResponse } from '@angular/common/http';
import { HttpHeaders, HTTP_INTERCEPTORS, HttpXsrfTokenExtractor } from '@angular/common/http';
import { DspaceRestService } from '../dspace-rest/dspace-rest.service';
import { RestRequestMethod } from '../data/rest-request-method';
import { CookieService } from '../services/cookie.service';
import { CookieServiceMock } from '../../shared/mocks/cookie.service.mock';
import { XsrfInterceptor } from './xsrf.interceptor';

/**
* A Mock TokenExtractor which just returns whatever token it is initialized with.
* This mock object is injected into our XsrfInterceptor, so that it always finds
* the same fake XSRF token.
*/
class MockTokenExtractor extends HttpXsrfTokenExtractor {
constructor(private token: string | null) { super(); }

getToken(): string | null { return this.token; }
}
import { HttpXsrfTokenExtractorMock } from '../../shared/mocks/http-xsrf-token-extractor.mock';

describe(`XsrfInterceptor`, () => {
let service: DspaceRestService;
let httpMock: HttpTestingController;
let cookieService: CookieService;

// Create a MockTokenExtractor which always returns "test-token". This will
// be used as the test HttpXsrfTokenExtractor, see below.
// mock XSRF token
const testToken = 'test-token';
const mockTokenExtractor = new MockTokenExtractor(testToken);

// Mock payload/statuses are dummy content as we are not testing the results
// of any below requests. We are only testing for X-XSRF-TOKEN header.
Expand All @@ -46,7 +34,7 @@ describe(`XsrfInterceptor`, () => {
useClass: XsrfInterceptor,
multi: true,
},
{ provide: HttpXsrfTokenExtractor, useValue: mockTokenExtractor },
{ provide: HttpXsrfTokenExtractor, useValue: new HttpXsrfTokenExtractorMock(testToken) },
{ provide: CookieService, useValue: new CookieServiceMock() }
],
});
Expand Down
28 changes: 15 additions & 13 deletions src/app/core/xsrf/xsrf.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
import { CookieService } from '../services/cookie.service';
import { throwError } from 'rxjs';

// Name of XSRF header we may send in requests to backend (this is a standard name defined by Angular)
export const XSRF_REQUEST_HEADER = 'X-XSRF-TOKEN';
// Name of XSRF header we may receive in responses from backend
export const XSRF_RESPONSE_HEADER = 'DSPACE-XSRF-TOKEN';
// Name of cookie where we store the XSRF token
export const XSRF_COOKIE = 'XSRF-TOKEN';

/**
* Custom Http Interceptor intercepting Http Requests & Responses to
* exchange XSRF/CSRF tokens with the backend.
Expand Down Expand Up @@ -43,11 +50,6 @@ export class XsrfInterceptor implements HttpInterceptor {
* @param next
*/
intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
// Name of XSRF header we may send in requests to backend (this is a standard name defined by Angular)
const requestCsrfHeader = 'X-XSRF-TOKEN';
// Name of XSRF header we may receive in responses from backend
const responseCsrfHeader = 'DSPACE-XSRF-TOKEN';

// Ensure EVERY request from Angular includes "withCredentials: true".
// This allows Angular to receive & send cookies via a CORS request (to
// the backend). ONLY requests with credentials will:
Expand All @@ -71,8 +73,8 @@ export class XsrfInterceptor implements HttpInterceptor {
const token = this.tokenExtractor.getToken() as string;

// send token in request's X-XSRF-TOKEN header (anti-CSRF security) to backend
if (token !== null && !req.headers.has(requestCsrfHeader)) {
req = req.clone({ headers: req.headers.set(requestCsrfHeader, token) });
if (token !== null && !req.headers.has(XSRF_REQUEST_HEADER)) {
req = req.clone({ headers: req.headers.set(XSRF_REQUEST_HEADER, token) });
}
}
// Pass to next interceptor, but intercept EVERY response event as well
Expand All @@ -82,19 +84,19 @@ export class XsrfInterceptor implements HttpInterceptor {
if (response instanceof HttpResponse) {
// For every response that comes back, check for the custom
// DSPACE-XSRF-TOKEN header sent from the backend.
if (response.headers.has(responseCsrfHeader)) {
if (response.headers.has(XSRF_RESPONSE_HEADER)) {
// value of header is a new XSRF token
this.saveXsrfToken(response.headers.get(responseCsrfHeader));
this.saveXsrfToken(response.headers.get(XSRF_RESPONSE_HEADER));
}
}
}),
catchError((error) => {
if (error instanceof HttpErrorResponse) {
// For every error that comes back, also check for the custom
// DSPACE-XSRF-TOKEN header sent from the backend.
if (error.headers.has(responseCsrfHeader)) {
if (error.headers.has(XSRF_RESPONSE_HEADER)) {
// value of header is a new XSRF token
this.saveXsrfToken(error.headers.get(responseCsrfHeader));
this.saveXsrfToken(error.headers.get(XSRF_RESPONSE_HEADER));
}
}
// Return error response as is.
Expand All @@ -111,7 +113,7 @@ export class XsrfInterceptor implements HttpInterceptor {
// Save token value as a *new* value of our client-side XSRF-TOKEN cookie.
// This is the cookie that is parsed by Angular's tokenExtractor(),
// which we will send back in the X-XSRF-TOKEN header per Angular best practices.
this.cookieService.remove('XSRF-TOKEN');
this.cookieService.set('XSRF-TOKEN', token);
this.cookieService.remove(XSRF_COOKIE);
this.cookieService.set(XSRF_COOKIE, token);
}
}
12 changes: 12 additions & 0 deletions src/app/shared/mocks/http-xsrf-token-extractor.mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { HttpXsrfTokenExtractor } from '@angular/common/http';

/**
* A Mock TokenExtractor which just returns whatever token it is initialized with.
* This mock object is injected into our XsrfInterceptor, so that it always finds
* the same fake XSRF token.
*/
export class HttpXsrfTokenExtractorMock extends HttpXsrfTokenExtractor {
constructor(private token: string | null) { super(); }

getToken(): string | null { return this.token; }
}
8 changes: 7 additions & 1 deletion src/app/shared/uploader/uploader.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import { UploaderComponent } from './uploader.component';
import { FileUploadModule } from 'ng2-file-upload';
import { TranslateModule } from '@ngx-translate/core';
import { createTestComponent } from '../testing/utils.test';
import { HttpXsrfTokenExtractor } from '@angular/common/http';
import { CookieService } from '../../core/services/cookie.service';
import { CookieServiceMock } from '../mocks/cookie.service.mock';
import { HttpXsrfTokenExtractorMock } from '../mocks/http-xsrf-token-extractor.mock';

describe('Chips component', () => {

Expand All @@ -33,7 +37,9 @@ describe('Chips component', () => {
ChangeDetectorRef,
ScrollToService,
UploaderComponent,
UploaderService
UploaderService,
{ provide: HttpXsrfTokenExtractor, useValue: new HttpXsrfTokenExtractorMock('mock-token') },
{ provide: CookieService, useValue: new CookieServiceMock() },
],
schemas: [CUSTOM_ELEMENTS_SCHEMA]
});
Expand Down
47 changes: 41 additions & 6 deletions src/app/shared/uploader/uploader.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import { UploaderOptions } from './uploader-options.model';
import { hasValue, isNotEmpty, isUndefined } from '../empty.util';
import { UploaderService } from './uploader.service';
import { UploaderProperties } from './uploader-properties.model';
import { HttpXsrfTokenExtractor } from '@angular/common/http';
import { XSRF_REQUEST_HEADER, XSRF_RESPONSE_HEADER, XSRF_COOKIE } from '../../core/xsrf/xsrf.interceptor';
import { CookieService } from '../../core/services/cookie.service';

@Component({
selector: 'ds-uploader',
Expand Down Expand Up @@ -91,7 +94,9 @@ export class UploaderComponent {
}
}

constructor(private cdr: ChangeDetectorRef, private scrollToService: ScrollToService, private uploaderService: UploaderService) {
constructor(private cdr: ChangeDetectorRef, private scrollToService: ScrollToService,
private uploaderService: UploaderService, private tokenExtractor: HttpXsrfTokenExtractor,
private cookieService: CookieService) {
}

/**
Expand All @@ -108,7 +113,7 @@ export class UploaderComponent {
removeAfterUpload: true,
autoUpload: this.uploadFilesOptions.autoUpload,
method: this.uploadFilesOptions.method,
queueLimit: this.uploadFilesOptions.maxFileNumber
queueLimit: this.uploadFilesOptions.maxFileNumber,
});

if (isUndefined(this.enableDragOverDocument)) {
Expand All @@ -123,10 +128,6 @@ export class UploaderComponent {
}

ngAfterViewInit() {
// Maybe to remove: needed to avoid CORS issue with our temp upload server
this.uploader.onAfterAddingFile = ((item) => {
item.withCredentials = false;
});
this.uploader.onAfterAddingAll = ((items) => {
this.onFileSelected.emit(items);
});
Expand All @@ -137,6 +138,8 @@ export class UploaderComponent {
if (item.url !== this.uploader.options.url) {
item.url = this.uploader.options.url;
}
// Ensure the current XSRF token is included in every upload request (token may change between items uploaded)
this.uploader.options.headers = [{ name: XSRF_REQUEST_HEADER, value: this.tokenExtractor.getToken() }];
this.onBeforeUpload();
this.isOverDocumentDropZone = observableOf(false);

Expand All @@ -152,12 +155,30 @@ export class UploaderComponent {
};
}
this.uploader.onCompleteItem = (item: any, response: any, status: any, headers: any) => {
// Check for a changed XSRF token in response & save new token if found (to both cookie & header for next request)
// NOTE: this is only necessary because ng2-file-upload doesn't use an Http service and therefore never
// triggers our xsrf.interceptor.ts. See this bug: https://github.com/valor-software/ng2-file-upload/issues/950
const token = headers[XSRF_RESPONSE_HEADER.toLowerCase()];
if (isNotEmpty(token)) {
this.saveXsrfToken(token);
this.uploader.options.headers = [{ name: XSRF_REQUEST_HEADER, value: this.tokenExtractor.getToken() }];
}

if (isNotEmpty(response)) {
const responsePath = JSON.parse(response);
this.onCompleteItem.emit(responsePath);
}
};
this.uploader.onErrorItem = (item: any, response: any, status: any, headers: any) => {
// Check for a changed XSRF token in response & save new token if found (to both cookie & header for next request)
// NOTE: this is only necessary because ng2-file-upload doesn't use an Http service and therefore never
// triggers our xsrf.interceptor.ts. See this bug: https://github.com/valor-software/ng2-file-upload/issues/950
const token = headers[XSRF_RESPONSE_HEADER.toLowerCase()];
if (isNotEmpty(token)) {
this.saveXsrfToken(token);
this.uploader.options.headers = [{ name: XSRF_REQUEST_HEADER, value: this.tokenExtractor.getToken() }];
}

this.onUploadError.emit({ item: item, response: response, status: status, headers: headers });
this.uploader.cancelAll();
};
Expand Down Expand Up @@ -201,4 +222,18 @@ export class UploaderComponent {
}
}

/**
* Save XSRF token found in response. This is a temporary copy of the method in xsrf.interceptor.ts
* It can be removed once ng2-file-upload supports interceptors (see https://github.com/valor-software/ng2-file-upload/issues/950),
* or we switch to a new upload library (see https://github.com/DSpace/dspace-angular/issues/820)
* @param token token found
*/
private saveXsrfToken(token: string) {
// Save token value as a *new* value of our client-side XSRF-TOKEN cookie.
// This is the cookie that is parsed by Angular's tokenExtractor(),
// which we will send back in the X-XSRF-TOKEN header per Angular best practices.
this.cookieService.remove(XSRF_COOKIE);
this.cookieService.set(XSRF_COOKIE, token);
}

}