angular: NavigationExtras when returning URLTree from guard

🚀 feature request

Relevant Package

This feature request is for @angular/router

Description

In #26521 (Angular 7.1.0), a feature was introduced that allows returning a URLTree from guards in order to initiate a redirect. However, this doesn’t seem to allow using NavigationExtras such as skipLocationChange.

Since the change in #26521 wasn’t just syntax sugar, but is relevant to have predictable behavior when dealing with multiple guards, it should be considered to somehow allow using these options when returning a URLTree as well.

Describe the solution you’d like

I don’t have a really good proposal at this moment apart from next to returning URLTree also returning some kind of

// TODO: Find a better name?
interface NavigationOptions {
  urlTree: URLTree;
  navigationExtras: NavigationExtras;
}

such that I could use

class AwesomeGuard implements CanActivate {
  constructor(private router: Router) {}

  canActivate(): NavigationOptions | Observable<NavigationOptions> {
    return {
      urlTree: this.router.parseUrl("…"),
      navigationExtras: {
        skipLocationChange: true,
      },
    };
  }
}

I guess the exact typing of the guard would have to be

// Just a placeholder name
type GuardReturnType = boolean | URLTree | NavigationOptions;

interface CanActivate {
  canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): 
    GuardReturnType 
    | Promise<GuardReturnType> 
    | Observable<GuardReturnType>
}

Admittedly, the return type of this function is growing increasingly complex with this.

Describe alternatives you’ve considered

Possible options that I don’t think would make good solutions would be

  • Returning an array [URLTree, NavigationExtras] (this just feels unclean)
  • Extending URLTree with the navigationExtras and extend parseUrl to take them as well (this muddies the entire API for a change in a specific location)

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 97
  • Comments: 45 (18 by maintainers)

Commits related to this issue

Most upvoted comments

+1 to this issue as having navigationExtras when creating a UrlTree allows canActivate to preserve extras like state if the url is changed.

In addition, it would be nice if there’s a way to preserve the navigation extras when returning a url tree from CanActivate.

@Airblader @jayoungers @scott-ho @axelboc

Thanks for all the comments on this issue. I want to get this resolved somehow in the very near future. Ideally in the RC period of version 8 (there might be a bit of an issue doing so because it would be adding a feature).

I’m happy to see the 404 and 301 examples above from @axelboc.

We were looking at an issue internally and found that, with the redirects being done the way they currently are, the back button is essentially broken. I was looking at making a fix such that returning a UrlTree will cause the same redirect, but will always be done with replaceUrl. This would be turned on by default, because without it hitting back will end up redirecting again, and the user will not be able to go back past the page they redirected to.

I would love some feedback from anyone using this feature or wanting to use this feature. We can get this implemented quickly and pushed out as part of RC as a fix rather than the full feature described above.

That being said, the original design did contain an option to provide NavigationExtras but we didn’t have concrete use cases where it would be needed. Now, with those provided, we should be able to implement this API as well.

Two concrete use cases I have throughout my project in canActivate guards:

1) Redirecting to a 404 page when trying to access an unknown resource

  • The expected behaviour is that of skipLocationChange: the user should see the URL of the unknown resource, so they can fix a typo in the URL and try again, for instance.
  • With the UrlTree technique, at the end of the redirect, the URL shows /404 and the user can navigate back.

2) Performing a 301 redirect to a resource’s canonical URL

  • This is useful when a resource’s URL can be renamed or aliased, for instance. The expected behaviour is that of replaceUrl : the user should see the canonical URL without being able to navigate back to the obsolete/aliased URL.
  • With the UrlTree technique, the user can navigate back to the resource’s obsolete/aliased URL.

@jasonaden Thanks for the reply! Our concrete usecase is e.g. a 403 redirect where we want to show an error page but retain the URL.

So no, the forwarding of the navigation extras wouldn’t help us here, unfortunately.

I was confused that this wasn’t implemented because of the function createUrlTree on the router, which has the NavigationExtras as a parameter. I misunderstood what that function did.

Just to add my own case, I had a CanActivate guard verifying that the user is part of a certain group before accessing a page. If he isn’t, then redirect to an AccessDenied page that can display the group which the user lacks. The group name would’ve been passed to the AccessDenied page via NavigationExtras’ state within the guard, since the guard is better positioned to know the group name, and the AccessDenied component may potentially be used in different contexts, by different guards.

