From 4e9f2e58957b65d08ff97bbc6374673597c65fa5 Mon Sep 17 00:00:00 2001 From: Jason Aden Date: Wed, 17 Oct 2018 09:30:45 -0700 Subject: [PATCH] feat(router): guard returning UrlTree cancels current navigation and redirects (#26521) Fixes #24618 FW-153 #resolve PR Close #26521 --- aio/content/guide/router.md | 4 +- packages/router/src/interfaces.ts | 22 ++++++-- packages/router/src/operators/check_guards.ts | 2 +- .../src/operators/prioritized_guard_value.ts | 9 ++- packages/router/src/router.ts | 27 +++++++-- packages/router/src/utils/type_guards.ts | 7 ++- packages/router/test/integration.spec.ts | 55 ++++++++++++++++++- packages/router/test/router.spec.ts | 13 +++-- 8 files changed, 116 insertions(+), 23 deletions(-) diff --git a/aio/content/guide/router.md b/aio/content/guide/router.md index 2dc35211fd..17ce59a557 100644 --- a/aio/content/guide/router.md +++ b/aio/content/guide/router.md @@ -3066,11 +3066,13 @@ A guard's return value controls the router's behavior: * If it returns `true`, the navigation process continues. * If it returns `false`, the navigation process stops and the user stays put. +* If it returns a `UrlTree`, the current navigation cancels and a new navigation is initiated to the `UrlTree` returned.
-**Note:** The guard can also tell the router to navigate elsewhere, effectively canceling the current navigation. +**Note:** The guard can also tell the router to navigate elsewhere, effectively canceling the current navigation. When +doing so inside a guard, the guard should return `false`;
diff --git a/packages/router/src/interfaces.ts b/packages/router/src/interfaces.ts index eb4d66428c..47fa23dc9c 100644 --- a/packages/router/src/interfaces.ts +++ b/packages/router/src/interfaces.ts @@ -17,6 +17,10 @@ import {UrlSegment, UrlTree} from './url_tree'; * @description * * Interface that a class can implement to be a guard deciding if a route can be activated. + * If all guards return `true`, navigation will continue. If any guard returns `false`, + * navigation will be cancelled. If any guard returns a `UrlTree`, current navigation will + * be cancelled and a new navigation will be kicked off to the `UrlTree` returned from the + * guard. * * ``` * class UserToken {} @@ -33,7 +37,7 @@ import {UrlSegment, UrlTree} from './url_tree'; * canActivate( * route: ActivatedRouteSnapshot, * state: RouterStateSnapshot - * ): Observable|Promise|boolean|UrlTree { + * ): Observable|Promise|boolean|UrlTree { * return this.permissions.canActivate(this.currentUser, route.params.id); * } * } @@ -90,7 +94,11 @@ export type CanActivateFn = (route: ActivatedRouteSnapshot, state: RouterStateSn * @description * * Interface that a class can implement to be a guard deciding if a child route can be activated. - * + * If all guards return `true`, navigation will continue. If any guard returns `false`, + * navigation will be cancelled. If any guard returns a `UrlTree`, current navigation will + * be cancelled and a new navigation will be kicked off to the `UrlTree` returned from the + * guard. + * * ``` * class UserToken {} * class Permissions { @@ -106,7 +114,7 @@ export type CanActivateFn = (route: ActivatedRouteSnapshot, state: RouterStateSn * canActivateChild( * route: ActivatedRouteSnapshot, * state: RouterStateSnapshot - * ): Observable|Promise|boolean|UrlTree { + * ): Observable|Promise|boolean|UrlTree { * return this.permissions.canActivate(this.currentUser, route.params.id); * } * } @@ -173,7 +181,11 @@ export type CanActivateChildFn = (childRoute: ActivatedRouteSnapshot, state: Rou * @description * * Interface that a class can implement to be a guard deciding if a route can be deactivated. - * + * If all guards return `true`, navigation will continue. If any guard returns `false`, + * navigation will be cancelled. If any guard returns a `UrlTree`, current navigation will + * be cancelled and a new navigation will be kicked off to the `UrlTree` returned from the + * guard. + * * ``` * class UserToken {} * class Permissions { @@ -191,7 +203,7 @@ export type CanActivateChildFn = (childRoute: ActivatedRouteSnapshot, state: Rou * currentRoute: ActivatedRouteSnapshot, * currentState: RouterStateSnapshot, * nextState: RouterStateSnapshot - * ): Observable|Promise|boolean|UrlTree { + * ): Observable|Promise|boolean|UrlTree { * return this.permissions.canDeactivate(this.currentUser, route.params.id); * } * } diff --git a/packages/router/src/operators/check_guards.ts b/packages/router/src/operators/check_guards.ts index a8e80212c0..39c6af9216 100644 --- a/packages/router/src/operators/check_guards.ts +++ b/packages/router/src/operators/check_guards.ts @@ -17,7 +17,7 @@ import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../router_state'; import {UrlTree} from '../url_tree'; import {wrapIntoObservable} from '../utils/collection'; import {CanActivate, CanDeactivate, getCanActivateChild, getToken} from '../utils/preactivation'; -import {isCanActivate, isCanActivateChild, isCanDeactivate, isFunction, isBoolean} from '../utils/type_guards'; +import {isBoolean, isCanActivate, isCanActivateChild, isCanDeactivate, isFunction} from '../utils/type_guards'; import {prioritizedGuardValue} from './prioritized_guard_value'; diff --git a/packages/router/src/operators/prioritized_guard_value.ts b/packages/router/src/operators/prioritized_guard_value.ts index c44e28acce..5971db743b 100644 --- a/packages/router/src/operators/prioritized_guard_value.ts +++ b/packages/router/src/operators/prioritized_guard_value.ts @@ -7,9 +7,10 @@ */ import {Observable, OperatorFunction, combineLatest} from 'rxjs'; -import {filter, scan, startWith, switchMap, take} from 'rxjs/operators'; +import {filter, map, scan, startWith, switchMap, take} from 'rxjs/operators'; import {UrlTree} from '../url_tree'; +import {isUrlTree} from '../utils/type_guards'; const INITIAL_VALUE = Symbol('INITIAL_VALUE'); declare type INTERIM_VALUES = typeof INITIAL_VALUE | boolean | UrlTree; @@ -38,7 +39,7 @@ export function prioritizedGuardValue(): // navigation if (val === false) return val; - if (i === list.length - 1 || val instanceof UrlTree) { + if (i === list.length - 1 || isUrlTree(val)) { return val; } } @@ -47,6 +48,8 @@ export function prioritizedGuardValue(): }, acc); }, INITIAL_VALUE), - filter(item => item !== INITIAL_VALUE), take(1)) as Observable; + filter(item => item !== INITIAL_VALUE), + map(item => isUrlTree(item) ? item : item === true), // + take(1)) as Observable; }); } diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 8e96990f34..ef6528f7a5 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -8,7 +8,7 @@ import {Location} from '@angular/common'; import {Compiler, Injector, NgModuleFactoryLoader, NgModuleRef, NgZone, Type, isDevMode, ɵConsole as Console} from '@angular/core'; -import {BehaviorSubject, EMPTY, Observable, Subject, Subscription, of } from 'rxjs'; +import {BehaviorSubject, EMPTY, Observable, Subject, Subscription, defer, of } from 'rxjs'; import {catchError, filter, finalize, map, switchMap, tap} from 'rxjs/operators'; import {QueryParamsHandling, Route, Routes, standardizeConfig, validateConfig} from './config'; @@ -25,10 +25,11 @@ import {DefaultRouteReuseStrategy, RouteReuseStrategy} from './route_reuse_strat import {RouterConfigLoader} from './router_config_loader'; import {ChildrenOutletContexts} from './router_outlet_context'; import {ActivatedRoute, RouterState, RouterStateSnapshot, createEmptyState} from './router_state'; -import {Params, isNavigationCancelingError} from './shared'; +import {Params, isNavigationCancelingError, navigationCancelingError} from './shared'; import {DefaultUrlHandlingStrategy, UrlHandlingStrategy} from './url_handling_strategy'; import {UrlSerializer, UrlTree, containsTree, createEmptyUrlTree} from './url_tree'; import {Checks, getAllRouteGuards} from './utils/preactivation'; +import {isUrlTree} from './utils/type_guards'; @@ -487,6 +488,14 @@ export class Router { })), checkGuards(this.ngModule.injector, (evt: Event) => this.triggerEvent(evt)), + tap(t => { + if (isUrlTree(t.guardsResult)) { + const error: Error&{url?: UrlTree} = navigationCancelingError( + `Redirecting to "${this.serializeUrl(t.guardsResult)}"`); + error.url = t.guardsResult; + throw error; + } + }), tap(t => { const guardsEnd = new GuardsCheckEnd( @@ -602,11 +611,19 @@ export class Router { * rather than an error. */ if (isNavigationCancelingError(e)) { this.navigated = true; - this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl); + const redirecting = isUrlTree(e.url); + if (!redirecting) { + this.resetStateAndUrl(t.currentRouterState, t.currentUrlTree, t.rawUrl); + } const navCancel = new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), e.message); eventsSubject.next(navCancel); t.resolve(false); + + if (redirecting) { + this.navigateByUrl(e.url); + } + /* All other errors should reset to the router's internal URL reference to the * pre-error state. */ } else { @@ -815,7 +832,7 @@ export class Router { `Navigation triggered outside Angular zone, did you forget to call 'ngZone.run()'?`); } - const urlTree = url instanceof UrlTree ? url : this.parseUrl(url); + const urlTree = isUrlTree(url) ? url : this.parseUrl(url); const mergedTree = this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree); return this.scheduleNavigation(mergedTree, 'imperative', null, extras); @@ -867,7 +884,7 @@ export class Router { /** Returns whether the url is activated */ isActive(url: string|UrlTree, exact: boolean): boolean { - if (url instanceof UrlTree) { + if (isUrlTree(url)) { return containsTree(this.currentUrlTree, url, exact); } diff --git a/packages/router/src/utils/type_guards.ts b/packages/router/src/utils/type_guards.ts index b6a29bcc80..91a32806e1 100644 --- a/packages/router/src/utils/type_guards.ts +++ b/packages/router/src/utils/type_guards.ts @@ -7,6 +7,7 @@ */ import {CanActivate, CanActivateChild, CanDeactivate, CanLoad} from '../interfaces'; +import {UrlTree} from '../url_tree'; /** * Simple function check, but generic so type inference will flow. Example: @@ -29,6 +30,10 @@ export function isBoolean(v: any): v is boolean { return typeof v === 'boolean'; } +export function isUrlTree(v: any): v is UrlTree { + return v instanceof UrlTree; +} + export function isCanLoad(guard: any): guard is CanLoad { return guard && isFunction(guard.canLoad); } @@ -38,7 +43,7 @@ export function isCanActivate(guard: any): guard is CanActivate { } export function isCanActivateChild(guard: any): guard is CanActivateChild { - return guard && isFunction(guard.canActivate); + return guard && isFunction(guard.canActivateChild); } export function isCanDeactivate(guard: any): guard is CanDeactivate { diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 03804df35c..3619c5f10f 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -9,7 +9,7 @@ import {CommonModule, Location} from '@angular/common'; import {SpyLocation} from '@angular/common/testing'; import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef, NgZone, OnDestroy, ɵConsole as Console, ɵNoopNgZone as NoopNgZone} from '@angular/core'; -import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/testing'; +import {ComponentFixture, TestBed, fakeAsync, flush, inject, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlSerializer, UrlTree} from '@angular/router'; @@ -2017,6 +2017,59 @@ describe('Integration', () => { }))); }); + describe('should redirect when guard returns UrlTree', () => { + beforeEach(() => TestBed.configureTestingModule({ + providers: [{ + provide: 'returnUrlTree', + useFactory: (router: Router) => () => { return router.parseUrl('/redirected'); }, + deps: [Router] + }] + })); + + it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => { + const recordedEvents: any[] = []; + let cancelEvent: NavigationCancel = null !; + router.events.forEach((e: any) => { + recordedEvents.push(e); + if (e instanceof NavigationCancel) cancelEvent = e; + }); + router.resetConfig([ + {path: '', component: SimpleCmp}, + {path: 'one', component: RouteCmp, canActivate: ['returnUrlTree']}, + {path: 'redirected', component: SimpleCmp} + ]); + + const fixture = TestBed.createComponent(RootCmp); + router.navigateByUrl('/one'); + + advance(fixture); + + expect(location.path()).toEqual('/redirected'); + expect(fixture.nativeElement).toHaveText('simple'); + expect(cancelEvent && cancelEvent.reason) + .toBe('NavigationCancelingError: Redirecting to "/redirected"'); + expectEvents(recordedEvents, [ + [NavigationStart, '/one'], + [RoutesRecognized, '/one'], + [GuardsCheckStart, '/one'], + [ChildActivationStart, undefined], + [ActivationStart, undefined], + [NavigationCancel, '/one'], + [NavigationStart, '/redirected'], + [RoutesRecognized, '/redirected'], + [GuardsCheckStart, '/redirected'], + [ChildActivationStart, undefined], + [ActivationStart, undefined], + [GuardsCheckEnd, '/redirected'], + [ResolveStart, '/redirected'], + [ResolveEnd, '/redirected'], + [ActivationEnd, undefined], + [ChildActivationEnd, undefined], + [NavigationEnd, '/redirected'], + ]); + }))); + }); + describe('runGuardsAndResolvers', () => { let guardRunCount = 0; let resolverRunCount = 0; diff --git a/packages/router/test/router.spec.ts b/packages/router/test/router.spec.ts index 926c10aa0a..81da038b92 100644 --- a/packages/router/test/router.spec.ts +++ b/packages/router/test/router.spec.ts @@ -672,10 +672,11 @@ function checkGuards( guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts()) } as Partial) .pipe(checkGuardsOperator(injector)) - .subscribe( - t => { - if (t.guardsResult === null) throw new Error('Guard result expected'); - return check(t.guardsResult); - }, - (e) => { throw e; }); + .subscribe({ + next(t) { + if (t.guardsResult === null) throw new Error('Guard result expected'); + return check(t.guardsResult); + }, + error(e) { throw e; } + }); }