114624: Made the legacyBitstreamURLRedirectGuard return false for valid bitstream urls in combination with a HardRedirectService#redirect, this will make ensure the redirect is visible for curl instead of being performed by Angular

(cherry picked from commit 23644e9ec7)
This commit is contained in:
Alexandre Vryghem
2024-05-24 14:21:57 +02:00
committed by github-actions[bot]
parent f1b397fc11
commit c332600633
3 changed files with 48 additions and 40 deletions

View File

@@ -1,14 +1,15 @@
import { cold } from 'jasmine-marbles';
import { EMPTY } from 'rxjs'; import { EMPTY } from 'rxjs';
import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
import { BitstreamDataService } from '../core/data/bitstream-data.service'; import { BitstreamDataService } from '../core/data/bitstream-data.service';
import { RemoteData } from '../core/data/remote-data'; import { RemoteData } from '../core/data/remote-data';
import { RequestEntryState } from '../core/data/request-entry-state.model'; import { RequestEntryState } from '../core/data/request-entry-state.model';
import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard'; import { BrowserHardRedirectService } from '../core/services/browser-hard-redirect.service';
import { RouterStub } from '../shared/testing/router.stub'; import { HardRedirectService } from '../core/services/hard-redirect.service';
import { ServerResponseServiceStub } from '../shared/testing/server-response-service.stub';
import { fakeAsync } from '@angular/core/testing';
import { cold } from 'jasmine-marbles';
import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
import { Bitstream } from '../core/shared/bitstream.model'; import { Bitstream } from '../core/shared/bitstream.model';
import { RouterStub } from '../shared/testing/router.stub';
import { legacyBitstreamURLRedirectGuard } from './legacy-bitstream-url-redirect.guard';
describe('legacyBitstreamURLRedirectGuard', () => { describe('legacyBitstreamURLRedirectGuard', () => {
let resolver: any; let resolver: any;
@@ -16,21 +17,26 @@ describe('legacyBitstreamURLRedirectGuard', () => {
let remoteDataMocks: { [type: string]: RemoteData<any> }; let remoteDataMocks: { [type: string]: RemoteData<any> };
let route; let route;
let state; let state;
let serverResponseService: ServerResponseServiceStub; let hardRedirectService: HardRedirectService;
let router: RouterStub; let router: RouterStub;
let bitstream: Bitstream;
beforeEach(() => { beforeEach(() => {
route = { route = {
params: {}, params: {},
queryParams: {} queryParams: {}
}; };
router = new RouterStub(); router = new RouterStub();
serverResponseService = new ServerResponseServiceStub(); hardRedirectService = new BrowserHardRedirectService(window.location);
state = {}; state = {};
bitstream = Object.assign(new Bitstream(), {
uuid: 'bitstream-id',
});
remoteDataMocks = { remoteDataMocks = {
RequestPending: new RemoteData(undefined, 0, 0, RequestEntryState.RequestPending, undefined, undefined, undefined), RequestPending: new RemoteData(undefined, 0, 0, RequestEntryState.RequestPending, undefined, undefined, undefined),
ResponsePending: new RemoteData(undefined, 0, 0, RequestEntryState.ResponsePending, undefined, undefined, undefined), ResponsePending: new RemoteData(undefined, 0, 0, RequestEntryState.ResponsePending, undefined, undefined, undefined),
Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, new Bitstream(), 200), Success: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, bitstream, 200),
NoContent: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, undefined, 204), NoContent: new RemoteData(0, 0, 0, RequestEntryState.Success, undefined, undefined, 204),
Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500), Error: new RemoteData(0, 0, 0, RequestEntryState.Error, 'Internal server error', undefined, 500),
}; };
@@ -54,7 +60,7 @@ describe('legacyBitstreamURLRedirectGuard', () => {
}); });
}); });
it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => { it(`should call findByItemHandle with the handle, sequence id, and filename from the route`, () => {
resolver(route, state, bitstreamDataService, serverResponseService, router); resolver(route, state, bitstreamDataService, hardRedirectService, router);
expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith(
`${route.params.prefix}/${route.params.suffix}`, `${route.params.prefix}/${route.params.suffix}`,
route.params.sequence_id, route.params.sequence_id,
@@ -79,7 +85,7 @@ describe('legacyBitstreamURLRedirectGuard', () => {
}); });
}); });
it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => { it(`should call findByItemHandle with the handle and filename from the route, and the sequence ID from the queryParams`, () => {
resolver(route, state, bitstreamDataService, serverResponseService, router); resolver(route, state, bitstreamDataService, hardRedirectService, router);
expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith(
`${route.params.prefix}/${route.params.suffix}`, `${route.params.prefix}/${route.params.suffix}`,
route.queryParams.sequenceId, route.queryParams.sequenceId,
@@ -99,7 +105,7 @@ describe('legacyBitstreamURLRedirectGuard', () => {
}); });
}); });
it(`should call findByItemHandle with the handle, and filename from the route`, () => { it(`should call findByItemHandle with the handle, and filename from the route`, () => {
resolver(route, state, bitstreamDataService, serverResponseService, router); resolver(route, state, bitstreamDataService, hardRedirectService, router);
expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith( expect(bitstreamDataService.findByItemHandle).toHaveBeenCalledWith(
`${route.params.prefix}/${route.params.suffix}`, `${route.params.prefix}/${route.params.suffix}`,
undefined, undefined,
@@ -109,45 +115,44 @@ describe('legacyBitstreamURLRedirectGuard', () => {
}); });
}); });
describe('should return and complete after the RemoteData has...', () => { describe('should return and complete after the RemoteData has...', () => {
it('...failed', fakeAsync(() => { it('...failed', () => {
spyOn(router, 'createUrlTree').and.callThrough(); spyOn(router, 'createUrlTree').and.callThrough();
spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', {
a: remoteDataMocks.RequestPending, a: remoteDataMocks.RequestPending,
b: remoteDataMocks.ResponsePending, b: remoteDataMocks.ResponsePending,
c: remoteDataMocks.Error, c: remoteDataMocks.Error,
})); }));
resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => { resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => {
expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled();
expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]);
}); });
})); });
it('...succeeded without content', fakeAsync(() => { it('...succeeded without content', () => {
spyOn(router, 'createUrlTree').and.callThrough(); spyOn(router, 'createUrlTree').and.callThrough();
spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', {
a: remoteDataMocks.RequestPending, a: remoteDataMocks.RequestPending,
b: remoteDataMocks.ResponsePending, b: remoteDataMocks.ResponsePending,
c: remoteDataMocks.NoContent, c: remoteDataMocks.NoContent,
})); }));
resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => { resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => {
expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled();
expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]); expect(router.createUrlTree).toHaveBeenCalledWith([PAGE_NOT_FOUND_PATH]);
}); });
})); });
it('...succeeded', fakeAsync(() => { it('...succeeded', () => {
spyOn(serverResponseService, 'setStatus').and.callThrough(); spyOn(hardRedirectService, 'redirect');
spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', { spyOn(bitstreamDataService, 'findByItemHandle').and.returnValue(cold('a-b-c', {
a: remoteDataMocks.RequestPending, a: remoteDataMocks.RequestPending,
b: remoteDataMocks.ResponsePending, b: remoteDataMocks.ResponsePending,
c: remoteDataMocks.Success, c: remoteDataMocks.Success,
})); }));
resolver(route, state, bitstreamDataService, serverResponseService, router).subscribe(() => { resolver(route, state, bitstreamDataService, hardRedirectService, router).subscribe(() => {
expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled(); expect(bitstreamDataService.findByItemHandle).toHaveBeenCalled();
expect(serverResponseService.setStatus).toHaveBeenCalledWith(301); expect(hardRedirectService.redirect).toHaveBeenCalledWith(new URL(`/bitstreams/${bitstream.uuid}/download`, window.location.origin).href, 301);
expect(router.parseUrl).toHaveBeenCalled(); });
}); });
}));
}); });
}); });
}); });