Hope that makes sense.

Hi, no progress or ETA on this, but here to provide some alternative workaround hackery for the skipLocationChange redirect:

TL;DR - for the skipLocationChange case, you can add this one-liner before the return <urlTree> in your guard: this.router.getCurrentNavigation().extras.skipLocationChange = true; Please add a test for this in with RouterTestingModule

Two pre-understanding points:

  • When the Router uses the UrlTree for redirecting, it inherits the skipLocationChange value from the last request (code reference.
  • The currentNavigation property in the Router gets the extras from the navigation request without doing a deep copy (code reference)

Because of the above two points, you can actually simply do this.router.getCurrentNavigation().extras.skipLocationChange = true; before the return <urlTree> in your guard. Doing this certainly depends on the Router implementation details so please add a test for your guard using RouterTestingModule and understand that it is possible that this could break with changes to the Router code (and those changes would not be considered breaking changes since the internal implementation isn’t public API).

Please don’t forget about some sort of feature parity with Resolve<T> when considering changes to URLTrees from guards.

Being able to redirect elegantly from a Resolver is very important - especially in the case where an invalid URL is provided (eg. a mistyped ID).

IMO, the developer defined state that gets added to the NavigationExtras should also be passed through when returning a UrlTree. Otherwise this additional information gets lost during the redirect

+1 for this feature request, I was honestly surprised that you can’t utilize anything like navigationExtras in a guard when you can do it everywhere else.

I was hoping to build some process validation into my navigation guards to prevent users from getting clever and trying to bypass the usual workflow by changing the URL. I can still do that the way thing are now, but I can’t funnel any custom data back through the navigationExtras in a neat and readable way.

I was initially thinking a good solution would be to provide some sort of “alternateRouterCommand” interface on the guard itself, but I think I might have a better solution that covers more scenarios. Would it be reasonable to provide an “onGuarded” callback property on a Route that aggregates the values returned by negative guards into an array until all guards have been processed, then calls the function you provide in the “onGuarded” property with the array of values returned from failed guards, target route, and current route? That would allow developers to build dynamic responses from rejected routes, predict the order in which guards will run, and put a lot of their error handling in a neat, centralized location.

Similar to other mentioned use cases, we would like to preserve the URL for error pages:

  • 403: Logged in, but is missing permission (should not occur by regular app usage). Manipulating URL or reloading page after permissions are revoked could trigger it. Show a generic ForbiddenComponent with error message (known by guard)
  • 404: For cases where a route matches (e.g. /user/:id), but a resource does not exist (or has been removed. A guard could be used to show a NotFound component

We also use a lot of redirects currently to populate query parameters. It would be great if it was possible to redirect and add missing parameters:

canActivate(route: ActivatedRouteSnapshot): boolean | UrlTree {
  if (route.params.page) return true;
  const queryParams = { page: '1' };
  const urlTree = this.router.createUrlTree(['.'], { relativeTo: route, queryParams });
  return { urlTree, preserveQueryParams: true, queryParamsHandling: 'merge' };
}

So basically, all options from NavigationExtras except relativeTo, queryParams, and fragment (since these are handled by router.createUrlTree) would be great to have IMO. 😀

The use case I have is similar to https://github.com/angular/angular/issues/17004 - being able to render a 404 response for the current route. I don’t want to redirect to a ‘404’ / ‘not-found’ route.

Hey, wanted to know the status of this feature request. No update for few months now, was it missed again?

Just stumbled upon this issue, I guess the only workaround now to not break back button is to return false and schedule a navigation in set timeout in a guard.

Interesting to see that not many people are hitting this issue

EDIT: This seems to work as well in canActivate:

this.router.events.pipe(
  filter(ev => ev instanceof NavigationCancel),
  take(1),
  delay(1)
).subscribe(() => {
  this.router.navigateByUrl(redirectUrl, { replaceUrl: true });
});

EDIT: added delay(1) operator, apparently it is needed to avoid duplicate redirect/events at least when HashLocationStrategy is used (#16710)

@jasonaden the v8 fix you’re suggesting sounds great. You’re right, it would at least fix the back button for now.

@timminss not a good one, I’m afraid. We did something similar to the below code snippet, but using a generic “error” page for various errors instead of a specific route for each error. The errors generated are from the HttpClient when resolving data, so they had err.status that we could check for routing purposes.

This was the best we could come up with 2-3 years ago when we wrote the code. We weren’t able to find much documentation for more advanced routing use-cases at the time.

@Injectable()
export class Route403Service {
  constructor(private router: Router, private location: Location) {}

  init() {
    let navError: NavigationError;
    this.router.events
      .pipe(filter((e): e is NavigationError => e instanceof NavigationError))
      .subscribe(e => navError = e);

    this.router.errorHandler = err => {
      if (err.status === 403) {
        this.router.navigate(['/not-found'], { skipLocationChange: true }).then(() => {
          // Show the route that couldn't be navigated to in the browser
          this.location.replaceState(navError.url);
        });
      }
    };
  }
}

You may be better off with some sort of view cover indicator that exists at a root level to indicate the route is loading: I think you’ll run into issues eventually for more complicated scenarios with redirecting from the intended route and back

My apologies on this. It should have been in 8, but got missed. See above for the fix. This should land shortly.

But the benefit and entire point of allowing returning a tree is that it makes how multiple guards interact predictable and stable. This feature wasn’t about specific usecases. I’d like to start returning URL tree in as many cases as I can (whenever I don’t return true).

@Airblader good point.


the current code is using a serialized url to redirecting, see the source.

@jasonaden Concrete use cases are not the major concern for the feature request. There are so many possibilities.

would that cause problems in the application? that would be an application problem but not a framework problem.

Since for the first glance, you don’t have valid point about what will be bad for the framework. I believe we could move on to make this happen.

But the benefit and entire point of allowing returning a tree is that it makes how multiple guards interact predictable and stable. This feature wasn’t about specific usecases. I’d like to start returning URL tree in as many cases as I can (whenever I don’t return true).

I see. I think the answer comes down to what the intention of returning UrlTree is:

I’m looking at is as though the requested route is not valid under any circumstance. If we’re looking at it as a short hand for the navigate method, then what you’re saying makes perfect sense.

In my mind I would have had the functionality setup so that if UrlTree is returned, then that is the route the user intended to go to (the guard result wasn’t true, it wasn’t false, it just lead to the correct path).

In scenarios such as authentication for example, where the user isn’t logged in or the session has expired, I don’t think UrlTree is the correct response: it should be false (this route is valid, but you don’t have access to it), and the guard router.navigates to the login page.

is the route replacement the unexpected/confusing part?

The unexpected part is that the navigation works differently than any other navigation where I don’t set any navigation extras. IMHO here Angular would suddenly dictate opinion over what navigation does or doesn’t make sense, and I would like to avoid that. Everywhere else the user gets to choose the options, so it should be the same here. Failing that, it should at least behave the same way as everywhere else (which is the current behavior).

I had a whole comment ready to go to post on the primary issue (#24618) assuming this was a bug until I ran into this issue: in my opinion I would say the default use case should be for it to consider this a redirectTo and skipLocationChange - I’m not sure under what scenarios you’d want to keep the original route.

Here are the main scenarios I’m returning UrlTree currently:

  1. The application is multi-tenant, where each user is a member of various tenants. If a user enters the site at the base / URL, my authorization guard will determine which tenants they are a member of and redirect the user to the last tenant they were viewing (e.g. tenantA).

    Result: user has entries of / and /tenantA in their browser history and can navigate back to /

  2. Shortened links: Let’s say I have a guard that will transform the url /tenantA/subentities/456 to the url /tenantA/entities/123/subentities/456 (via an API call that determined the parent id). The reason being other systems may not know the parent entity id.

    Result: user has entries of /tenantA/subentities/456 and /tenantA/entities/123/subentities/456, allowing them to navigate back to the invalid url

  3. Link repair: using the same example from above, given the url /tenantA/entities/123/subentities/456, if the entities id was really 222, the guard can return a repaired url of /tenantA/entities/222/subentities/456 (in these two use cases the subentity is truly the primary entity we care about, and the entities level is more of a vanity hierarchical collection)

    A similar example could be around formatting (i.e. if your route has a date like /1-1-2000/ and you redirect to standardize it to /01-01-2000/)

In all of these examples I wouldn’t want the original url to exist as valid locations in the browser