72699: Hard redirect after log in

src/app/app-routing.module.ts

@@ -12,6 +12,7 @@ import { getItemPageRoute } from './+item-page/item-page-routing.module';
 import { getCollectionPageRoute } from './+collection-page/collection-page-routing.module';
 import { SiteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
 import { UnauthorizedComponent } from './unauthorized/unauthorized.component';
+import { ReloadGuard } from './core/reload/reload.guard';
 
 const ITEM_MODULE_PATH = 'items';
 
@@ -88,7 +89,7 @@ export function getUnauthorizedPath() {
   imports: [
     RouterModule.forRoot([
       { path: '', redirectTo: '/home', pathMatch: 'full' },
-      { path: 'reload/:rnd', redirectTo: '/home', pathMatch: 'full' },
+      { path: 'reload/:rnd', component: PageNotFoundComponent, pathMatch: 'full', canActivate: [ReloadGuard] },
       { path: 'home', loadChildren: './+home-page/home-page.module#HomePageModule', data: { showBreadcrumbs: false } },
       { path: 'community-list', loadChildren: './community-list-page/community-list-page.module#CommunityListPageModule' },
       { path: 'id', loadChildren: './+lookup-by-id/lookup-by-id.module#LookupIdModule' },

src/app/core/auth/auth.effects.ts

@@ -201,13 +201,6 @@ export class AuthEffects {
       tap(() => this.authService.refreshAfterLogout())
     );
 
-  @Effect({ dispatch: false })
-  public redirectToLogin$: Observable<Action> = this.actions$
-    .pipe(ofType(AuthActionTypes.REDIRECT_AUTHENTICATION_REQUIRED),
-      tap(() => this.authService.removeToken()),
-      tap(() => this.authService.redirectToLogin())
-    );
-
   @Effect({ dispatch: false })
   public redirectToLoginTokenExpired$: Observable<Action> = this.actions$
     .pipe(

src/app/core/auth/auth.service.ts

@@ -36,6 +36,7 @@ import { RouteService } from '../services/route.service';
 import { EPersonDataService } from '../eperson/eperson-data.service';
 import { getAllSucceededRemoteDataPayload } from '../shared/operators';
 import { AuthMethod } from './models/auth.method';
+import { HardRedirectService } from '../services/hard-redirect.service';
 
 export const LOGIN_ROUTE = '/login';
 export const LOGOUT_ROUTE = '/logout';
@@ -62,7 +63,8 @@ export class AuthService {
               protected router: Router,
               protected routeService: RouteService,
               protected storage: CookieService,
-              protected store: Store<AppState>
+              protected store: Store<AppState>,
+              protected hardRedirectService: HardRedirectService
   ) {
     this.store.pipe(
       select(isAuthenticated),
@@ -440,26 +442,18 @@ export class AuthService {
   }
 
   protected navigateToRedirectUrl(redirectUrl: string) {
-    const url = decodeURIComponent(redirectUrl);
+    let url = `/reload/${new Date().getTime()}`;
-    // in case the user navigates directly to /login (via bookmark, etc), or the route history is not found.
+    if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) {
-    if (isEmpty(url) || url.startsWith(LOGIN_ROUTE)) {
+      url += `?redirect=${encodeURIComponent(redirectUrl)}`;
-      this.router.navigateByUrl('/');
-      /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */
-      // this._window.nativeWindow.location.href = '/';
-    } else {
-      /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */
-      // this._window.nativeWindow.location.href = url;
-      this.router.navigateByUrl(url);
     }
+    this.hardRedirectService.redirect(url);
   }
 
   /**
    * Refresh route navigated
    */
   public refreshAfterLogout() {
-    // Hard redirect to the reload page with a unique number behind it
+    this.navigateToRedirectUrl(undefined);
-    // so that all state is definitely lost
-    this._window.nativeWindow.location.href = `/reload/${new Date().getTime()}`;
   }
 
   /**

src/app/core/auth/authenticated.guard.ts

@@ -1,21 +1,27 @@
 import { Injectable } from '@angular/core';
-import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, Router, RouterStateSnapshot } from '@angular/router';
+import {
+  ActivatedRouteSnapshot,
+  CanActivate,
+  Route,
+  Router,
+  RouterStateSnapshot,
+  UrlTree
+} from '@angular/router';
 
 import { Observable } from 'rxjs';
-import { take } from 'rxjs/operators';
+import { map } from 'rxjs/operators';
 import { select, Store } from '@ngrx/store';
 
 import { CoreState } from '../core.reducers';
 import { isAuthenticated } from './selectors';
-import { AuthService } from './auth.service';
+import { AuthService, LOGIN_ROUTE } from './auth.service';
-import { RedirectWhenAuthenticationIsRequiredAction } from './auth.actions';
 
 /**
  * Prevent unauthorized activating and loading of routes
  * @class AuthenticatedGuard
  */
 @Injectable()
-export class AuthenticatedGuard implements CanActivate, CanLoad {
+export class AuthenticatedGuard implements CanActivate {
 
   /**
    * @constructor
@@ -24,46 +30,38 @@ export class AuthenticatedGuard implements CanActivate, CanLoad {
 
   /**
    * True when user is authenticated
+   * UrlTree with redirect to login page when user isn't authenticated
    * @method canActivate
    */
-  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> {
+  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean | UrlTree> {
     const url = state.url;
     return this.handleAuth(url);
   }
 
   /**
    * True when user is authenticated
+   * UrlTree with redirect to login page when user isn't authenticated
    * @method canActivateChild
    */
-  canActivateChild(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> {
+  canActivateChild(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean | UrlTree> {
     return this.canActivate(route, state);
   }
 
-  /**
+  private handleAuth(url: string): Observable<boolean | UrlTree> {
-   * True when user is authenticated
-   * @method canLoad
-   */
-  canLoad(route: Route): Observable<boolean> {
-    const url = `/${route.path}`;
-
-    return this.handleAuth(url);
-  }
-
-  private handleAuth(url: string): Observable<boolean> {
     // get observable
     const observable = this.store.pipe(select(isAuthenticated));
 
     // redirect to sign in page if user is not authenticated
-    observable.pipe(
+    return observable.pipe(
-      // .filter(() => isEmpty(this.router.routerState.snapshot.url) || this.router.routerState.snapshot.url === url)
+      map((authenticated) => {
-      take(1))
+        if (authenticated) {
-      .subscribe((authenticated) => {
+          return authenticated;
-        if (!authenticated) {
+        } else {
           this.authService.setRedirectUrl(url);
-          this.store.dispatch(new RedirectWhenAuthenticationIsRequiredAction('Login required'));
+          this.authService.removeToken();
+          return this.router.createUrlTree([LOGIN_ROUTE]);
         }
-    });
+      })
-
+    );
-    return observable;
   }
 }

src/app/core/core.module.ts

@@ -162,6 +162,7 @@ import { SubmissionCcLicenceUrl } from './submission/models/submission-cc-licens
 import { SubmissionCcLicenseUrlDataService } from './submission/submission-cc-license-url-data.service';
 import { ConfigurationDataService } from './data/configuration-data.service';
 import { ConfigurationProperty } from './shared/configuration-property.model';
+import { ReloadGuard } from './reload/reload.guard';
 
 /**
  * When not in production, endpoint responses can be mocked for testing purposes
@@ -289,6 +290,7 @@ const PROVIDERS = [
   MetadataSchemaDataService,
   MetadataFieldDataService,
   TokenResponseParsingService,
+  ReloadGuard,
   // register AuthInterceptor as HttpInterceptor
   {
     provide: HTTP_INTERCEPTORS,

src/app/core/reload/reload.guard.ts

@@ -0,0 +1,17 @@
+import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTree } from '@angular/router';
+import { Injectable } from '@angular/core';
+import { isNotEmpty } from '../../shared/empty.util';
+
+@Injectable()
+export class ReloadGuard implements CanActivate {
+  constructor(private router: Router) {
+  }
+
+  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): UrlTree {
+    if (isNotEmpty(route.queryParams.redirect)) {
+      return this.router.parseUrl(route.queryParams.redirect);
+    } else {
+      return this.router.createUrlTree(['home']);
+    }
+  }
+}

src/app/core/services/browser-hard-redirect.service.ts

@@ -0,0 +1,19 @@
+import {Inject, Injectable} from '@angular/core';
+import {LocationToken} from '../../../modules/app/browser-app.module';
+
+@Injectable()
+export class BrowserHardRedirectService {
+
+  constructor(
+    @Inject(LocationToken) protected location: Location,
+  ) {
+  }
+
+  redirect(url: string) {
+    this.location.href = url;
+  }
+
+  getOriginFromUrl() {
+    return this.location.origin;
+  }
+}

src/app/core/services/hard-redirect.service.ts

@@ -0,0 +1,21 @@
+import { Injectable } from '@angular/core';
+
+/**
+ * Service to take care of hard redirects
+ */
+@Injectable()
+export abstract class HardRedirectService {
+
+  /**
+   * Perform a hard redirect to a given location.
+   *
+   * @param url
+   *    the page to redirect to
+   */
+  abstract redirect(url: string);
+
+  /**
+   * Get the origin of a request
+   */
+  abstract getOriginFromUrl();
+}

src/app/core/services/server-hard-redirect.service.ts

@@ -0,0 +1,52 @@
+import { Inject, Injectable } from '@angular/core';
+import { Request, Response } from 'express';
+import { REQUEST, RESPONSE } from '@nguniversal/express-engine/tokens';
+
+@Injectable()
+export class ServerHardRedirectService {
+
+  constructor(
+    @Inject(REQUEST) protected req: Request,
+    @Inject(RESPONSE) protected res: Response,
+  ) {
+  }
+
+  redirect(url: string) {
+
+    if (url === this.req.url) {
+      return;
+    }
+
+    if (this.res.finished) {
+      const req: any = this.req;
+      req._r_count = (req._r_count || 0) + 1;
+
+      console.warn('Attempted to redirect on a finished response. From',
+        this.req.url, 'to', url);
+
+      if (req._r_count > 10) {
+        console.error('Detected a redirection loop. killing the nodejs process');
+        process.exit(1);
+      }
+    } else {
+      // attempt to use the already set status
+      let status = this.res.statusCode || 0;
+      if (status < 300 || status >= 400) {
+        // temporary redirect
+        status = 302;
+      }
+
+      console.log(`Redirecting from ${this.req.url} to ${url} with ${status}`);
+      this.res.redirect(status, url);
+      this.res.end();
+      // I haven't found a way to correctly stop Angular rendering.
+      // So we just let it end its work, though we have already closed
+      // the response.
+    }
+  }
+
+  getOriginFromUrl() {
+
+    return new URL(`${this.req.protocol}://${this.req.get('hostname')}`).toString();
+  }
+}

src/modules/app/browser-app.module.ts

@@ -1,5 +1,5 @@
 import { HttpClient, HttpClientModule } from '@angular/common/http';
-import { NgModule } from '@angular/core';
+import { InjectionToken, NgModule } from '@angular/core';
 import { BrowserModule, makeStateKey, TransferState } from '@angular/platform-browser';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import { RouterModule } from '@angular/router';
@@ -21,6 +21,8 @@ import { AuthService } from '../../app/core/auth/auth.service';
 import { Angulartics2RouterlessModule } from 'angulartics2/routerlessmodule';
 import { SubmissionService } from '../../app/submission/submission.service';
 import { StatisticsModule } from '../../app/statistics/statistics.module';
+import { HardRedirectService } from '../../app/core/services/hard-redirect.service';
+import { BrowserHardRedirectService } from '../../app/core/services/browser-hard-redirect.service';
 
 export const REQ_KEY = makeStateKey<string>('req');
 
@@ -32,6 +34,13 @@ export function getRequest(transferState: TransferState): any {
   return transferState.get<any>(REQ_KEY, {});
 }
 
+export const LocationToken = new InjectionToken('Location');
+
+export function locationProvider(): Location {
+  return window.location;
+}
+
+
 @NgModule({
   bootstrap: [AppComponent],
   imports: [
@@ -78,7 +87,15 @@ export function getRequest(transferState: TransferState): any {
     {
       provide: SubmissionService,
       useClass: SubmissionService
-    }
+    },
+    {
+      provide: HardRedirectService,
+      useClass: BrowserHardRedirectService,
+    },
+    {
+      provide: LocationToken,
+      useFactory: locationProvider,
+    },
   ]
 })
 export class BrowserAppModule {

src/modules/app/server-app.module.ts

@@ -29,6 +29,8 @@ import { ServerLocaleService } from 'src/app/core/locale/server-locale.service';
 import { LocaleService } from 'src/app/core/locale/locale.service';
 import { HTTP_INTERCEPTORS } from '@angular/common/http';
 import { ForwardClientIpInterceptor } from '../../app/core/forward-client-ip/forward-client-ip.interceptor';
+import { HardRedirectService } from '../../app/core/services/hard-redirect.service';
+import { ServerHardRedirectService } from '../../app/core/services/server-hard-redirect.service';
 
 export function createTranslateLoader() {
   return new TranslateJson5UniversalLoader('dist/server/assets/i18n/', '.json5');
@@ -88,6 +90,10 @@ export function createTranslateLoader() {
       useClass: ForwardClientIpInterceptor,
       multi: true
     },
+    {
+      provide: HardRedirectService,
+      useClass: ServerHardRedirectService,
+    },
   ]
 })
 export class ServerAppModule {