From 0c58a5bf0599fde6f0dfa958cc1338a10b478b5a Mon Sep 17 00:00:00 2001 From: Kim Shepherd Date: Thu, 13 Mar 2025 11:26:57 +0100 Subject: [PATCH] Request-a-copy: Refactor for angular control flow changes --- .../data/item-request-data.service.spec.ts | 126 ++++++++++++++- .../core/data/item-request-data.service.ts | 144 +++++++++++++++++- src/app/core/shared/item-request.model.ts | 10 ++ ...tstream-request-a-copy-page.component.html | 8 + ...ream-request-a-copy-page.component.spec.ts | 35 ++++- ...bitstream-request-a-copy-page.component.ts | 45 +++++- .../email-request-copy.component.html | 45 +++++- .../email-request-copy.component.spec.ts | 1 + .../email-request-copy.component.ts | 40 ++++- .../themed-email-request-copy.component.ts | 13 +- .../grant-deny-request-copy.component.html | 62 +++++--- .../grant-request-copy.component.html | 33 +++- .../grant-request-copy.component.spec.ts | 14 +- .../grant-request-copy.component.ts | 65 +++++++- 14 files changed, 581 insertions(+), 60 deletions(-) diff --git a/src/app/core/data/item-request-data.service.spec.ts b/src/app/core/data/item-request-data.service.spec.ts index 68577ae6e2..b9cfe689fa 100644 --- a/src/app/core/data/item-request-data.service.spec.ts +++ b/src/app/core/data/item-request-data.service.spec.ts @@ -1,10 +1,17 @@ +import { HttpHeaders } from '@angular/common/http'; import { of as observableOf } from 'rxjs'; import { RequestCopyEmail } from '../../request-copy/email-request-copy/request-copy-email.model'; +import { MockBitstream1 } from '../../shared/mocks/item.mock'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { ConfigurationProperty } from '../shared/configuration-property.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { ItemRequest } from '../shared/item-request.model'; +import { ConfigurationDataService } from './configuration-data.service'; +import { AuthorizationDataService } from './feature-authorization/authorization-data.service'; +import { FeatureID } from './feature-authorization/feature-id'; +import { FindListOptions } from './find-list-options.model'; import { ItemRequestDataService } from './item-request-data.service'; import { PostRequest } from './request.models'; import { RequestService } from './request.service'; @@ -16,12 +23,36 @@ describe('ItemRequestDataService', () => { let requestService: RequestService; let rdbService: RemoteDataBuildService; let halService: HALEndpointService; + let configService: ConfigurationDataService; + let authorizationDataService: AuthorizationDataService; const restApiEndpoint = 'rest/api/endpoint/'; const requestId = 'request-id'; let itemRequest: ItemRequest; beforeEach(() => { + configService = jasmine.createSpyObj('ConfigurationDataService', ['findByPropertyName']); + (configService.findByPropertyName as jasmine.Spy).and.callFake((propertyName: string) => { + switch (propertyName) { + case 'request.item.create.captcha': + return createSuccessfulRemoteDataObject$(Object.assign(new ConfigurationProperty(), { + name: 'request.item.create.captcha', + values: ['true'], + })); + case 'request.item.grant.link.period': + return createSuccessfulRemoteDataObject$(Object.assign(new ConfigurationProperty(), { + name: 'request.item.grant.link.period', + values: ['3600', '7200', '86400'], + })); + default: + return createSuccessfulRemoteDataObject$(new ConfigurationProperty()); + } + }); + + + authorizationDataService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(false), + }); itemRequest = Object.assign(new ItemRequest(), { token: 'item-request-token', }); @@ -36,13 +67,42 @@ describe('ItemRequestDataService', () => { getEndpoint: observableOf(restApiEndpoint), }); - service = new ItemRequestDataService(requestService, rdbService, null, halService); + service = new ItemRequestDataService(requestService, rdbService, null, halService, configService, authorizationDataService); + }); + + describe('searchBy', () => { + it('should use searchData to perform search operations', () => { + const searchMethod = 'testMethod'; + const options = new FindListOptions(); + + const searchDataSpy = spyOn((service as any).searchData, 'searchBy').and.returnValue(observableOf(null)); + + service.searchBy(searchMethod, options); + + expect(searchDataSpy).toHaveBeenCalledWith( + searchMethod, + options, + undefined, + undefined, + ); + }); }); describe('requestACopy', () => { it('should send a POST request containing the provided item request', (done) => { - service.requestACopy(itemRequest).subscribe(() => { - expect(requestService.send).toHaveBeenCalledWith(new PostRequest(requestId, restApiEndpoint, itemRequest)); + const captchaPayload = 'payload'; + service.requestACopy(itemRequest, captchaPayload).subscribe(() => { + expect(requestService.send).toHaveBeenCalledWith( + new PostRequest( + requestId, + restApiEndpoint, + itemRequest, + { + headers: new HttpHeaders().set('x-captcha-payload', captchaPayload), + }, + ), + false, + ); done(); }); }); @@ -59,11 +119,16 @@ describe('ItemRequestDataService', () => { service.grant(itemRequest.token, email, true).subscribe(() => { expect(requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ method: RestRequestMethod.PUT, + href: `${restApiEndpoint}/${itemRequest.token}`, body: JSON.stringify({ acceptRequest: true, responseMessage: email.message, subject: email.subject, suggestOpenAccess: true, + accessPeriod: 0, + }), + options: jasmine.objectContaining({ + headers: jasmine.any(HttpHeaders), }), })); done(); @@ -82,15 +147,70 @@ describe('ItemRequestDataService', () => { service.deny(itemRequest.token, email).subscribe(() => { expect(requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ method: RestRequestMethod.PUT, + href: `${restApiEndpoint}/${itemRequest.token}`, body: JSON.stringify({ acceptRequest: false, responseMessage: email.message, subject: email.subject, suggestOpenAccess: false, + accessPeriod: 0, + }), + options: jasmine.objectContaining({ + headers: jasmine.any(HttpHeaders), }), })); done(); }); }); }); + + describe('requestACopy', () => { + it('should send a POST request containing the provided item request', (done) => { + const captchaPayload = 'payload'; + service.requestACopy(itemRequest, captchaPayload).subscribe(() => { + expect(requestService.send).toHaveBeenCalledWith( + new PostRequest( + requestId, + restApiEndpoint, + itemRequest, + { + headers: new HttpHeaders().set('x-captcha-payload', captchaPayload), + }, + ), + false, + ); + done(); + }); + }); + }); + + describe('getConfiguredAccessPeriods', () => { + it('should return parsed integer values from config', () => { + service.getConfiguredAccessPeriods().subscribe(periods => { + expect(periods).toEqual([3600, 7200, 86400]); + }); + }); + }); + describe('isProtectedByCaptcha', () => { + it('should return true when config value is "true"', () => { + const mockConfigProperty = { + name: 'request.item.create.captcha', + values: ['true'], + } as ConfigurationProperty; + service.isProtectedByCaptcha().subscribe(result => { + expect(result).toBe(true); + }); + }); + }); + + describe('canDownload', () => { + it('should check authorization for bitstream download', () => { + service.canDownload(MockBitstream1).subscribe(result => { + expect(authorizationDataService.isAuthorized).toHaveBeenCalledWith(FeatureID.CanDownload, MockBitstream1.self); + expect(result).toBe(false); + }); + }); + }); + + }); diff --git a/src/app/core/data/item-request-data.service.ts b/src/app/core/data/item-request-data.service.ts index 5c85ed1471..db6221572f 100644 --- a/src/app/core/data/item-request-data.service.ts +++ b/src/app/core/data/item-request-data.service.ts @@ -13,14 +13,27 @@ import { hasValue, isNotEmpty, } from '../../shared/empty.util'; +import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { RequestParam } from '../cache/models/request-param.model'; import { ObjectCacheService } from '../cache/object-cache.service'; import { HttpOptions } from '../dspace-rest/dspace-rest.service'; +import { Bitstream } from '../shared/bitstream.model'; +import { ConfigurationProperty } from '../shared/configuration-property.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { ItemRequest } from '../shared/item-request.model'; import { getFirstCompletedRemoteData } from '../shared/operators'; import { sendRequest } from '../shared/request.operators'; import { IdentifiableDataService } from './base/identifiable-data.service'; +import { + SearchData, + SearchDataImpl, +} from './base/search-data'; +import { ConfigurationDataService } from './configuration-data.service'; +import { AuthorizationDataService } from './feature-authorization/authorization-data.service'; +import { FeatureID } from './feature-authorization/feature-id'; +import { FindListOptions } from './find-list-options.model'; +import { PaginatedList } from './paginated-list.model'; import { RemoteData } from './remote-data'; import { PostRequest, @@ -34,14 +47,21 @@ import { RequestService } from './request.service'; @Injectable({ providedIn: 'root', }) -export class ItemRequestDataService extends IdentifiableDataService { +export class ItemRequestDataService extends IdentifiableDataService implements SearchData { + + // TODO: This is only public for access by the test class - smell? + private searchData: SearchDataImpl; + constructor( protected requestService: RequestService, protected rdbService: RemoteDataBuildService, protected objectCache: ObjectCacheService, protected halService: HALEndpointService, + protected configService: ConfigurationDataService, + protected authorizationService: AuthorizationDataService, ) { super('itemrequests', requestService, rdbService, objectCache, halService); + this.searchData = new SearchDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive); } getItemRequestEndpoint(): Observable { @@ -61,17 +81,26 @@ export class ItemRequestDataService extends IdentifiableDataService /** * Request a copy of an item * @param itemRequest + * @param captchaPayload payload of captcha verification */ - requestACopy(itemRequest: ItemRequest): Observable> { + requestACopy(itemRequest: ItemRequest, captchaPayload: string): Observable> { const requestId = this.requestService.generateRequestId(); const href$ = this.getItemRequestEndpoint(); + // Inject captcha payload into headers + const options: HttpOptions = Object.create({}); + if (captchaPayload) { + let headers = new HttpHeaders(); + headers = headers.set('x-captcha-payload', captchaPayload); + options.headers = headers; + } + href$.pipe( find((href: string) => hasValue(href)), map((href: string) => { - const request = new PostRequest(requestId, href, itemRequest); - this.requestService.send(request); + const request = new PostRequest(requestId, href, itemRequest, options); + this.requestService.send(request, false); }), ).subscribe(); @@ -94,9 +123,10 @@ export class ItemRequestDataService extends IdentifiableDataService * @param token Token of the {@link ItemRequest} * @param email Email to send back to the user requesting the item * @param suggestOpenAccess Whether or not to suggest the item to become open access + * @param accessPeriod How long in seconds to grant access, from the decision date (only applies to links, not attachments) */ - grant(token: string, email: RequestCopyEmail, suggestOpenAccess = false): Observable> { - return this.process(token, email, true, suggestOpenAccess); + grant(token: string, email: RequestCopyEmail, suggestOpenAccess = false, accessPeriod = 0): Observable> { + return this.process(token, email, true, suggestOpenAccess, accessPeriod); } /** @@ -105,8 +135,9 @@ export class ItemRequestDataService extends IdentifiableDataService * @param email Email to send back to the user requesting the item * @param grant Grant or deny the request (true = grant, false = deny) * @param suggestOpenAccess Whether or not to suggest the item to become open access + * @param accessPeriod How long in seconds to grant access, from the decision date (only applies to links, not attachments) */ - process(token: string, email: RequestCopyEmail, grant: boolean, suggestOpenAccess = false): Observable> { + process(token: string, email: RequestCopyEmail, grant: boolean, suggestOpenAccess = false, accessPeriod = 0): Observable> { const requestId = this.requestService.generateRequestId(); this.getItemRequestEndpointByToken(token).pipe( @@ -121,6 +152,7 @@ export class ItemRequestDataService extends IdentifiableDataService responseMessage: email.message, subject: email.subject, suggestOpenAccess, + accessPeriod: accessPeriod, }), options); }), sendRequest(this.requestService), @@ -128,4 +160,102 @@ export class ItemRequestDataService extends IdentifiableDataService return this.rdbService.buildFromRequestUUID(requestId); } + + // TODO: Remove this, after discussion about implications and compare to bitstream data service byItemHandle + // Reviewers may ask that we instead just wrap the REST response in pagination even though we only expect one obj + /** + * Get a sanitized item request using the searchBy method and the access token sent to the original requester. + * + * @param accessToken access token contained in the secure link sent to a requester + */ + getSanitizedRequestByAccessTokenPaged(accessToken: string): Observable>> { + // We only expect / want one result as access tokens are unique + const findListOptions = Object.assign({}, new FindListOptions(), { + elementsPerPage: 1, + currentPage: 1, + searchParams: [ + new RequestParam('accessToken', accessToken), + ], + }); + // Pipe the paginated searchBy results and return a single item request + return this.searchBy('byAccessToken', findListOptions); + } + + /** + * Get a sanitized item request using the searchBy method and the access token sent to the original requester. + * + * @param accessToken access token contained in the secure link sent to a requester + */ + getSanitizedRequestByAccessToken(accessToken: string): Observable> { + const findListOptions = Object.assign({}, new FindListOptions(), { + searchParams: [ + new RequestParam('accessToken', accessToken), + ], + }); + const hrefObs = this.getSearchByHref( + 'byAccessToken', + findListOptions, + ); + + return this.searchData.findByHref( + hrefObs, + ); + } + + searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig[]): Observable>> { + return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale); + } + + /** + * Get configured access periods (in seconds) to populate the dropdown in the item request approval form + * if the 'send secure link' feature is configured. + * Expects integer values, conversion to number is done in this processing + */ + getConfiguredAccessPeriods(): Observable { + return this.configService.findByPropertyName('request.item.grant.link.period').pipe( + getFirstCompletedRemoteData(), + map((propertyRD: RemoteData) => propertyRD.hasSucceeded ? propertyRD.payload.values : []), + map((values) => values.map(value => parseInt(value, 10))), + ); + } + + /** + * Is the request copy form protected by a captcha? This will be used to decide whether to render the captcha + * component in bitstream-request-a-copy-page component + */ + isProtectedByCaptcha(): Observable { + return this.configService.findByPropertyName('request.item.create.captcha').pipe( + getFirstCompletedRemoteData(), + map((rd) => { + if (rd.hasSucceeded) { + return rd.payload.values.length > 0 && rd.payload.values[0] === 'true'; + } else { + return false; + } + })); + } + + /** + * Create the HREF for a specific object's search method with given options object + * + * @param searchMethod The search method for the object + * @param options The [[FindListOptions]] object + * @return {Observable} + * Return an observable that emits created HREF + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved + */ + public getSearchByHref(searchMethod: string, options?: FindListOptions, ...linksToFollow: FollowLinkConfig[]): Observable { + return this.searchData.getSearchByHref(searchMethod, options, ...linksToFollow); + } + + /** + * Authorization check to see if the user already has download access to the given bitstream. + * Wrapped in this service to give it a central place and make it easy to mock for testing. + * + * @param bitstream The bitstream to be downloaded + * @return {Observable} true if user may download, false if not + */ + canDownload(bitstream: Bitstream): Observable { + return this.authorizationService.isAuthorized(FeatureID.CanDownload, bitstream?.self); + } } diff --git a/src/app/core/shared/item-request.model.ts b/src/app/core/shared/item-request.model.ts index 5a4f912363..8fa3398d24 100644 --- a/src/app/core/shared/item-request.model.ts +++ b/src/app/core/shared/item-request.model.ts @@ -80,6 +80,16 @@ export class ItemRequest implements CacheableObject { */ @autoserialize bitstreamId: string; + /** + * Access token of the request (read-only) + */ + @autoserialize + accessToken: string; + /** + * Access period of the request + */ + @autoserialize + accessPeriod: number; /** * The {@link HALLink}s for this ItemRequest diff --git a/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.html b/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.html index 4e40883602..1f39afbdcf 100644 --- a/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.html +++ b/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.html @@ -84,6 +84,14 @@ + + @if (!!(captchaEnabled$ | async)) { +
+ + +
+ } +
diff --git a/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.spec.ts b/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.spec.ts index 5a5a8e6fca..7456ec6200 100644 --- a/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.spec.ts +++ b/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.spec.ts @@ -16,19 +16,27 @@ import { ActivatedRoute, Router, } from '@angular/router'; +import { Store } from '@ngrx/store'; +import { provideMockStore } from '@ngrx/store/testing'; import { TranslateModule } from '@ngx-translate/core'; import { of as observableOf } from 'rxjs'; +import { APP_DATA_SERVICES_MAP } from '../../../../config/app-config.interface'; import { AuthService } from '../../../core/auth/auth.service'; import { DSONameService } from '../../../core/breadcrumbs/dso-name.service'; +import { RestResponse } from '../../../core/cache/response.models'; import { BitstreamDataService } from '../../../core/data/bitstream-data.service'; import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; import { ItemRequestDataService } from '../../../core/data/item-request-data.service'; +import { RequestService } from '../../../core/data/request.service'; +import { RequestEntry } from '../../../core/data/request-entry.model'; import { EPerson } from '../../../core/eperson/models/eperson.model'; import { Bitstream } from '../../../core/shared/bitstream.model'; import { Item } from '../../../core/shared/item.model'; +import { ITEM } from '../../../core/shared/item.resource-type'; import { ItemRequest } from '../../../core/shared/item-request.model'; import { DSONameServiceMock } from '../../../shared/mocks/dso-name.service.mock'; +import { getMockRequestService } from '../../../shared/mocks/request.service.mock'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { createFailedRemoteDataObject$, @@ -39,6 +47,9 @@ import { NotificationsServiceStub } from '../../../shared/testing/notifications- import { RouterStub } from '../../../shared/testing/router.stub'; import { BitstreamRequestACopyPageComponent } from './bitstream-request-a-copy-page.component'; +const mockDataServiceMap: any = new Map([ + [ITEM.value, () => import('../../../shared/testing/test-data-service.mock').then(m => m.TestDataService)], +]); describe('BitstreamRequestACopyPageComponent', () => { let component: BitstreamRequestACopyPageComponent; @@ -48,10 +59,11 @@ describe('BitstreamRequestACopyPageComponent', () => { let authorizationService: AuthorizationDataService; let activatedRoute; let router; - let itemRequestDataService; + let itemRequestDataService: ItemRequestDataService; let notificationsService; let location; let bitstreamDataService; + let requestService; let item: Item; let bitstream: Bitstream; @@ -75,8 +87,20 @@ describe('BitstreamRequestACopyPageComponent', () => { itemRequestDataService = jasmine.createSpyObj('itemRequestDataService', { requestACopy: createSuccessfulRemoteDataObject$({}), + isProtectedByCaptcha: observableOf(true), }); + requestService = Object.assign(getMockRequestService(), { + getByHref(requestHref: string) { + const responseCacheEntry = new RequestEntry(); + responseCacheEntry.response = new RestResponse(true, 200, 'OK'); + return observableOf(responseCacheEntry); + }, + removeByHrefSubstring(href: string) { + // Do nothing + }, + }) as RequestService; + location = jasmine.createSpyObj('location', { back: {}, }); @@ -124,6 +148,9 @@ describe('BitstreamRequestACopyPageComponent', () => { { provide: NotificationsService, useValue: notificationsService }, { provide: DSONameService, useValue: new DSONameServiceMock() }, { provide: BitstreamDataService, useValue: bitstreamDataService }, + { provide: Store, useValue: provideMockStore() }, + { provide: RequestService, useValue: requestService }, + { provide: APP_DATA_SERVICES_MAP, useValue: mockDataServiceMap }, ], }) .compileComponents(); @@ -246,6 +273,7 @@ describe('BitstreamRequestACopyPageComponent', () => { component.email.patchValue('user@name.org'); component.allfiles.patchValue('false'); component.message.patchValue('I would like to request a copy'); + component.captchaPayload.patchValue('payload'); component.onSubmit(); const itemRequest = Object.assign(new ItemRequest(), @@ -258,7 +286,7 @@ describe('BitstreamRequestACopyPageComponent', () => { requestMessage: 'I would like to request a copy', }); - expect(itemRequestDataService.requestACopy).toHaveBeenCalledWith(itemRequest); + expect(itemRequestDataService.requestACopy).toHaveBeenCalledWith(itemRequest, 'payload'); expect(notificationsService.success).toHaveBeenCalled(); expect(location.back).toHaveBeenCalled(); }); @@ -280,6 +308,7 @@ describe('BitstreamRequestACopyPageComponent', () => { component.email.patchValue('user@name.org'); component.allfiles.patchValue('false'); component.message.patchValue('I would like to request a copy'); + component.captchaPayload.patchValue('payload'); component.onSubmit(); const itemRequest = Object.assign(new ItemRequest(), @@ -292,7 +321,7 @@ describe('BitstreamRequestACopyPageComponent', () => { requestMessage: 'I would like to request a copy', }); - expect(itemRequestDataService.requestACopy).toHaveBeenCalledWith(itemRequest); + expect(itemRequestDataService.requestACopy).toHaveBeenCalledWith(itemRequest, 'payload'); expect(notificationsService.error).toHaveBeenCalled(); expect(location.back).not.toHaveBeenCalled(); }); diff --git a/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.ts b/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.ts index dfbcef4111..9d4975f9bb 100644 --- a/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.ts +++ b/src/app/item-page/bitstreams/request-a-copy/bitstream-request-a-copy-page.component.ts @@ -1,9 +1,13 @@ +import 'altcha'; + import { AsyncPipe, Location, } from '@angular/common'; import { + ChangeDetectorRef, Component, + CUSTOM_ELEMENTS_SCHEMA, OnDestroy, OnInit, } from '@angular/core'; @@ -46,6 +50,7 @@ import { BitstreamDataService } from '../../../core/data/bitstream-data.service' import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { ItemRequestDataService } from '../../../core/data/item-request-data.service'; +import { ProofOfWorkCaptchaDataService } from '../../../core/data/proof-of-work-captcha-data.service'; import { EPerson } from '../../../core/eperson/models/eperson.model'; import { Bitstream } from '../../../core/shared/bitstream.model'; import { Item } from '../../../core/shared/item.model'; @@ -60,7 +65,9 @@ import { isNotEmpty, } from '../../../shared/empty.util'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { VarDirective } from '../../../shared/utils/var.directive'; import { getItemPageRoute } from '../../item-page-routing-paths'; +import { AltchaCaptchaComponent } from './altcha-captcha.component'; @Component({ selector: 'ds-bitstream-request-a-copy-page', @@ -71,7 +78,10 @@ import { getItemPageRoute } from '../../item-page-routing-paths'; AsyncPipe, ReactiveFormsModule, BtnDisabledDirective, + VarDirective, + AltchaCaptchaComponent, ], + schemas: [CUSTOM_ELEMENTS_SCHEMA], standalone: true, }) /** @@ -92,6 +102,10 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { bitstream: Bitstream; bitstreamName: string; + // Captcha settings + captchaEnabled$: Observable; + challengeHref$: Observable; + constructor(private location: Location, private translateService: TranslateService, private route: ActivatedRoute, @@ -103,6 +117,8 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { private notificationsService: NotificationsService, private dsoNameService: DSONameService, private bitstreamService: BitstreamDataService, + private captchaService: ProofOfWorkCaptchaDataService, + private changeDetectorRef: ChangeDetectorRef, ) { } @@ -117,8 +133,15 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { }), allfiles: new UntypedFormControl(''), message: new UntypedFormControl(''), + // Payload here is initialised as "required", but this validator will be cleared + // if the config property comes back as 'captcha not enabled' + captchaPayload: new UntypedFormControl('', { + validators: [Validators.required], + }), }); + this.captchaEnabled$ = this.itemRequestDataService.isProtectedByCaptcha(); + this.challengeHref$ = this.captchaService.getChallengeHref(); this.item$ = this.route.data.pipe( map((data) => data.dso), @@ -172,6 +195,10 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { return this.requestCopyForm.get('allfiles'); } + get captchaPayload() { + return this.requestCopyForm.get('captchaPayload'); + } + /** * Initialise the form values based on the current user. */ @@ -185,6 +212,17 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { this.bitstream$.pipe(take(1)).subscribe((bitstream) => { this.requestCopyForm.patchValue({ allfiles: 'false' }); }); + this.subs.push(this.captchaEnabled$.pipe( + take(1), + ).subscribe((enabled) => { + if (!enabled) { + // Captcha not required? Clear validators to allow the form to be submitted normally + this.requestCopyForm.get('captchaPayload').clearValidators(); + this.requestCopyForm.get('captchaPayload').reset(); + this.requestCopyForm.updateValueAndValidity(); + } + this.changeDetectorRef.detectChanges(); + })); } /** @@ -218,8 +256,9 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { itemRequest.requestEmail = this.email.value; itemRequest.requestName = this.name.value; itemRequest.requestMessage = this.message.value; + const captchaPayloadString: string = this.captchaPayload.value; - this.itemRequestDataService.requestACopy(itemRequest).pipe( + this.itemRequestDataService.requestACopy(itemRequest, captchaPayloadString).pipe( getFirstCompletedRemoteData(), ).subscribe((rd) => { if (rd.hasSucceeded) { @@ -231,6 +270,10 @@ export class BitstreamRequestACopyPageComponent implements OnInit, OnDestroy { }); } + handlePayload(event): void { + this.requestCopyForm.patchValue({ captchaPayload: event }); + } + ngOnDestroy(): void { if (hasValue(this.subs)) { this.subs.forEach((sub) => { diff --git a/src/app/request-copy/email-request-copy/email-request-copy.component.html b/src/app/request-copy/email-request-copy/email-request-copy.component.html index 50e3e73470..473d976d0d 100644 --- a/src/app/request-copy/email-request-copy/email-request-copy.component.html +++ b/src/app/request-copy/email-request-copy/email-request-copy.component.html @@ -1,7 +1,8 @@
- + @if (!subject || subject.length === 0) {
{{ 'grant-deny-request-copy.email.subject.empty' | translate }} @@ -12,18 +13,46 @@
+ + @if (hasValue(validAccessPeriods) && validAccessPeriods.length > 0) { +
+ +
+ + + +
+ @for (accessPeriod of validAccessPeriods; track accessPeriod) { + + } +
+
+
+ }
diff --git a/src/app/request-copy/email-request-copy/email-request-copy.component.spec.ts b/src/app/request-copy/email-request-copy/email-request-copy.component.spec.ts index 2e84a9a9c1..676f987c0f 100644 --- a/src/app/request-copy/email-request-copy/email-request-copy.component.spec.ts +++ b/src/app/request-copy/email-request-copy/email-request-copy.component.spec.ts @@ -45,6 +45,7 @@ describe('EmailRequestCopyComponent', () => { spyOn(component.send, 'emit').and.stub(); component.subject = 'test-subject'; component.message = 'test-message'; + component.validAccessPeriods = [0]; component.submit(); expect(component.send.emit).toHaveBeenCalledWith(new RequestCopyEmail('test-subject', 'test-message')); }); diff --git a/src/app/request-copy/email-request-copy/email-request-copy.component.ts b/src/app/request-copy/email-request-copy/email-request-copy.component.ts index fc9d42aa60..bb455f7fca 100644 --- a/src/app/request-copy/email-request-copy/email-request-copy.component.ts +++ b/src/app/request-copy/email-request-copy/email-request-copy.component.ts @@ -1,3 +1,5 @@ +import 'altcha'; + import { Location, NgClass, @@ -6,30 +8,33 @@ import { Component, EventEmitter, Input, + OnInit, Output, } from '@angular/core'; import { FormsModule } from '@angular/forms'; +import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; import { TranslateModule } from '@ngx-translate/core'; import { BtnDisabledDirective } from '../../shared/btn-disabled.directive'; +import { hasValue } from '../../shared/empty.util'; import { RequestCopyEmail } from './request-copy-email.model'; - @Component({ selector: 'ds-base-email-request-copy', styleUrls: ['./email-request-copy.component.scss'], templateUrl: './email-request-copy.component.html', standalone: true, - imports: [FormsModule, NgClass, TranslateModule, BtnDisabledDirective], + imports: [FormsModule, NgClass, TranslateModule, BtnDisabledDirective, NgbDropdownModule], }) /** * A form component for an email to send back to the user requesting an item */ -export class EmailRequestCopyComponent { +export class EmailRequestCopyComponent implements OnInit { /** * Event emitter for sending the email */ @Output() send: EventEmitter = new EventEmitter(); + @Output() selectedAccessPeriod: EventEmitter = new EventEmitter(); /** * The subject of the email @@ -41,9 +46,28 @@ export class EmailRequestCopyComponent { */ @Input() message: string; + /** + * A list of valid access periods to render in a drop-down menu + */ + @Input() validAccessPeriods: number[] = []; + + /** + * The selected access period + */ + accessPeriod = 0; + + protected readonly hasValue = hasValue; + constructor(protected location: Location) { } + ngOnInit(): void { + // If access periods are present, set the default to the first in the array + if (hasValue(this.validAccessPeriods) && this.validAccessPeriods.length > 0) { + this.selectAccessPeriod(this.validAccessPeriods[0]); + } + } + /** * Submit the email */ @@ -57,4 +81,14 @@ export class EmailRequestCopyComponent { return() { this.location.back(); } + + /** + * Update the access period when a dropdown menu button is clicked for a value + * @param accessPeriod + */ + selectAccessPeriod(accessPeriod: number) { + this.accessPeriod = accessPeriod; + this.selectedAccessPeriod.emit(accessPeriod); + } + } diff --git a/src/app/request-copy/email-request-copy/themed-email-request-copy.component.ts b/src/app/request-copy/email-request-copy/themed-email-request-copy.component.ts index f641045ef2..a767704f55 100644 --- a/src/app/request-copy/email-request-copy/themed-email-request-copy.component.ts +++ b/src/app/request-copy/email-request-copy/themed-email-request-copy.component.ts @@ -25,6 +25,11 @@ export class ThemedEmailRequestCopyComponent extends ThemedComponent = new EventEmitter(); + /** + * Event emitter for a selected / changed access period + */ + @Output() selectedAccessPeriod: EventEmitter = new EventEmitter(); + /** * The subject of the email */ @@ -35,7 +40,13 @@ export class ThemedEmailRequestCopyComponent extends ThemedComponent -

{{'grant-deny-request-copy.header' | translate}}

+

{{ 'grant-deny-request-copy.header' | translate }}

+ @if (itemRequestRD && itemRequestRD.hasSucceeded) {
- @if (!itemRequestRD.payload.decisionDate) { + + @if (!itemRequestRD.payload.decisionDate || (itemRequestRD.payload.acceptRequest === true && itemRequestRD.payload.accessToken)) {
-

-

{{'grant-deny-request-copy.intro2' | translate}}

+

+

{{ 'grant-deny-request-copy.intro2' | translate }}

+ @if (itemRequestRD.payload.decisionDate) { +

{{ 'grant-deny-request-copy.previous-decision' | translate }}

+ }
- - {{'grant-deny-request-copy.grant' | translate }} - - - {{'grant-deny-request-copy.deny' | translate }} - + + @if (!itemRequestRD.payload.decisionDate) { + + {{ 'grant-deny-request-copy.grant' | translate }} + + + + {{ 'grant-deny-request-copy.deny' | translate }} + + } + @if (itemRequestRD.payload.decisionDate && itemRequestRD.payload.acceptRequest === true && itemRequestRD.payload.accessToken) { + + {{ 'grant-deny-request-copy.revoke' | translate }} + + }
- } - @if (itemRequestRD.payload.decisionDate) { -
-

{{'grant-deny-request-copy.processed' | translate}}

-

- {{'grant-deny-request-copy.home-page' | translate}} -

-
+ + @if (itemRequestRD.payload.decisionDate && (itemRequestRD.payload.acceptRequest === false || !itemRequestRD.payload.accessToken)) { +
+

{{ 'grant-deny-request-copy.processed' | translate }}

+

+ {{ 'grant-deny-request-copy.home-page' | translate }} +

+
+ } }
} diff --git a/src/app/request-copy/grant-request-copy/grant-request-copy.component.html b/src/app/request-copy/grant-request-copy/grant-request-copy.component.html index c85f9005b6..715008e746 100644 --- a/src/app/request-copy/grant-request-copy/grant-request-copy.component.html +++ b/src/app/request-copy/grant-request-copy/grant-request-copy.component.html @@ -1,14 +1,37 @@
-

{{'grant-request-copy.header' | translate}}

+

{{ 'grant-request-copy.header' | translate }}

@if (itemRequestRD && itemRequestRD.hasSucceeded) {
-

{{'grant-request-copy.intro' | translate}}

- + +

{{ 'grant-request-copy.intro.' + (sendAsAttachment ? 'attachment' : 'link') | translate }}

+ + @if (!sendAsAttachment && hasValue(previewLink)) { +
+

{{ 'grant-request-copy.intro.link.preview' | translate }} + + {{ previewLink }} + +

+
+ } + + +

{{ 'grant-deny-request-copy.email.permissions.info' | translate }}

- - + +
diff --git a/src/app/request-copy/grant-request-copy/grant-request-copy.component.spec.ts b/src/app/request-copy/grant-request-copy/grant-request-copy.component.spec.ts index e9865d4dc3..7d5e92e87e 100644 --- a/src/app/request-copy/grant-request-copy/grant-request-copy.component.spec.ts +++ b/src/app/request-copy/grant-request-copy/grant-request-copy.component.spec.ts @@ -20,6 +20,7 @@ import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { ItemDataService } from '../../core/data/item-data.service'; import { ItemRequestDataService } from '../../core/data/item-request-data.service'; import { EPerson } from '../../core/eperson/models/eperson.model'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; import { Item } from '../../core/shared/item.model'; import { ItemRequest } from '../../core/shared/item-request.model'; import { DSONameServiceMock } from '../../shared/mocks/dso-name.service.mock'; @@ -46,6 +47,7 @@ describe('GrantRequestCopyComponent', () => { let itemDataService: ItemDataService; let itemRequestService: ItemRequestDataService; let notificationsService: NotificationsService; + let hardRedirectService: HardRedirectService; let itemRequest: ItemRequest; let user: EPerson; @@ -90,7 +92,6 @@ describe('GrantRequestCopyComponent', () => { ], }, }); - router = jasmine.createSpyObj('router', { navigateByUrl: jasmine.createSpy('navigateByUrl'), }); @@ -106,11 +107,17 @@ describe('GrantRequestCopyComponent', () => { itemDataService = jasmine.createSpyObj('itemDataService', { findById: createSuccessfulRemoteDataObject$(item), }); - itemRequestService = jasmine.createSpyObj('itemRequestService', { + itemRequestService = jasmine.createSpyObj('ItemRequestDataService', { + getSanitizedRequestByAccessToken: observableOf(createSuccessfulRemoteDataObject(itemRequest)), grant: createSuccessfulRemoteDataObject$(itemRequest), + getConfiguredAccessPeriods: observableOf([3600, 7200, 14400]), // Common access periods in seconds + }); + + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + getAuthenticatedUserFromStore: observableOf(user), }); notificationsService = jasmine.createSpyObj('notificationsService', ['success', 'error']); - return TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([]), GrantRequestCopyComponent, VarDirective], providers: [ @@ -121,6 +128,7 @@ describe('GrantRequestCopyComponent', () => { { provide: DSONameService, useValue: new DSONameServiceMock() }, { provide: ItemRequestDataService, useValue: itemRequestService }, { provide: NotificationsService, useValue: notificationsService }, + { provide: HardRedirectService, useValue: hardRedirectService }, { provide: ThemeService, useValue: getMockThemeService() }, ], schemas: [NO_ERRORS_SCHEMA], diff --git a/src/app/request-copy/grant-request-copy/grant-request-copy.component.ts b/src/app/request-copy/grant-request-copy/grant-request-copy.component.ts index 4a71226919..1455875b4e 100644 --- a/src/app/request-copy/grant-request-copy/grant-request-copy.component.ts +++ b/src/app/request-copy/grant-request-copy/grant-request-copy.component.ts @@ -1,4 +1,8 @@ -import { AsyncPipe } from '@angular/common'; +import { + AsyncPipe, + CommonModule, + NgClass, +} from '@angular/common'; import { Component, OnInit, @@ -7,6 +11,7 @@ import { FormsModule } from '@angular/forms'; import { ActivatedRoute, Router, + RouterLink, } from '@angular/router'; import { TranslateModule, @@ -16,17 +21,21 @@ import { Observable } from 'rxjs'; import { map, switchMap, + tap, } from 'rxjs/operators'; +import { getAccessTokenRequestRoute } from '../../app-routing-paths'; import { AuthService } from '../../core/auth/auth.service'; import { ItemRequestDataService } from '../../core/data/item-request-data.service'; import { RemoteData } from '../../core/data/remote-data'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; import { redirectOn4xx } from '../../core/shared/authorized.operators'; import { ItemRequest } from '../../core/shared/item-request.model'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload, } from '../../core/shared/operators'; +import { hasValue } from '../../shared/empty.util'; import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { VarDirective } from '../../shared/utils/var.directive'; @@ -38,7 +47,7 @@ import { ThemedEmailRequestCopyComponent } from '../email-request-copy/themed-em styleUrls: ['./grant-request-copy.component.scss'], templateUrl: './grant-request-copy.component.html', standalone: true, - imports: [VarDirective, ThemedEmailRequestCopyComponent, FormsModule, ThemedLoadingComponent, AsyncPipe, TranslateModule], + imports: [CommonModule, VarDirective, NgIf, ThemedEmailRequestCopyComponent, FormsModule, ThemedLoadingComponent, AsyncPipe, TranslateModule, RouterLink, NgClass], }) /** * Component for granting an item request @@ -59,11 +68,39 @@ export class GrantRequestCopyComponent implements OnInit { message$: Observable; /** - * Whether or not the item should be open access, to avoid future requests + * Whether the item should be open access, to avoid future requests * Defaults to false */ suggestOpenAccess = false; + /** + * A list of integers determining valid access periods in seconds + */ + validAccessPeriods$: Observable; + + /** + * The currently selected access period + */ + accessPeriod: any = 0; + + /** + * Will this email attach file(s) directly, or send a secure link with an access token to provide temporary access? + * This will be false if the access token is populated, since the configuration and min file size checks are + * done at the time of request creation, with a default of true. + */ + sendAsAttachment = true; + + /** + * Preview link to be sent to a request applicant + */ + previewLinkOptions: { + routerLink: string, + queryParams: any, + }; + previewLink: string; + + protected readonly hasValue = hasValue; + constructor( private router: Router, private route: ActivatedRoute, @@ -71,17 +108,33 @@ export class GrantRequestCopyComponent implements OnInit { private translateService: TranslateService, private itemRequestService: ItemRequestDataService, private notificationsService: NotificationsService, + private hardRedirectService: HardRedirectService, ) { } ngOnInit(): void { + // Get item request data via the router (async) this.itemRequestRD$ = this.route.data.pipe( map((data) => data.request as RemoteData), getFirstCompletedRemoteData(), + tap((rd) => { + // If an access token is present then the backend has checked configuration and file sizes + // and appropriately created a token to use with a secure link instead of attaching file directly + if (rd.hasSucceeded && hasValue(rd.payload.accessToken)) { + this.sendAsAttachment = false; + this.previewLinkOptions = getAccessTokenRequestRoute(rd.payload.itemId, rd.payload.accessToken); + this.previewLink = this.hardRedirectService.getCurrentOrigin() + + this.previewLinkOptions.routerLink + '?accessToken=' + rd.payload.accessToken; + } + }), redirectOn4xx(this.router, this.authService), ); + // Get configured access periods + this.validAccessPeriods$ = this.itemRequestService.getConfiguredAccessPeriods(); + + // Get the subject line of the email this.subject$ = this.translateService.get('grant-request-copy.email.subject'); } @@ -92,7 +145,7 @@ export class GrantRequestCopyComponent implements OnInit { grant(email: RequestCopyEmail) { this.itemRequestRD$.pipe( getFirstSucceededRemoteDataPayload(), - switchMap((itemRequest: ItemRequest) => this.itemRequestService.grant(itemRequest.token, email, this.suggestOpenAccess)), + switchMap((itemRequest: ItemRequest) => this.itemRequestService.grant(itemRequest.token, email, this.suggestOpenAccess, this.accessPeriod)), getFirstCompletedRemoteData(), ).subscribe((rd) => { if (rd.hasSucceeded) { @@ -104,4 +157,8 @@ export class GrantRequestCopyComponent implements OnInit { }); } + selectAccessPeriod(accessPeriod: number) { + this.accessPeriod = accessPeriod; + } + }