View File

@@ -1,14 +1,21 @@
import { inject } from '@angular/core'; import { inject } from '@angular/core';
import { ActivatedRouteSnapshot, CanActivateFn, UrlTree, Router, RouterStateSnapshot } from '@angular/router'; import {
ActivatedRouteSnapshot,
CanActivateFn,
Router,
RouterStateSnapshot,
UrlTree,
} from '@angular/router';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { RemoteData } from '../core/data/remote-data'; import { map } from 'rxjs/operators';
import { Bitstream } from '../core/shared/bitstream.model';
import { hasNoValue } from '../shared/empty.util';
import { BitstreamDataService } from '../core/data/bitstream-data.service';
import { ServerResponseService } from '../core/services/server-response.service';
import { map, tap } from 'rxjs/operators';
import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths'; import { PAGE_NOT_FOUND_PATH } from '../app-routing-paths';
import { BitstreamDataService } from '../core/data/bitstream-data.service';
import { RemoteData } from '../core/data/remote-data';
import { HardRedirectService } from '../core/services/hard-redirect.service';
import { Bitstream } from '../core/shared/bitstream.model';
import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { getFirstCompletedRemoteData } from '../core/shared/operators';
import { hasNoValue } from '../shared/empty.util';
/** /**
* Redirects to a bitstream based on the handle of the item, and the sequence id or the filename of the * Redirects to a bitstream based on the handle of the item, and the sequence id or the filename of the
@@ -21,9 +28,9 @@ export const legacyBitstreamURLRedirectGuard: CanActivateFn = (
route: ActivatedRouteSnapshot, route: ActivatedRouteSnapshot,
state: RouterStateSnapshot, state: RouterStateSnapshot,
bitstreamDataService: BitstreamDataService = inject(BitstreamDataService), bitstreamDataService: BitstreamDataService = inject(BitstreamDataService),
serverResponseService: ServerResponseService = inject(ServerResponseService), serverHardRedirectService: HardRedirectService = inject(HardRedirectService),
router: Router = inject(Router), router: Router = inject(Router),
): Observable<UrlTree> => { ): Observable<UrlTree | boolean> => {
const prefix = route.params.prefix; const prefix = route.params.prefix;
const suffix = route.params.suffix; const suffix = route.params.suffix;
const filename = route.params.filename; const filename = route.params.filename;
@@ -37,14 +44,10 @@ export const legacyBitstreamURLRedirectGuard: CanActivateFn = (
filename, filename,
).pipe( ).pipe(
getFirstCompletedRemoteData(), getFirstCompletedRemoteData(),
tap((rd: RemoteData<Bitstream>) => {
if (rd.hasSucceeded && !rd.hasNoContent) {
serverResponseService.setStatus(301);
}
}),
map((rd: RemoteData<Bitstream>) => { map((rd: RemoteData<Bitstream>) => {
if (rd.hasSucceeded && !rd.hasNoContent) { if (rd.hasSucceeded && !rd.hasNoContent) {
return router.parseUrl(`/bitstreams/${rd.payload.uuid}/download`); serverHardRedirectService.redirect(new URL(`/bitstreams/${rd.payload.uuid}/download`, serverHardRedirectService.getCurrentOrigin()).href, 301);
return false;
} else { } else {
return router.createUrlTree([PAGE_NOT_FOUND_PATH]); return router.createUrlTree([PAGE_NOT_FOUND_PATH]);
} }

View File

@@ -171,7 +171,7 @@ export class BitstreamDataService extends IdentifiableDataService<Bitstream> imp
searchParams.push(new RequestParam('sequenceId', sequenceId)); searchParams.push(new RequestParam('sequenceId', sequenceId));
} }
if (hasValue(filename)) { if (hasValue(filename)) {
searchParams.push(new RequestParam('filename', filename)); searchParams.push(new RequestParam('filename', encodeURIComponent(filename)));
} }
const hrefObs = this.getSearchByHref( const hrefObs = this.getSearchByHref(