74612: Remove 401 pages and add login redirect to feature-authorization guards

src/app/+collection-page/collection-page-administrator.guard.ts

@@ -7,6 +7,7 @@ import { of as observableOf } from 'rxjs';
 import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard';
 import { Observable } from 'rxjs/internal/Observable';
 import { FeatureID } from '../core/data/feature-authorization/feature-id';
+import { AuthService } from '../core/auth/auth.service';
 
 @Injectable({
   providedIn: 'root'
@@ -17,8 +18,9 @@ import { FeatureID } from '../core/data/feature-authorization/feature-id';
 export class CollectionPageAdministratorGuard extends DsoPageFeatureGuard<Collection> {
   constructor(protected resolver: CollectionPageResolver,
               protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
-    super(resolver, authorizationService, router);
+              protected authService: AuthService) {
+    super(resolver, authorizationService, router, authService);
   }
 
   /**

src/app/+community-page/community-page-administrator.guard.ts

@@ -7,6 +7,7 @@ import { of as observableOf } from 'rxjs';
 import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard';
 import { Observable } from 'rxjs/internal/Observable';
 import { FeatureID } from '../core/data/feature-authorization/feature-id';
+import { AuthService } from '../core/auth/auth.service';
 
 @Injectable({
   providedIn: 'root'
@@ -17,8 +18,9 @@ import { FeatureID } from '../core/data/feature-authorization/feature-id';
 export class CommunityPageAdministratorGuard extends DsoPageFeatureGuard<Community> {
   constructor(protected resolver: CommunityPageResolver,
               protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
-    super(resolver, authorizationService, router);
+              protected authService: AuthService) {
+    super(resolver, authorizationService, router, authService);
   }
 
   /**

src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts

@@ -7,6 +7,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro
 import { Observable } from 'rxjs/internal/Observable';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { of as observableOf } from 'rxjs';
+import { AuthService } from '../../core/auth/auth.service';
 
 @Injectable({
   providedIn: 'root'
@@ -17,8 +18,9 @@ import { of as observableOf } from 'rxjs';
 export class ItemPageReinstateGuard extends DsoPageFeatureGuard<Item> {
   constructor(protected resolver: ItemPageResolver,
               protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
-    super(resolver, authorizationService, router);
+              protected authService: AuthService) {
+    super(resolver, authorizationService, router, authService);
   }
 
   /**

src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts

@@ -7,6 +7,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro
 import { Observable } from 'rxjs/internal/Observable';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { of as observableOf } from 'rxjs';
+import { AuthService } from '../../core/auth/auth.service';
 
 @Injectable({
   providedIn: 'root'
@@ -17,8 +18,9 @@ import { of as observableOf } from 'rxjs';
 export class ItemPageWithdrawGuard extends DsoPageFeatureGuard<Item> {
   constructor(protected resolver: ItemPageResolver,
               protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
-    super(resolver, authorizationService, router);
+              protected authService: AuthService) {
+    super(resolver, authorizationService, router, authService);
   }
 
   /**

src/app/+item-page/item-page-administrator.guard.ts

@@ -7,6 +7,7 @@ import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-
 import { Observable } from 'rxjs/internal/Observable';
 import { FeatureID } from '../core/data/feature-authorization/feature-id';
 import { of as observableOf } from 'rxjs';
+import { AuthService } from '../core/auth/auth.service';
 
 @Injectable({
   providedIn: 'root'
@@ -17,8 +18,9 @@ import { of as observableOf } from 'rxjs';
 export class ItemPageAdministratorGuard extends DsoPageFeatureGuard<Item> {
   constructor(protected resolver: ItemPageResolver,
               protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
-    super(resolver, authorizationService, router);
+              protected authService: AuthService) {
+    super(resolver, authorizationService, router, authService);
   }
 
   /**

src/app/app-routing-paths.ts

@@ -55,12 +55,6 @@ export function getDSORoute(dso: DSpaceObject): string {
   }
 }
 
-export const UNAUTHORIZED_PATH = '401';
-
-export function getUnauthorizedRoute() {
-  return `/${UNAUTHORIZED_PATH}`;
-}
-
 export const FORBIDDEN_PATH = '403';
 
 export function getForbiddenRoute() {

src/app/app-routing.module.ts

@@ -5,16 +5,15 @@ import { AuthBlockingGuard } from './core/auth/auth-blocking.guard';
 import { PageNotFoundComponent } from './pagenotfound/pagenotfound.component';
 import { AuthenticatedGuard } from './core/auth/authenticated.guard';
 import { SiteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
-import { UnauthorizedComponent } from './unauthorized/unauthorized.component';
 import {
-  UNAUTHORIZED_PATH,
   WORKFLOW_ITEM_MODULE_PATH,
   FORGOT_PASSWORD_PATH,
   REGISTER_PATH,
   PROFILE_MODULE_PATH,
   ADMIN_MODULE_PATH,
   BITSTREAM_MODULE_PATH,
-  INFO_MODULE_PATH, FORBIDDEN_PATH
+  INFO_MODULE_PATH,
+  FORBIDDEN_PATH,
 } from './app-routing-paths';
 import { COLLECTION_MODULE_PATH } from './+collection-page/collection-page-routing-paths';
 import { COMMUNITY_MODULE_PATH } from './+community-page/community-page-routing-paths';
@@ -69,7 +68,6 @@ import { ForbiddenComponent } from './forbidden/forbidden.component';
             },
             { path: 'processes', loadChildren: './process-page/process-page.module#ProcessPageModule', canActivate: [AuthenticatedGuard, EndUserAgreementCurrentUserGuard] },
             { path: INFO_MODULE_PATH, loadChildren: './info/info.module#InfoModule' },
-            { path: UNAUTHORIZED_PATH, component: UnauthorizedComponent },
             { path: FORBIDDEN_PATH, component: ForbiddenComponent },
             {
               path: 'statistics',

src/app/app.module.ts

@@ -41,7 +41,6 @@ import { SharedModule } from './shared/shared.module';
 import { BreadcrumbsComponent } from './breadcrumbs/breadcrumbs.component';
 import { environment } from '../environments/environment';
 import { BrowserModule } from '@angular/platform-browser';
-import { UnauthorizedComponent } from './unauthorized/unauthorized.component';
 import { ForbiddenComponent } from './forbidden/forbidden.component';
 
 export function getBase() {
@@ -118,7 +117,6 @@ const DECLARATIONS = [
   NotificationsBoardComponent,
   SearchNavbarComponent,
   BreadcrumbsComponent,
-  UnauthorizedComponent,
   ForbiddenComponent,
 ];
 

src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts

@@ -7,6 +7,7 @@ import { DSpaceObject } from '../../../shared/dspace-object.model';
 import { DsoPageFeatureGuard } from './dso-page-feature.guard';
 import { FeatureID } from '../feature-id';
 import { Observable } from 'rxjs/internal/Observable';
+import { AuthService } from '../../../auth/auth.service';
 
 /**
  * Test implementation of abstract class DsoPageAdministratorGuard
@@ -15,8 +16,9 @@ class DsoPageFeatureGuardImpl extends DsoPageFeatureGuard<any> {
   constructor(protected resolver: Resolve<RemoteData<any>>,
               protected authorizationService: AuthorizationDataService,
               protected router: Router,
+              protected authService: AuthService,
               protected featureID: FeatureID) {
-    super(resolver, authorizationService, router);
+    super(resolver, authorizationService, router, authService);
   }
 
   getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<FeatureID> {
@@ -28,6 +30,7 @@ describe('DsoPageAdministratorGuard', () => {
   let guard: DsoPageFeatureGuard<any>;
   let authorizationService: AuthorizationDataService;
   let router: Router;
+  let authService: AuthService;
   let resolver: Resolve<RemoteData<any>>;
   let object: DSpaceObject;
 
@@ -45,7 +48,10 @@ describe('DsoPageAdministratorGuard', () => {
     resolver = jasmine.createSpyObj('resolver', {
       resolve: createSuccessfulRemoteDataObject$(object)
     });
-    guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, undefined);
+    authService = jasmine.createSpyObj('authService', {
+      isAuthenticated: observableOf(true)
+    });
+    guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, authService, undefined);
   }
 
   beforeEach(() => {

src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts

@@ -6,6 +6,7 @@ import { getAllSucceededRemoteDataPayload } from '../../../shared/operators';
 import { map } from 'rxjs/operators';
 import { DSpaceObject } from '../../../shared/dspace-object.model';
 import { FeatureAuthorizationGuard } from './feature-authorization.guard';
+import { AuthService } from '../../../auth/auth.service';
 
 /**
  * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature
@@ -14,8 +15,9 @@ import { FeatureAuthorizationGuard } from './feature-authorization.guard';
 export abstract class DsoPageFeatureGuard<T extends DSpaceObject> extends FeatureAuthorizationGuard {
   constructor(protected resolver: Resolve<RemoteData<T>>,
               protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
-    super(authorizationService, router);
+              protected authService: AuthService) {
+    super(authorizationService, router, authService);
   }
 
   /**

src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts

@@ -4,6 +4,7 @@ import { FeatureID } from '../feature-id';
 import { of as observableOf } from 'rxjs';
 import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
 import { Observable } from 'rxjs/internal/Observable';
+import { AuthService } from '../../../auth/auth.service';
 
 /**
  * Test implementation of abstract class FeatureAuthorizationGuard
@@ -12,10 +13,11 @@ import { Observable } from 'rxjs/internal/Observable';
 class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard {
   constructor(protected authorizationService: AuthorizationDataService,
               protected router: Router,
+              protected authService: AuthService,
               protected featureId: FeatureID,
               protected objectUrl: string,
               protected ePersonUuid: string) {
-    super(authorizationService, router);
+    super(authorizationService, router, authService);
   }
 
   getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<FeatureID> {
@@ -35,6 +37,7 @@ describe('FeatureAuthorizationGuard', () => {
   let guard: FeatureAuthorizationGuard;
   let authorizationService: AuthorizationDataService;
   let router: Router;
+  let authService: AuthService;
 
   let featureId: FeatureID;
   let objectUrl: string;
@@ -51,7 +54,10 @@ describe('FeatureAuthorizationGuard', () => {
     router = jasmine.createSpyObj('router', {
       parseUrl: {}
     });
-    guard = new FeatureAuthorizationGuardImpl(authorizationService, router, featureId, objectUrl, ePersonUuid);
+    authService = jasmine.createSpyObj('authService', {
+      isAuthenticated: observableOf(true)
+    });
+    guard = new FeatureAuthorizationGuardImpl(authorizationService, router, authService, featureId, objectUrl, ePersonUuid);
   }
 
   beforeEach(() => {
@@ -60,7 +66,7 @@ describe('FeatureAuthorizationGuard', () => {
 
   describe('canActivate', () => {
     it('should call authorizationService.isAuthenticated with the appropriate arguments', () => {
-      guard.canActivate(undefined, undefined).subscribe();
+      guard.canActivate(undefined, { url: 'current-url' } as any).subscribe();
       expect(authorizationService.isAuthorized).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid);
     });
   });

src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts

@@ -8,9 +8,10 @@ import {
 import { AuthorizationDataService } from '../authorization-data.service';
 import { FeatureID } from '../feature-id';
 import { Observable } from 'rxjs/internal/Observable';
-import { returnUnauthorizedUrlTreeOnFalse } from '../../../shared/operators';
+import { returnForbiddenUrlTreeOrLoginOnFalse } from '../../../shared/operators';
 import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs';
 import { switchMap } from 'rxjs/operators';
+import { AuthService } from '../../../auth/auth.service';
 
 /**
  * Abstract Guard for preventing unauthorized activating and loading of routes when a user
@@ -19,7 +20,8 @@ import { switchMap } from 'rxjs/operators';
  */
 export abstract class FeatureAuthorizationGuard implements CanActivate {
   constructor(protected authorizationService: AuthorizationDataService,
-              protected router: Router) {
+              protected router: Router,
+              protected authService: AuthService) {
   }
 
   /**
@@ -29,7 +31,7 @@ export abstract class FeatureAuthorizationGuard implements CanActivate {
   canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean | UrlTree> {
     return observableCombineLatest(this.getFeatureID(route, state), this.getObjectUrl(route, state), this.getEPersonUuid(route, state)).pipe(
       switchMap(([featureID, objectUrl, ePersonUuid]) => this.authorizationService.isAuthorized(featureID, objectUrl, ePersonUuid)),
-      returnUnauthorizedUrlTreeOnFalse(this.router)
+      returnForbiddenUrlTreeOrLoginOnFalse(this.router, this.authService, state.url)
     );
   }
 

src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts

@@ -5,6 +5,7 @@ import { AuthorizationDataService } from '../authorization-data.service';
 import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
 import { of as observableOf } from 'rxjs';
 import { Observable } from 'rxjs/internal/Observable';
+import { AuthService } from '../../../auth/auth.service';
 
 /**
  * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator
@@ -14,8 +15,8 @@ import { Observable } from 'rxjs/internal/Observable';
   providedIn: 'root'
 })
 export class SiteAdministratorGuard extends FeatureAuthorizationGuard {
-  constructor(protected authorizationService: AuthorizationDataService, protected router: Router) {
+  constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) {
-    super(authorizationService, router);
+    super(authorizationService, router, authService);
   }
 
   /**

src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts

@@ -5,6 +5,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro
 import { Observable } from 'rxjs/internal/Observable';
 import { FeatureID } from '../feature-id';
 import { of as observableOf } from 'rxjs';
+import { AuthService } from '../../../auth/auth.service';
 
 /**
  * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have registration
@@ -14,8 +15,8 @@ import { of as observableOf } from 'rxjs';
   providedIn: 'root'
 })
 export class SiteRegisterGuard extends FeatureAuthorizationGuard {
-  constructor(protected authorizationService: AuthorizationDataService, protected router: Router) {
+  constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) {
-    super(authorizationService, router);
+    super(authorizationService, router, authService);
   }
 
   /**

src/app/core/shared/operators.spec.ts

@@ -226,13 +226,6 @@ describe('Core Module - RxJS Operators', () => {
       expect(router.navigateByUrl).toHaveBeenCalledWith('/403', { skipLocationChange: true });
     });
 
-    it('should call navigateByUrl to a 401 page, when the remote data contains a 401 error', () => {
-      const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(401, 'Unauthorized', 'The current user is unauthorized'));
-
-      observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe();
-      expect(router.navigateByUrl).toHaveBeenCalledWith('/401', { skipLocationChange: true });
-    });
-
     it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains another error than a 404, 403 or 401', () => {
       const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(500, 'Server Error', 'Something went wrong'));
 

src/app/core/shared/operators.ts

@@ -13,7 +13,7 @@ import { MetadataField } from '../metadata/metadata-field.model';
 import { MetadataSchema } from '../metadata/metadata-schema.model';
 import { BrowseDefinition } from './browse-definition.model';
 import { DSpaceObject } from './dspace-object.model';
-import { getForbiddenRoute, getPageNotFoundRoute, getUnauthorizedRoute } from '../../app-routing-paths';
+import { getForbiddenRoute, getPageNotFoundRoute } from '../../app-routing-paths';
 import { getEndUserAgreementPath } from '../../info/info-routing-paths';
 import { AuthService } from '../auth/auth.service';
 
@@ -190,11 +190,7 @@ export const redirectOn4xx = (router: Router, authService: AuthService) =>
             router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true});
           } else if (rd.error.statusCode === 403 || rd.error.statusCode === 401) {
             if (isAuthenticated) {
-              if (rd.error.statusCode === 403) {
+              router.navigateByUrl(getForbiddenRoute(), {skipLocationChange: true});
-                router.navigateByUrl(getForbiddenRoute(), {skipLocationChange: true});
-              } else if (rd.error.statusCode === 401) {
-                router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true});
-              }
             } else {
               authService.setRedirectUrl(router.url);
               router.navigateByUrl('login');
@@ -205,14 +201,25 @@ export const redirectOn4xx = (router: Router, authService: AuthService) =>
       }));
 
 /**
- * Operator that returns a UrlTree to the unauthorized page when the boolean received is false
+ * Operator that returns a UrlTree to a forbidden page or the login page when the boolean received is false
- * @param router
+ * @param router      The router used to navigate to a forbidden page
+ * @param authService The AuthService used to determine whether or not the user is logged in
+ * @param redirectUrl The URL to redirect back to after logging in
  */
-export const returnUnauthorizedUrlTreeOnFalse = (router: Router) =>
+export const returnForbiddenUrlTreeOrLoginOnFalse = (router: Router, authService: AuthService, redirectUrl: string) =>
   (source: Observable<boolean>): Observable<boolean | UrlTree> =>
-    source.pipe(
+    observableCombineLatest(source, authService.isAuthenticated()).pipe(
-      map((authorized: boolean) => {
+      map(([authorized, authenticated]: [boolean, boolean]) => {
-        return authorized ? authorized : router.parseUrl(getUnauthorizedRoute())
+        if (authorized) {
+          return authorized;
+        } else {
+          if (authenticated) {
+            return router.parseUrl(getForbiddenRoute());
+          } else {
+            authService.setRedirectUrl(redirectUrl);
+            return router.parseUrl('login');
+          }
+        }
       }));
 
 /**

src/app/unauthorized/unauthorized.component.html

@@ -1,10 +0,0 @@
-<div class="unauthorized container">
-  <h1>401</h1>
-  <h2><small>{{"401.unauthorized" | translate}}</small></h2>
-  <br/>
-  <p>{{"401.help" | translate}}</p>
-  <br/>
-  <p class="text-center">
-    <a routerLink="/home" class="btn btn-primary">{{"401.link.home-page" | translate}}</a>
-  </p>
-</div>

src/app/unauthorized/unauthorized.component.ts

@@ -1,32 +0,0 @@
-import { Component, OnInit } from '@angular/core';
-import { AuthService } from '../core/auth/auth.service';
-import { ServerResponseService } from '../core/services/server-response.service';
-
-/**
- * This component representing the `Unauthorized` DSpace page.
- */
-@Component({
-  selector: 'ds-unauthorized',
-  templateUrl: './unauthorized.component.html',
-  styleUrls: ['./unauthorized.component.scss']
-})
-export class UnauthorizedComponent implements OnInit {
-
-  /**
-   * Initialize instance variables
-   *
-   * @param {AuthService} authservice
-   * @param {ServerResponseService} responseService
-   */
-  constructor(private authservice: AuthService, private responseService: ServerResponseService) {
-    this.responseService.setUnauthorized();
-  }
-
-  /**
-   * Remove redirect url from the state
-   */
-  ngOnInit(): void {
-    this.authservice.clearRedirectUrl();
-  }
-
-}