Skip to content

Commit

Permalink
[DSC-802] Fix issue with accessing items without permission
Browse files Browse the repository at this point in the history
  • Loading branch information
atarix83 committed Dec 30, 2022
1 parent 5fc0085 commit effcf07
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 57 deletions.
15 changes: 15 additions & 0 deletions src/app/core/shared/authorized.operators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ export const redirectOn4xx = <T>(router: Router, authService: AuthService) =>
}),
map(([rd,]: [RemoteData<T>, boolean]) => rd)
);

export const redirectOn204 = <T>(router: Router, authService: AuthService) =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(
withLatestFrom(authService.isAuthenticated()),
filter(([rd, isAuthenticated]: [RemoteData<T>, boolean]) => {
if (rd.hasNoContent) {
router.navigateByUrl(getPageNotFoundRoute(), { skipLocationChange: true });
return false;
}
return true;
}),
map(([rd,]: [RemoteData<T>, boolean]) => rd)
);

/**
* Operator that returns a UrlTree to a forbidden page or the login page when the boolean received is false
* @param router The router used to navigate to a forbidden page
Expand Down
15 changes: 9 additions & 6 deletions src/app/cris-item-page/cris-item-page.component.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<div class="container">
<ds-item-alerts [item]="(itemRD$ | async)?.payload"></ds-item-alerts>
</div>
<ds-view-tracker [object]="(itemRD$ | async)?.payload"></ds-view-tracker>
<ds-loading *ngIf="(itemRD$ | async)?.isLoading" message="{{'loading.item' | translate}}"></ds-loading>
<ds-cris-layout *ngIf="!((itemRD$ | async)?.payload?.isWithdrawn) || (isAdmin$|async)" [item]="(itemRD$ | async)?.payload"></ds-cris-layout>
<ds-themed-loading *ngIf="(itemRD$ | async)?.isLoading" message="{{'loading.item' | translate}}"></ds-themed-loading>
<ng-container *ngIf="((itemRD$ | async)?.hasSucceeded && !(itemRD$ | async)?.hasNoContent)">
<div class="container">
<ds-item-alerts [item]="(itemRD$ | async)?.payload"></ds-item-alerts>
</div>
<ds-view-tracker [object]="(itemRD$ | async)?.payload"></ds-view-tracker>
<ds-cris-layout *ngIf="!((itemRD$ | async)?.payload?.isWithdrawn) || (isAdmin$|async)"
[item]="(itemRD$ | async)?.payload"></ds-cris-layout>
</ng-container>
33 changes: 30 additions & 3 deletions src/app/cris-item-page/cris-item-page.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';

import { CrisItemPageComponent } from './cris-item-page.component';
import { Item } from '../core/shared/item.model';
import {
createNoContentRemoteDataObject,
createPendingRemoteDataObject$,
createSuccessfulRemoteDataObject,
createSuccessfulRemoteDataObject$
Expand Down Expand Up @@ -45,7 +46,7 @@ describe('CrisItemPageComponent', () => {
data: of({ dso: createSuccessfulRemoteDataObject(mockItem) })
});

beforeEach(async(() => {
beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [TranslateModule.forRoot({
loader: {
Expand Down Expand Up @@ -79,8 +80,34 @@ describe('CrisItemPageComponent', () => {
});

it('should display a loading component', () => {
const loading = fixture.debugElement.query(By.css('ds-loading'));
const loading = fixture.debugElement.query(By.css('ds-themed-loading'));
expect(loading.nativeElement).toBeDefined();
});
});

describe('when the item is loaded', () => {
beforeEach(() => {
component.itemRD$ = createSuccessfulRemoteDataObject$(mockItem);
fixture.detectChanges();
});

it('should display the cris layout component', () => {
const layout = fixture.debugElement.query(By.css('ds-cris-layout'));
expect(layout.nativeElement).toBeDefined();
});
});

describe('when the item is no content', () => {
beforeEach(() => {
const itemRD = createNoContentRemoteDataObject<Item>();
itemRD.statusCode = 204;
component.itemRD$ = of(itemRD);
fixture.detectChanges();
});

it('should not display the cris layout component', () => {
const layout = fixture.debugElement.query(By.css('ds-cris-layout'));
expect(layout).toBeNull();
});
});
});
5 changes: 3 additions & 2 deletions src/app/cris-item-page/cris-item-page.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { map } from 'rxjs/operators';

import { RemoteData } from '../core/data/remote-data';
import { Item } from '../core/shared/item.model';
import { redirectOn4xx } from '../core/shared/authorized.operators';
import { redirectOn204, redirectOn4xx } from '../core/shared/authorized.operators';
import { fadeInOut } from '../shared/animations/fade';
import { AuthService } from '../core/auth/auth.service';
import { FeatureID } from '../core/data/feature-authorization/feature-id';
Expand Down Expand Up @@ -41,7 +41,8 @@ export class CrisItemPageComponent implements OnInit {
map((data) => {
return data.dso as RemoteData<Item>;
}),
redirectOn4xx(this.router, this.authService)
redirectOn204<Item>(this.router, this.authService),
redirectOn4xx<Item>(this.router, this.authService)
);

this.isAdmin$ = this.authorizationService.isAuthorized(FeatureID.AdministratorOf);
Expand Down
6 changes: 4 additions & 2 deletions src/app/cris-item-page/cris-item-page.resolver.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Injectable } from '@angular/core';
import { Observable } from 'rxjs';
import { Resolve, ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router';
import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/router';
import { RemoteData } from '../core/data/remote-data';
import { ItemDataService } from '../core/data/item-data.service';
import { followLink } from '../shared/utils/follow-link-config.model';
Expand Down Expand Up @@ -32,7 +32,9 @@ export class CrisItemPageResolver implements Resolve<RemoteData<Item>> {
followLink('bundles'),
followLink('relationships'),
followLink('version', {}, followLink('versionhistory')),
).pipe(getFirstCompletedRemoteData());
).pipe(
getFirstCompletedRemoteData()
);
}

}
79 changes: 40 additions & 39 deletions src/app/item-page/cris-item-page-tab.resolver.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { ItemDataService } from './../core/data/item-data.service';
import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot, Router, ActivatedRoute } from '@angular/router';
import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router';

import { Observable } from 'rxjs';
import { map, switchMap } from 'rxjs/operators';

import { ItemDataService } from '../core/data/item-data.service';
import { RemoteData } from '../core/data/remote-data';
import { CrisLayoutTab } from '../core/layout/models/tab.model';
import { TabDataService } from '../core/layout/tab-data.service';
import { Observable } from 'rxjs';
import { PaginatedList } from '../core/data/paginated-list.model';
import { getFirstCompletedRemoteData, getRemoteDataPayload, getPaginatedListPayload } from '../core/shared/operators';
import { switchMap, map } from 'rxjs/operators';
import { getFirstCompletedRemoteData } from '../core/shared/operators';
import { Item } from '../core/shared/item.model';
import { getItemPageRoute } from './item-page-routing-paths';
import { hasValue, isNotEmpty } from '../shared/empty.util';
import { createFailedRemoteDataObject$ } from '../shared/remote-data.utils';

/**
* This class represents a resolver that requests the tabs of specific
Expand All @@ -19,7 +21,7 @@ import { hasValue, isNotEmpty } from '../shared/empty.util';
@Injectable()
export class CrisItemPageTabResolver implements Resolve<RemoteData<PaginatedList<CrisLayoutTab>>> {

constructor(private tabService: TabDataService, private itemDataService: ItemDataService, private router: Router, private aroute: ActivatedRoute) { }
constructor(private tabService: TabDataService, private itemDataService: ItemDataService, private router: Router) { }

/**
* Method for resolving the tabs of item based on the parameters in the current route
Expand All @@ -31,41 +33,40 @@ export class CrisItemPageTabResolver implements Resolve<RemoteData<PaginatedList
resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<RemoteData<PaginatedList<CrisLayoutTab>>> {
return this.itemDataService.findById(route.params.id).pipe(
getFirstCompletedRemoteData(),
getRemoteDataPayload(),
map((item: Item) => {
if (!item || (!!item && !item.uuid)) {
this.router.navigate(['/404']);
} else if (hasValue(item.metadata) && isNotEmpty(item.metadata['cris.customurl'])) {
if (route.params.id !== item.metadata['cris.customurl'][0].value) {
this.router.navigateByUrl(getItemPageRoute(item));
}
}
return item;
}),
switchMap((item: Item) => this.tabService.findByItem(
item.uuid, // Item UUID
true
).pipe(
getFirstCompletedRemoteData(),
map((tabs: RemoteData<PaginatedList<CrisLayoutTab>>) => {
// By splititng the url with uuid we can understand if the item is primary item page or a tab
const urlSplited = state.url.split(route.params.id);
if (!!tabs.payload && !!tabs.payload.page && tabs.payload.page.length > 0 && !urlSplited[1]) {
const selectedTab = tabs.payload.page.filter((tab) => !tab.leading)[0];
if (!!selectedTab) {
let tabName = selectedTab.shortname;
switchMap((itemRD: RemoteData<Item>) => {
if (itemRD.hasSucceeded && itemRD.statusCode === 200) {
return this.tabService.findByItem(
itemRD.payload.uuid,
true
).pipe(
getFirstCompletedRemoteData(),
map((tabsRD: RemoteData<PaginatedList<CrisLayoutTab>>) => {
if (tabsRD.hasSucceeded && tabsRD?.payload?.page?.length > 0) {
// By splitting the url with uuid we can understand if the item is primary item page or a tab
const urlSplit = state.url.split(route.params.id);
// If a no or wrong tab is given redirect to the first tab available
if (!urlSplit[1] || !tabsRD.payload.page.some((tab: CrisLayoutTab) => `/${tab.shortname}` === urlSplit[1])) {
const selectedTab = tabsRD.payload.page.filter((tab) => !tab.leading)[0];
if (!!selectedTab) {
let tabName = selectedTab.shortname;

if (tabName.includes('::')) {
tabName = tabName.split('::')[1];
}

if (tabName.includes('::')) {
tabName = tabName.split('::')[1];
this.router.navigateByUrl(getItemPageRoute(itemRD.payload) + '/' + tabName);
}
}
}
return tabsRD;
})
);

this.router.navigateByUrl(getItemPageRoute(item) + '/' + tabName);
}
}
return tabs;
})
)
));
} else {
return createFailedRemoteDataObject$<PaginatedList<CrisLayoutTab>>(itemRD?.errorMessage, itemRD?.statusCode, itemRD?.timeCompleted);
}
})
);
}

}
6 changes: 3 additions & 3 deletions src/app/item-page/simple/item-page.component.html
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<div *ngVar="(tabsRD$ | async) as tabsRD">
<div
[ngClass]="{'container': itemRD?.hasSucceeded && tabsRD?.payload?.pageInfo?.totalElements < 1}"
[ngClass]="{'container': (itemRD?.hasSucceeded && !itemRD?.hasNoContent) && tabsRD?.payload?.pageInfo?.totalElements < 1}"
*ngVar="(itemRD$ | async) as itemRD">
<div class="item-page" *ngIf="itemRD?.hasSucceeded && tabsRD?.payload?.pageInfo?.totalElements < 1" @fadeInOut>
<div class="item-page" *ngIf="(itemRD?.hasSucceeded && !itemRD?.hasNoContent) && tabsRD?.payload?.pageInfo?.totalElements < 1" @fadeInOut>
<div *ngIf="itemRD?.payload as item">
<ds-item-alerts [item]="item"></ds-item-alerts>
<ds-item-versions-notice [item]="item"></ds-item-versions-notice>
Expand All @@ -11,7 +11,7 @@
<ds-item-versions class="mt-2" [item]="item" [displayActions]="false"></ds-item-versions>
</div>
</div>
<ds-cris-item-page *ngIf="itemRD?.hasSucceeded && tabsRD?.payload?.pageInfo?.totalElements > 0"></ds-cris-item-page>
<ds-cris-item-page *ngIf="(itemRD?.hasSucceeded && !itemRD?.hasNoContent) && tabsRD?.payload?.pageInfo?.totalElements > 0"></ds-cris-item-page>
<ds-error *ngIf="itemRD?.hasFailed" message="{{'error.item' | translate}}"></ds-error>
<ds-themed-loading *ngIf="itemRD?.isLoading" message="{{'loading.item' | translate}}"></ds-themed-loading>
</div>
Expand Down
5 changes: 3 additions & 2 deletions src/app/item-page/simple/item-page.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { getAllSucceededRemoteDataPayload } from '../../core/shared/operators';
import { ViewMode } from '../../core/shared/view-mode.model';
import { AuthService } from '../../core/auth/auth.service';
import { getItemPageRoute } from '../item-page-routing-paths';
import { redirectOn4xx } from '../../core/shared/authorized.operators';
import { redirectOn204, redirectOn4xx } from '../../core/shared/authorized.operators';
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
import { FeatureID } from '../../core/data/feature-authorization/feature-id';
import { CrisLayoutTab } from '../../core/layout/models/tab.model';
Expand Down Expand Up @@ -79,7 +79,8 @@ export class ItemPageComponent implements OnInit {
ngOnInit(): void {
this.itemRD$ = this.route.data.pipe(
map((data) => data.dso as RemoteData<Item>),
redirectOn4xx(this.router, this.authService)
redirectOn204<Item>(this.router, this.authService),
redirectOn4xx<Item>(this.router, this.authService)
);
this.tabsRD$ = this.route.data.pipe(
map((data) => data.tabs as RemoteData<PaginatedList<CrisLayoutTab>>),
Expand Down

0 comments on commit effcf07

Please sign in to comment.