diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index c8ee6ecd8b..a317cf9334 100644 --- a/src/app/app-routing.module.ts +++ b/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' }, diff --git a/src/app/core/auth/auth.effects.ts b/src/app/core/auth/auth.effects.ts index 37ef3b79bc..7d1750c04e 100644 --- a/src/app/core/auth/auth.effects.ts +++ b/src/app/core/auth/auth.effects.ts @@ -201,13 +201,6 @@ export class AuthEffects { tap(() => this.authService.refreshAfterLogout()) ); - @Effect({ dispatch: false }) - public redirectToLogin$: Observable = this.actions$ - .pipe(ofType(AuthActionTypes.REDIRECT_AUTHENTICATION_REQUIRED), - tap(() => this.authService.removeToken()), - tap(() => this.authService.redirectToLogin()) - ); - @Effect({ dispatch: false }) public redirectToLoginTokenExpired$: Observable = this.actions$ .pipe( diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 7d854d9d4d..b8e6c6609a 100644 --- a/src/app/core/auth/auth.service.ts +++ b/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 + protected store: Store, + protected hardRedirectService: HardRedirectService ) { this.store.pipe( select(isAuthenticated), @@ -440,26 +442,18 @@ export class AuthService { } protected navigateToRedirectUrl(redirectUrl: string) { - const url = decodeURIComponent(redirectUrl); - // in case the user navigates directly to /login (via bookmark, etc), or the route history is not found. - if (isEmpty(url) || url.startsWith(LOGIN_ROUTE)) { - 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); + let url = `/reload/${new Date().getTime()}`; + if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) { + url += `?redirect=${encodeURIComponent(redirectUrl)}`; } + this.hardRedirectService.redirect(url); } /** * Refresh route navigated */ public refreshAfterLogout() { - // Hard redirect to the reload page with a unique number behind it - // so that all state is definitely lost - this._window.nativeWindow.location.href = `/reload/${new Date().getTime()}`; + this.navigateToRedirectUrl(undefined); } /** diff --git a/src/app/core/auth/authenticated.guard.ts b/src/app/core/auth/authenticated.guard.ts index 7a2f39854c..2e70a70310 100644 --- a/src/app/core/auth/authenticated.guard.ts +++ b/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 { RedirectWhenAuthenticationIsRequiredAction } from './auth.actions'; +import { AuthService, LOGIN_ROUTE } from './auth.service'; /** * 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 { + canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { 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 { + canActivateChild(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { return this.canActivate(route, state); } - /** - * True when user is authenticated - * @method canLoad - */ - canLoad(route: Route): Observable { - const url = `/${route.path}`; - - return this.handleAuth(url); - } - - private handleAuth(url: string): Observable { + private handleAuth(url: string): Observable { // get observable const observable = this.store.pipe(select(isAuthenticated)); // redirect to sign in page if user is not authenticated - observable.pipe( - // .filter(() => isEmpty(this.router.routerState.snapshot.url) || this.router.routerState.snapshot.url === url) - take(1)) - .subscribe((authenticated) => { - if (!authenticated) { + return observable.pipe( + map((authenticated) => { + if (authenticated) { + return authenticated; + } else { this.authService.setRedirectUrl(url); - this.store.dispatch(new RedirectWhenAuthenticationIsRequiredAction('Login required')); + this.authService.removeToken(); + return this.router.createUrlTree([LOGIN_ROUTE]); } - }); - - return observable; + }) + ); } } diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 5aa462d5e0..bc2f80830c 100644 --- a/src/app/core/core.module.ts +++ b/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, diff --git a/src/app/core/reload/reload.guard.ts b/src/app/core/reload/reload.guard.ts new file mode 100644 index 0000000000..9880fabb69 --- /dev/null +++ b/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']); + } + } +} diff --git a/src/app/core/services/browser-hard-redirect.service.ts b/src/app/core/services/browser-hard-redirect.service.ts new file mode 100644 index 0000000000..56c9054297 --- /dev/null +++ b/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; + } +} diff --git a/src/app/core/services/hard-redirect.service.ts b/src/app/core/services/hard-redirect.service.ts new file mode 100644 index 0000000000..e2c18b6138 --- /dev/null +++ b/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(); +} diff --git a/src/app/core/services/server-hard-redirect.service.ts b/src/app/core/services/server-hard-redirect.service.ts new file mode 100644 index 0000000000..4d58443cf4 --- /dev/null +++ b/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(); + } +} diff --git a/src/modules/app/browser-app.module.ts b/src/modules/app/browser-app.module.ts index 73a49b0211..44f00fbae4 100644 --- a/src/modules/app/browser-app.module.ts +++ b/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('req'); @@ -32,6 +34,13 @@ export function getRequest(transferState: TransferState): any { return transferState.get(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 { diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index 0ba09182cc..c8ea81bdec 100644 --- a/src/modules/app/server-app.module.ts +++ b/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 {