Skip to content

Commit 672d206

Browse files
committed
Merged in UXP-98-2023_02_x (pull request DSpace#1144)
Fixes security issue with ProxyResourceController
2 parents a12b947 + 9ef5b6c commit 672d206

9 files changed

Lines changed: 381 additions & 365 deletions

File tree

src/app/core/submission/models/workspaceitem-section-unpaywall-object.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,6 @@ export interface WorkspaceitemSectionUnpaywallObject {
2525
*/
2626
status: UnpaywallSectionStatus;
2727

28-
/**
29-
* Item json record.
30-
*/
31-
jsonRecord: string;
32-
3328
/**
3429
* Timestamp created.
3530
*/

src/app/item-page/edit-item-page/edit-item-page.module.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { NgModule } from '@angular/core';
22
import { CommonModule } from '@angular/common';
33

4-
import { NgbTooltipModule, NgbModule } from '@ng-bootstrap/ng-bootstrap';
4+
import { NgbModule, NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap';
55

66
import { SharedModule } from '../../shared/shared.module';
77
import { EditItemPageRoutingModule } from './edit-item-page.routing.module';
@@ -20,14 +20,22 @@ import { SearchPageModule } from '../../search-page/search-page.module';
2020
import { ItemCollectionMapperComponent } from './item-collection-mapper/item-collection-mapper.component';
2121
import { ItemRelationshipsComponent } from './item-relationships/item-relationships.component';
2222
import { EditRelationshipComponent } from './item-relationships/edit-relationship/edit-relationship.component';
23-
import { EditRelationshipListComponent } from './item-relationships/edit-relationship-list/edit-relationship-list.component';
23+
import {
24+
EditRelationshipListComponent
25+
} from './item-relationships/edit-relationship-list/edit-relationship-list.component';
2426
import { AbstractItemUpdateComponent } from './abstract-item-update/abstract-item-update.component';
2527
import { ItemMoveComponent } from './item-move/item-move.component';
26-
import { ItemEditBitstreamBundleComponent } from './item-bitstreams/item-edit-bitstream-bundle/item-edit-bitstream-bundle.component';
28+
import {
29+
ItemEditBitstreamBundleComponent
30+
} from './item-bitstreams/item-edit-bitstream-bundle/item-edit-bitstream-bundle.component';
2731
import { BundleDataService } from '../../core/data/bundle-data.service';
2832
import { DragDropModule } from '@angular/cdk/drag-drop';
29-
import { ItemEditBitstreamDragHandleComponent } from './item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component';
30-
import { PaginatedDragAndDropBitstreamListComponent } from './item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component';
33+
import {
34+
ItemEditBitstreamDragHandleComponent
35+
} from './item-bitstreams/item-edit-bitstream-drag-handle/item-edit-bitstream-drag-handle.component';
36+
import {
37+
PaginatedDragAndDropBitstreamListComponent
38+
} from './item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component';
3139
import { VirtualMetadataComponent } from './virtual-metadata/virtual-metadata.component';
3240
import { ItemVersionHistoryComponent } from './item-version-history/item-version-history.component';
3341
import { ItemAuthorizationsComponent } from './item-authorizations/item-authorizations.component';
@@ -43,9 +51,7 @@ import { ThemedItemStatusComponent } from './item-status/themed-item-status.comp
4351

4452
import { ItemAccessControlComponent } from './item-access-control/item-access-control.component';
4553
import { ResultsBackButtonModule } from '../../shared/results-back-button/results-back-button.module';
46-
import {
47-
AccessControlFormModule
48-
} from '../../shared/access-control-form-container/access-control-form.module';
54+
import { AccessControlFormModule } from '../../shared/access-control-form-container/access-control-form.module';
4955
import { ItemUnlinkOrcidComponent } from './item-unlink-orcid/item-unlink-orcid.component';
5056
import { EditMetadataSecurityComponent } from './edit-metadata-security/edit-metadata-security.component';
5157
import { EditItemResolver } from '../../core/shared/resolvers/edit-item.resolver';
@@ -108,6 +114,7 @@ import { EditItemResolver } from '../../core/shared/resolvers/edit-item.resolver
108114
exports: [
109115
EditMetadataSecurityComponent,
110116
ItemOperationComponent,
117+
ItemVersionHistoryComponent,
111118
]
112119
})
113120
export class EditItemPageModule {

src/app/submission/objects/submission-objects.reducer.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ import {
4747
UpdateSectionVisibilityAction
4848
} from './submission-objects.actions';
4949
import { WorkspaceitemSectionUploadObject } from '../../core/submission/models/workspaceitem-section-upload.model';
50-
import { WorkspaceitemSectionDetectDuplicateObject } from '../../core/submission/models/workspaceitem-section-deduplication.model';
50+
import {
51+
WorkspaceitemSectionDetectDuplicateObject
52+
} from '../../core/submission/models/workspaceitem-section-deduplication.model';
5153
import { SubmissionSectionObject } from './submission-section-object.model';
5254
import { MetadataSecurityConfiguration } from '../../core/submission/models/metadata-security-configuration';
5355

@@ -889,8 +891,7 @@ function newFile(state: SubmissionObjectState, action: NewUploadedFileAction): S
889891
files: [action.payload.data]
890892
};
891893
} else {
892-
newData = filesData;
893-
newData.files.push(action.payload.data);
894+
newData = { ...filesData, files: [].concat(...filesData.files, action.payload.data) };
894895
}
895896

