From deabffd429517391e4f5fbf6b9639a9cd8ec6755 Mon Sep 17 00:00:00 2001 From: Samuel Date: Tue, 7 Jul 2020 14:31:14 +0200 Subject: [PATCH 1/5] fix #630 Item delete broken --- .../item-delete/item-delete.component.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts b/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts index eecbdf8c6f..fd29ab1aca 100644 --- a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts +++ b/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts @@ -1,5 +1,5 @@ import {Component, Input, OnInit} from '@angular/core'; -import {filter, first, map, switchMap, take} from 'rxjs/operators'; +import { defaultIfEmpty, filter, first, map, switchMap, take } from 'rxjs/operators'; import {AbstractSimpleItemActionComponent} from '../simple-item-action/abstract-simple-item-action.component'; import {getItemEditPath} from '../../item-page-routing.module'; import {NgbModal, NgbModalRef} from '@ng-bootstrap/ng-bootstrap'; @@ -19,6 +19,8 @@ import {TranslateService} from '@ngx-translate/core'; import {ObjectUpdatesService} from '../../../core/data/object-updates/object-updates.service'; import {RelationshipService} from '../../../core/data/relationship.service'; import {EntityTypeService} from '../../../core/data/entity-type.service'; +import { LinkService } from "../../../core/cache/builders/link.service"; +import { followLink } from "../../../shared/utils/follow-link-config.model"; @Component({ selector: 'ds-item-delete', @@ -80,6 +82,7 @@ export class ItemDeleteComponent protected objectUpdatesService: ObjectUpdatesService, protected relationshipService: RelationshipService, protected entityTypeService: EntityTypeService, + protected linkService: LinkService, ) { super( route, @@ -187,6 +190,7 @@ export class ItemDeleteComponent observableCombineLatest( relationships.map((relationship) => this.getRelationshipType(relationship)) ).pipe( + defaultIfEmpty([]), map((types) => relationships.filter( (relationship, index) => relationshipType.id === types[index].id )), @@ -205,6 +209,12 @@ export class ItemDeleteComponent */ private getRelationshipType(relationship: Relationship): Observable { + this.linkService.resolveLinks( + relationship, + followLink('relationshipType'), + followLink('leftItem'), + followLink('rightItem'), + ); return relationship.relationshipType.pipe( getSucceededRemoteData(), getRemoteDataPayload(), @@ -305,6 +315,7 @@ export class ItemDeleteComponent combineLatest( types.map((type) => this.isSelected(type)) ).pipe( + defaultIfEmpty([]), map((selection) => types.filter( (type, index) => selection[index] )), From 52289d8d2340797b92f22c39680f65f5c5ebfad7 Mon Sep 17 00:00:00 2001 From: Samuel Date: Wed, 8 Jul 2020 12:02:25 +0200 Subject: [PATCH 2/5] fix #630 Item delete broken - handle items without entity type --- .../item-delete/item-delete.component.ts | 95 +++++++++--------- .../item-relationships.component.html | 98 ++++++++++--------- .../item-relationships.component.ts | 44 +++++---- src/assets/i18n/en.json5 | 2 + 4 files changed, 128 insertions(+), 111 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts b/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts index fd29ab1aca..209ddf22a9 100644 --- a/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts +++ b/src/app/+item-page/edit-item-page/item-delete/item-delete.component.ts @@ -1,26 +1,26 @@ -import {Component, Input, OnInit} from '@angular/core'; +import { Component, Input, OnInit } from '@angular/core'; import { defaultIfEmpty, filter, first, map, switchMap, take } from 'rxjs/operators'; -import {AbstractSimpleItemActionComponent} from '../simple-item-action/abstract-simple-item-action.component'; -import {getItemEditPath} from '../../item-page-routing.module'; -import {NgbModal, NgbModalRef} from '@ng-bootstrap/ng-bootstrap'; -import {combineLatest as observableCombineLatest, combineLatest, Observable} from 'rxjs'; -import {RelationshipType} from '../../../core/shared/item-relationships/relationship-type.model'; -import {VirtualMetadata} from '../virtual-metadata/virtual-metadata.component'; -import {Relationship} from '../../../core/shared/item-relationships/relationship.model'; -import {getRemoteDataPayload, getSucceededRemoteData} from '../../../core/shared/operators'; -import {hasValue, isNotEmpty} from '../../../shared/empty.util'; -import {Item} from '../../../core/shared/item.model'; -import {MetadataValue} from '../../../core/shared/metadata.models'; -import {ViewMode} from '../../../core/shared/view-mode.model'; -import {ActivatedRoute, Router} from '@angular/router'; -import {NotificationsService} from '../../../shared/notifications/notifications.service'; -import {ItemDataService} from '../../../core/data/item-data.service'; -import {TranslateService} from '@ngx-translate/core'; -import {ObjectUpdatesService} from '../../../core/data/object-updates/object-updates.service'; -import {RelationshipService} from '../../../core/data/relationship.service'; -import {EntityTypeService} from '../../../core/data/entity-type.service'; -import { LinkService } from "../../../core/cache/builders/link.service"; -import { followLink } from "../../../shared/utils/follow-link-config.model"; +import { AbstractSimpleItemActionComponent } from '../simple-item-action/abstract-simple-item-action.component'; +import { getItemEditPath } from '../../item-page-routing.module'; +import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; +import { combineLatest as observableCombineLatest, combineLatest, Observable, of as observableOf } from 'rxjs'; +import { RelationshipType } from '../../../core/shared/item-relationships/relationship-type.model'; +import { VirtualMetadata } from '../virtual-metadata/virtual-metadata.component'; +import { Relationship } from '../../../core/shared/item-relationships/relationship.model'; +import { getRemoteDataPayload, getSucceededRemoteData } from '../../../core/shared/operators'; +import { hasValue, isNotEmpty } from '../../../shared/empty.util'; +import { Item } from '../../../core/shared/item.model'; +import { MetadataValue } from '../../../core/shared/metadata.models'; +import { ViewMode } from '../../../core/shared/view-mode.model'; +import { ActivatedRoute, Router } from '@angular/router'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { ItemDataService } from '../../../core/data/item-data.service'; +import { TranslateService } from '@ngx-translate/core'; +import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service'; +import { RelationshipService } from '../../../core/data/relationship.service'; +import { EntityTypeService } from '../../../core/data/entity-type.service'; +import { LinkService } from '../../../core/cache/builders/link.service'; +import { followLink } from '../../../shared/utils/follow-link-config.model'; @Component({ selector: 'ds-item-delete', @@ -101,30 +101,33 @@ export class ItemDeleteComponent super.ngOnInit(); this.url = this.router.url; - this.types$ = this.entityTypeService.getEntityTypeByLabel( - this.item.firstMetadataValue('relationship.type') - ).pipe( - getSucceededRemoteData(), - getRemoteDataPayload(), - switchMap((entityType) => this.entityTypeService.getEntityTypeRelationships(entityType.id)), - getSucceededRemoteData(), - getRemoteDataPayload(), - map((relationshipTypes) => relationshipTypes.page), - switchMap((types) => - combineLatest(types.map((type) => this.getRelationships(type))).pipe( - map((relationships) => - types.reduce((includedTypes, type, index) => { - if (!includedTypes.some((includedType) => includedType.id === type.id) - && !(relationships[index].length === 0)) { - return [...includedTypes, type]; - } else { - return includedTypes; - } - }, []) - ), - ) - ), - ); + const label = this.item.firstMetadataValue('relationship.type'); + if (label !== undefined) { + this.types$ = this.entityTypeService.getEntityTypeByLabel(label).pipe( + getSucceededRemoteData(), + getRemoteDataPayload(), + switchMap((entityType) => this.entityTypeService.getEntityTypeRelationships(entityType.id)), + getSucceededRemoteData(), + getRemoteDataPayload(), + map((relationshipTypes) => relationshipTypes.page), + switchMap((types) => + combineLatest(types.map((type) => this.getRelationships(type))).pipe( + map((relationships) => + types.reduce((includedTypes, type, index) => { + if (!includedTypes.some((includedType) => includedType.id === type.id) + && !(relationships[index].length === 0)) { + return [...includedTypes, type]; + } else { + return includedTypes; + } + }, []) + ), + ) + ), + ); + } else { + this.types$ = observableOf([]); + } this.types$.pipe( take(1), diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html index 384a469f24..0c9d92dfbf 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html @@ -1,48 +1,56 @@
-
- - - -
-
- -
-
-
- - - + + +
+ + + +
+
+ +
+
+
+ + + +
+
+ + -
+
diff --git a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts index 2634b4e262..0e228dc246 100644 --- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts +++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts @@ -3,7 +3,7 @@ import { Item } from '../../../core/shared/item.model'; import { DeleteRelationship, FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer'; import { Observable } from 'rxjs/internal/Observable'; import { filter, map, switchMap, take } from 'rxjs/operators'; -import { zip as observableZip } from 'rxjs'; +import { of as observableOf, zip as observableZip} from 'rxjs'; import { followLink } from '../../../shared/utils/follow-link-config.model'; import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component'; import { ItemDataService } from '../../../core/data/item-data.service'; @@ -87,26 +87,30 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent impl */ public initializeUpdates(): void { - this.entityType$ = this.entityTypeService.getEntityTypeByLabel( - this.item.firstMetadataValue('relationship.type') - ).pipe( - getSucceededRemoteData(), - getRemoteDataPayload(), - ); + const label = this.item.firstMetadataValue('relationship.type'); + if (label !== undefined) { - this.relationshipTypes$ = this.entityType$.pipe( - switchMap((entityType) => - this.entityTypeService.getEntityTypeRelationships( - entityType.id, - followLink('leftType'), - followLink('rightType')) - .pipe( - getSucceededRemoteData(), - getRemoteDataPayload(), - map((relationshipTypes) => relationshipTypes.page), - ) - ), - ); + this.entityType$ = this.entityTypeService.getEntityTypeByLabel(label).pipe( + getSucceededRemoteData(), + getRemoteDataPayload(), + ); + + this.relationshipTypes$ = this.entityType$.pipe( + switchMap((entityType) => + this.entityTypeService.getEntityTypeRelationships( + entityType.id, + followLink('leftType'), + followLink('rightType')) + .pipe( + getSucceededRemoteData(), + getRemoteDataPayload(), + map((relationshipTypes) => relationshipTypes.page), + ) + ), + ); + } else { + this.entityType$ = observableOf(undefined); + } } /** diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 71b5e2c0cc..da4a6c0952 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1378,6 +1378,8 @@ "item.edit.relationships.save-button": "Save", + "item.edit.relationships.no-entity-type": "Add 'relationship.type' metadata to enable relationships for this item", + "item.edit.tabs.bitstreams.head": "Bitstreams", From 5a1b82d93cd2cc58c145426e319f0cd853798b45 Mon Sep 17 00:00:00 2001 From: Samuel Date: Wed, 8 Jul 2020 13:45:53 +0200 Subject: [PATCH 3/5] fix #630 Item delete broken - repair tests --- .../process-page/form/process-form.component.spec.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/app/process-page/form/process-form.component.spec.ts b/src/app/process-page/form/process-form.component.spec.ts index 12326111da..5e27ff3de4 100644 --- a/src/app/process-page/form/process-form.component.spec.ts +++ b/src/app/process-page/form/process-form.component.spec.ts @@ -19,6 +19,7 @@ describe('ProcessFormComponent', () => { let component: ProcessFormComponent; let fixture: ComponentFixture; let scriptService; + let router; let parameterValues; let script; @@ -41,7 +42,10 @@ describe('ProcessFormComponent', () => { } }) } - ) + ); + router = { + navigateByUrl: () => undefined, + }; } beforeEach(async(() => { @@ -59,8 +63,8 @@ describe('ProcessFormComponent', () => { providers: [ { provide: ScriptDataService, useValue: scriptService }, { provide: NotificationsService, useClass: NotificationsServiceStub }, - { provide: RequestService, useValue: jasmine.createSpyObj('requestService', ['removeBySubstring']) }, - { provide: Router, useValue: {} }, + { provide: RequestService, useValue: jasmine.createSpyObj('requestService', ['removeByHrefSubstring']) }, + { provide: Router, useValue: router }, ], schemas: [NO_ERRORS_SCHEMA] }) From 0fa1e17078f705ed84418d1e0043bd3474c79605 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Mon, 13 Jul 2020 15:47:59 +0200 Subject: [PATCH 4/5] 71809: ForwardClientIpInterceptor --- src/app/core/auth/server-auth.service.ts | 3 -- .../forward-client-ip.interceptor.spec.ts | 44 +++++++++++++++++++ .../forward-client-ip.interceptor.ts | 23 ++++++++++ src/modules/app/server-app.module.ts | 10 ++++- 4 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 src/app/core/forward-client-ip/forward-client-ip.interceptor.spec.ts create mode 100644 src/app/core/forward-client-ip/forward-client-ip.interceptor.ts diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index 30767be85a..7b78255001 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -27,9 +27,6 @@ export class ServerAuthService extends AuthService { headers = headers.append('Accept', 'application/json'); headers = headers.append('Authorization', `Bearer ${token.accessToken}`); - // NB this is used to pass server client IP check. - const clientIp = this.req.get('x-forwarded-for') || this.req.connection.remoteAddress; - headers = headers.append('X-Forwarded-For', clientIp); options.headers = headers; return this.authRequestService.getRequest('status', options).pipe( diff --git a/src/app/core/forward-client-ip/forward-client-ip.interceptor.spec.ts b/src/app/core/forward-client-ip/forward-client-ip.interceptor.spec.ts new file mode 100644 index 0000000000..49acd5b46d --- /dev/null +++ b/src/app/core/forward-client-ip/forward-client-ip.interceptor.spec.ts @@ -0,0 +1,44 @@ +import { ForwardClientIpInterceptor } from './forward-client-ip.interceptor'; +import { DSpaceRESTv2Service } from '../dspace-rest-v2/dspace-rest-v2.service'; +import { TestBed } from '@angular/core/testing'; +import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; +import { HTTP_INTERCEPTORS, HttpRequest } from '@angular/common/http'; +import { REQUEST } from '@nguniversal/express-engine/tokens'; + +describe('ForwardClientIpInterceptor', () => { + let service: DSpaceRESTv2Service; + let httpMock: HttpTestingController; + + let requestUrl; + let clientIp; + + beforeEach(() => { + requestUrl = 'test-url'; + clientIp = '1.2.3.4'; + + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [ + DSpaceRESTv2Service, + { + provide: HTTP_INTERCEPTORS, + useClass: ForwardClientIpInterceptor, + multi: true, + }, + { provide: REQUEST, useValue: { get: () => undefined, connection: { remoteAddress: clientIp } }} + ], + }); + + service = TestBed.get(DSpaceRESTv2Service); + httpMock = TestBed.get(HttpTestingController); + }); + + it('should add an X-Forwarded-For header matching the client\'s IP', () => { + service.get(requestUrl).subscribe((response) => { + expect(response).toBeTruthy(); + }); + + const httpRequest = httpMock.expectOne(requestUrl); + expect(httpRequest.request.headers.get('X-Forwarded-For')).toEqual(clientIp); + }); +}); diff --git a/src/app/core/forward-client-ip/forward-client-ip.interceptor.ts b/src/app/core/forward-client-ip/forward-client-ip.interceptor.ts new file mode 100644 index 0000000000..2e1be09c96 --- /dev/null +++ b/src/app/core/forward-client-ip/forward-client-ip.interceptor.ts @@ -0,0 +1,23 @@ +import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http'; +import { Inject, Injectable } from '@angular/core'; +import { Observable } from 'rxjs/internal/Observable'; +import { REQUEST } from '@nguniversal/express-engine/tokens'; + +@Injectable() +/** + * Http Interceptor intercepting Http Requests, adding the client's IP to their X-Forwarded-For header + */ +export class ForwardClientIpInterceptor implements HttpInterceptor { + constructor(@Inject(REQUEST) protected req: any) { + } + + /** + * Intercept http requests and add the client's IP to the X-Forwarded-For header + * @param httpRequest + * @param next + */ + intercept(httpRequest: HttpRequest, next: HttpHandler): Observable> { + const clientIp = this.req.get('x-forwarded-for') || this.req.connection.remoteAddress; + return next.handle(httpRequest.clone({ setHeaders: { 'X-Forwarded-For': clientIp } })); + } +} diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index 5abd8e3aa1..d46a013b3b 100644 --- a/src/modules/app/server-app.module.ts +++ b/src/modules/app/server-app.module.ts @@ -25,6 +25,8 @@ import { ServerSubmissionService } from '../../app/submission/server-submission. import { Angulartics2DSpace } from '../../app/statistics/angulartics/dspace-provider'; import { Angulartics2RouterlessModule } from 'angulartics2/routerlessmodule'; import { ModuleMapLoaderModule } from '@nguniversal/module-map-ngfactory-loader'; +import { HTTP_INTERCEPTORS } from '@angular/common/http'; +import { ForwardClientIpInterceptor } from '../../app/core/forward-client-ip/forward-client-ip.interceptor'; export function createTranslateLoader() { return new TranslateJson5UniversalLoader('dist/server/assets/i18n/', '.json5'); @@ -73,7 +75,13 @@ export function createTranslateLoader() { { provide: SubmissionService, useClass: ServerSubmissionService - } + }, + // register ForwardClientIpInterceptor as HttpInterceptor + { + provide: HTTP_INTERCEPTORS, + useClass: ForwardClientIpInterceptor, + multi: true + }, ] }) export class ServerAppModule { From cf2759f6ca6d9eee6d873fbace89afc9d1f90b45 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 16 Jul 2020 14:20:15 -0500 Subject: [PATCH 5/5] Update and rename .github/workflows/pull_request_opened.yml to .github/disabled-workflows/pull_request_opened.yml --- .../{workflows => disabled-workflows}/pull_request_opened.yml | 2 ++ 1 file changed, 2 insertions(+) rename .github/{workflows => disabled-workflows}/pull_request_opened.yml (78%) diff --git a/.github/workflows/pull_request_opened.yml b/.github/disabled-workflows/pull_request_opened.yml similarity index 78% rename from .github/workflows/pull_request_opened.yml rename to .github/disabled-workflows/pull_request_opened.yml index 20cc3cd65c..0dc718c0b9 100644 --- a/.github/workflows/pull_request_opened.yml +++ b/.github/disabled-workflows/pull_request_opened.yml @@ -1,4 +1,6 @@ # This workflow runs whenever a new pull request is created +# TEMPORARILY DISABLED. Unfortunately this doesn't work for PRs created from forked repositories (which is how we tend to create PRs). +# There is no known workaround yet. See https://github.community/t/how-to-use-github-token-for-prs-from-forks/16818 name: Pull Request opened # Only run for newly opened PRs against the "main" branch