896897
return Object.assign({}, state, {

src/app/submission/sections/unpaywall/models/unpaywall-section-status.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@ export enum UnpaywallSectionStatus {
22
PENDING = 'PENDING',
33
NOT_FOUND = 'NOT_FOUND',
44
SUCCESSFUL = 'SUCCESSFUL',
5-
NO_FILE = 'NO_FILE'
5+
NO_FILE = 'NO_FILE',
6+
IMPORTED = 'IMPORTED',
67
}

src/app/submission/sections/unpaywall/submission-section-unpaywall.component.html

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,45 @@
1-
<div class="d-flex justify-content-center">
2-
<button (click)="refreshApiCheck()" [disabled]="(loading$ | async)" type="button" class="btn btn-secondary mr-2"
3-
aria-label="refresh">
4-
<span aria-hidden="true">{{'submission.sections.unpaywall.refresh' | translate}}</span>
5-
</button>
6-
<button [disabled]="(status$ | async) !== UnpaywallSectionStatus.SUCCESSFUL || (loading$ | async)" type="button"
7-
(click)="uploadFileIfNeeded()" class="btn btn-success" aria-label="import from unpaywall">
8-
<span aria-hidden="true">{{'submission.sections.unpaywall.import' | translate}}</span>
9-
</button>
10-
</div>
11-
<br>
12-
<div class="d-flex justify-content-center">
13-
<ds-themed-loading *ngIf="(loading$ | async)" class="ds-themed-loading"></ds-themed-loading>
14-
</div>
15-
<div class="d-flex justify-content-center">
16-
<div [ngSwitch]="(status$ | async)" *ngIf="!(loading$ | async)">
17-
<div *ngSwitchCase="UnpaywallSectionStatus.SUCCESSFUL">
18-
{{'submission.sections.unpaywall.status.successful' | translate}}
19-
</div>
20-
<div *ngSwitchCase="UnpaywallSectionStatus.NOT_FOUND">
21-
{{'submission.sections.unpaywall.status.not-found' | translate}}
1+
<div class="container-fluid">
2+
<div class="row justify-content-center">
3+
<div class="col-12">
4+
<div [ngSwitch]="(status$ | async)" *ngIf="!(loading$ | async)">
5+
<ng-container *ngSwitchCase="UnpaywallSectionStatus.SUCCESSFUL">
6+
<ds-alert [type]="AlertType.Info"
7+
[content]="'submission.sections.unpaywall.status.successful' | translate"></ds-alert>
8+
</ng-container>
9+
<ng-container *ngSwitchCase="UnpaywallSectionStatus.IMPORTED">
10+
<ds-alert [type]="AlertType.Success"
11+
[content]="'submission.sections.unpaywall.status.imported' | translate"></ds-alert>
12+
</ng-container>
13+
<ng-container *ngSwitchCase="UnpaywallSectionStatus.PENDING">
14+
<ds-alert [type]="AlertType.Info"
15+
[content]="'submission.sections.unpaywall.status.pending' | translate"></ds-alert>
16+
</ng-container>
17+
<ng-container *ngSwitchCase="UnpaywallSectionStatus.NO_FILE">
18+
<ds-alert [type]="AlertType.Warning"
19+
[content]="'submission.sections.unpaywall.status.no-file' | translate"></ds-alert>
20+
</ng-container>
21+
<ng-container *ngSwitchCase="UnpaywallSectionStatus.NOT_FOUND">
22+
<ds-alert [type]="AlertType.Error"
23+
[content]="'submission.sections.unpaywall.status.not-found' | translate"></ds-alert>
24+
</ng-container>
25+
</div>
2226
</div>
23-
<div *ngSwitchCase="UnpaywallSectionStatus.PENDING">
24-
{{'submission.sections.unpaywall.status.pending' | translate}}
25-
</div>
26-
<div *ngSwitchCase="UnpaywallSectionStatus.NO_FILE">
27-
{{'submission.sections.unpaywall.status.no-file' | translate}}
27+
</div>
28+
<div class="row justify-content-center">
29+
<div class="col-12 d-flex justify-content-center">
30+
<ng-container [ngSwitch]="(loading$ | async)">
31+
<ds-themed-loading *ngSwitchCase="true" class="ds-themed-loading"></ds-themed-loading>
32+
<ng-container *ngSwitchCase="false">
33+
<button type="button" class="btn btn-secondary mr-2" aria-label="refresh"
34+
(click)="refreshApiCheck()">
35+
<span aria-hidden="true">{{'submission.sections.unpaywall.refresh' | translate}}</span>
36+
</button>
37+
<button type="button" class="btn btn-success" aria-label="import from unpaywall"
38+
(click)="confirmImport()" [disabled]="(status$ | async) !== UnpaywallSectionStatus.SUCCESSFUL">
39+
<span aria-hidden="true">{{'submission.sections.unpaywall.import' | translate}}</span>
40+
</button>
41+
</ng-container>
42+
</ng-container>
2843
</div>
2944
</div>
30-
3145
</div>
32-
33-

src/app/submission/sections/unpaywall/submission-section-unpaywall.component.spec.ts

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,9 @@ import {
4848
WorkspaceitemSectionUnpaywallObject
4949
} from '../../../core/submission/models/workspaceitem-section-unpaywall-object';
5050
import { UnpaywallSectionStatus } from './models/unpaywall-section-status';
51-
import { UnpaywallApi } from './models/unpaywall-api';
52-
import { ResourceService } from '../../../core/services/resource.service';
5351
import { of } from 'rxjs';
5452
import { AuthTokenInfo } from '../../../core/auth/models/auth-token-info.model';
5553
import { provideMockStore } from '@ngrx/store/testing';
56-
import { FileItem } from 'ng2-file-upload';
5754
import { SubmissionObject } from '../../../core/submission/models/submission-object.model';
5855
import { SectionsType } from '../sections-type';
5956
import { WorkspaceitemSectionUploadObject } from '../../../core/submission/models/workspaceitem-section-upload.model';
@@ -62,12 +59,13 @@ import {
6259
} from '../../../core/submission/models/workspaceitem-section-upload-file.model';
6360
import { RawRestResponse } from '../../../core/dspace-rest/raw-rest-response.model';
6461
import { SubmissionState } from '../../submission.reducers';
62+
import { SectionUploadService } from '../upload/section-upload.service';
63+
import { getMockSectionUploadService } from '../../../shared/mocks/section-upload.service.mock';
6564

6665
describe('SubmissionSectionUnpaywallComponentComponent', () => {
6766
let component: SubmissionSectionUnpaywallComponent;
6867
let fixture: ComponentFixture<SubmissionSectionUnpaywallComponent>;
6968
let httpMock: HttpTestingController;
70-
let resourceService: ResourceService;
7169
let submissionService: SubmissionService;
7270
let halService: HALEndpointService;
7371
let tokenExtractor: HttpXsrfTokenExtractor;
@@ -76,6 +74,7 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
7674
let translate: TranslateService;
7775
let restApi: DspaceRestService;
7876
let store: Store<SubmissionState>;
77+
let sectionUploadService: SectionUploadService;
7978
const xsrfToken = 'mock-token';
8079

8180
const initialState = {
@@ -140,6 +139,7 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
140139
{ provide: 'collectionIdProvider', useValue: mockSubmissionCollectionId },
141140
{ provide: 'sectionDataProvider', useValue: {} },
142141
{ provide: 'submissionIdProvider', useValue: mockSubmissionId },
142+
{ provide: SectionUploadService, useValue: getMockSectionUploadService() },
143143
],
144144
declarations: [SubmissionSectionUnpaywallComponent],
145145
schemas: [NO_ERRORS_SCHEMA]
@@ -149,7 +149,6 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
149149

150150
beforeEach(() => {
151151
fixture = TestBed.createComponent(SubmissionSectionUnpaywallComponent);
152-
resourceService = TestBed.inject(ResourceService);
153152
submissionService = TestBed.inject(SubmissionService);
154153
halService = TestBed.inject(HALEndpointService);
155154
httpMock = TestBed.inject(HttpTestingController);
@@ -159,6 +158,7 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
159158
translate = TestBed.inject(TranslateService);
160159
restApi = TestBed.inject(DspaceRestService);
161160
store = TestBed.inject(Store);
161+
sectionUploadService = TestBed.inject(SectionUploadService);
162162
component = fixture.componentInstance;
163163
fixture.detectChanges();
164164
});
@@ -171,19 +171,24 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
171171
expect(component).toBeTruthy();
172172
});
173173

174-
describe('uploadFileIfNeeded', () => {
175-
it('should upload file if needed', () => {
176-
const resourceUrl = 'http://resourse-url/test.txt';
177-
const resource = new Blob(['Test data'], { type: 'text/plain' });
174+
describe('import file', () => {
175+
it('should import the linked resource if needed', () => {
178176
const submissionObjectName = 'workspaceitems';
179177
const testEndpoint = 'http://test-endpoint';
180178
const successLabel = of('success-label');
181-
const unpaywallApiResp = { best_oa_location: { url: resourceUrl } } as UnpaywallApi;
182-
const submissionObject = {
179+
const uploadSection: WorkspaceitemSectionUploadObject = { files: [] };
180+
const successfulSection: WorkspaceitemSectionUnpaywallObject = {
181+
id: 1,
182+
status: UnpaywallSectionStatus.SUCCESSFUL,
183+
doi: 'test-doi',
184+
itemId: 'test-item-id',
185+
timestampCreated: new Date(),
186+
timestampLastModified: new Date()
187+
};
188+
const importedResource = {
183189
sections: {
184190
[SectionsType.Unpaywall]: {
185-
status: UnpaywallSectionStatus.SUCCESSFUL,
186-
jsonRecord: JSON.stringify(unpaywallApiResp),
191+
status: UnpaywallSectionStatus.IMPORTED,
187192
doi: 'test-doi',
188193
itemId: 'test-item-id',
189194
timestampCreated: new Date(),
@@ -196,27 +201,32 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
196201
} as WorkspaceitemSectionUploadObject
197202
}
198203
} as unknown as SubmissionObject;
199-
component.section$.next(<WorkspaceitemSectionUnpaywallObject>submissionObject.sections.unpaywall);
204+
component.unpaywallSection$.next(successfulSection);
205+
component.uploadSection$.next({ [SectionsType.Upload]: uploadSection });
206+
const requestResponse = { payload: importedResource } as unknown as RawRestResponse;
207+
jasmine.clock().install();
200208

201-
spyOn(resourceService, 'download').withArgs(resourceUrl).and.returnValue(of(resource));
202209
spyOn(submissionService, 'getSubmissionObjectLinkName').and.returnValue(submissionObjectName);
203210
spyOn(halService, 'getEndpoint').withArgs(submissionObjectName).and.returnValue(of(testEndpoint));
204211
spyOn(tokenExtractor, 'getToken').and.returnValue(xsrfToken);
205-
spyOn(component.uploader, 'uploadAll').and.callFake(() =>
206-
(component.uploader as any)._onSuccessItem(new FileItem(component.uploader, new File([resource], 'test.txt'), {}), JSON.stringify(submissionObject), null, null));
212+
spyOn(restApi, 'request').and.returnValue(of(requestResponse));
207213
spyOn(sectionService, 'isSectionType')
208214
.withArgs(mockSubmissionId, SectionsType.Unpaywall, SectionsType.Upload).and.returnValue(of(false))
209215
.withArgs(mockSubmissionId, SectionsType.Upload, SectionsType.Upload).and.returnValue(of(true));
210-
spyOn(translate, 'get').withArgs('submission.sections.upload.upload-successful').and.returnValue(successLabel);
216+
spyOn(translate, 'get').withArgs('submission.sections.unpaywall.status.imported').and.returnValue(successLabel);
217+
spyOn(component.loading$, 'next');
218+
spyOn(component.status$, 'next');
219+
spyOn(component.unpaywallSection$, 'next');
211220
spyOn(notificationsService, 'success').withArgs(null, successLabel);
212221
spyOn(notificationsService, 'error');
213-
spyOn(sectionService, 'updateSectionData').withArgs(mockSubmissionId, SectionsType.Upload, submissionObject.sections[SectionsType.Upload], undefined, undefined);
214222

215-
component.uploadFileIfNeeded();
223+
component.confirmImport();
216224

217-
expect(component.uploader.uploadAll).toHaveBeenCalledTimes(1);
218-
expect(notificationsService.success).toHaveBeenCalledTimes(1);
219-
expect(notificationsService.error).not.toHaveBeenCalled();
225+
expect(component.loading$.next).toHaveBeenCalledWith(false);
226+
expect(component.status$.next).toHaveBeenCalledOnceWith((importedResource.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject).status);
227+
expect(component.unpaywallSection$.next).toHaveBeenCalledWith(importedResource.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject);
228+
expect(sectionUploadService.addUploadedFile).toHaveBeenCalledTimes(1);
229+
jasmine.clock().uninstall();
220230
});
221231
});
222232

@@ -228,7 +238,6 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
228238
sections: {
229239
[SectionsType.Unpaywall]: {
230240
status: UnpaywallSectionStatus.SUCCESSFUL,
231-
jsonRecord: '',
232241
doi: 'test-doi',
233242
itemId: 'test-item-id',
234243
timestampCreated: new Date(),
@@ -245,15 +254,15 @@ describe('SubmissionSectionUnpaywallComponentComponent', () => {
245254
spyOn(component.loading$, 'next');
246255
spyOn(component.status$, 'next')
247256
.withArgs((submissionObject.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject).status);
248-
spyOn(component.section$, 'next')
257+
spyOn(component.unpaywallSection$, 'next')
249258
.withArgs(submissionObject.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject);
250259

251260
component.refreshApiCheck();
252261
jasmine.clock().tick(3010);
253262

254263
expect(component.loading$.next).toHaveBeenCalledWith(false);
255264
expect(component.status$.next).toHaveBeenCalledOnceWith((submissionObject.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject).status);
256-
expect(component.section$.next).toHaveBeenCalledOnceWith(submissionObject.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject);
265+
expect(component.unpaywallSection$.next).toHaveBeenCalledOnceWith(submissionObject.sections[SectionsType.Unpaywall] as WorkspaceitemSectionUnpaywallObject);
257266
jasmine.clock().uninstall();
258267
});
259268
});

0 commit comments

Comments
 (0)