flutter icon indicating copy to clipboard operation
flutter copied to clipboard

[go_router] Prevent access to route when navigated to via a deep link

Open DamienMrtl opened this issue 2 years ago • 13 comments

Use case

Prevent access to a route with deeplink. In the redirect function I would like to detect if the navigation event is a deeplink and redirect to the parent route.

Proposal

Add a bool in the state that says if its deeplink or not

DamienMrtl avatar May 12 '22 21:05 DamienMrtl

Can you explain more on your use case?

chunhtai avatar May 19 '22 22:05 chunhtai

By default, any deep link is mapped to route configuration, even if route is not supposed to be accessed through deeplink. I suppose OP wants to disable deep linking to some pages. @chunhtai

AlexanderFarkas avatar May 20 '22 09:05 AlexanderFarkas

@chunhtai Yes I'm using Flutter Web and I want some routes to be accessed only by the user navigating "in-app" and not by entering the URL directly. For now I'm manually setting the extra argument to true when navigating in app, so I can differentiate between deeplinks or user navigation.

DamienMrtl avatar May 23 '22 17:05 DamienMrtl

Thanks for explanation, I think instead of add a boolean to the state, we should have a more comprehensive API to handle Deeplink

For example

GoRouter(
  ....
  onDeepLink: (RouteInformation info) => ....
)

and by default , it will go through the regular parsing if this method is not provided

chunhtai avatar May 23 '22 18:05 chunhtai

@chunhtai It's a good idea yes, but maybe the function onDeepLink should also be supported on each GoRoute(...). So we don't have to do some complicated "if else" or "switch case" for each routes we want to intercept in the global onDeepLink function.

DamienMrtl avatar May 23 '22 18:05 DamienMrtl

@chunhtai I need this feature also. But it maybe more suitable with a wider concept rather than deeplink merely. The wider concept is preventing an access going forward. By the way, is there any other method to prevent an access?

Thanks for explanation, I think instead of add a boolean to the state, we should have a more comprehensive API to handle Deeplink

For example

GoRouter(
  ....
  onDeepLink: (RouteInformation info) => ....
)

and by default , it will go through the regular parsing if this method is not provided

horo99 avatar Jun 13 '22 11:06 horo99

I would also like this feature, so I came to say my use case.

My whole app consists of 3 screens: register -> waiting for approval -> approved / rejected. I was looking for some way to prevent user from navigating by modifying URL. I mean, nothing would happen, but it looks better to disable that navigation instead of showing some error pages or so.

I made a workaround by using boolean which I set to true everytime I navigate using GoRouter. Then in redirect I'm checking if it's true (allow navigation) or false (prevent navigation). This currently works good enough for me.

redirect: (state) {
  // triggered by URL modification
  if (!_routerNavigated) {
    // allow app start
    if (_router.location == '') {
      return null;
    // allow manual navigation to register screen
    } else if (state.subloc.startsWith('/register')) {
      return null;
    // to prevent redirect loop
    } else if (_router.location == state.subloc) {
      return null;
    // otherwise return back to the current location
    } else {
      return _router.location;
    }
  // triggered by GoRouter navigation
  } else {
    _routerNavigated = false;
  }

  return null;
}

My other idea was some ability to allow ignoring URL bar completely. In my use case I don't need to update the URL bar, I don't need to preserve history and I also don't need to navigate using deeplinks (except for the first register screen).

leoshusar avatar Aug 01 '22 22:08 leoshusar

But it maybe more suitable with a wider concept rather than deeplink merely. The wider concept is preventing an access going forward. By the way, is there any other method to prevent an access?

you can use redirect to always redirect to a different path.

For the deeplink, right now browser backward and forward button are treated the same as deeplink. Although there is a way to differentiate them, they are really similar in nature. If app were to prevent deeplink access of a page, should the browser backward and forward button be prevented as well?

chunhtai avatar Oct 20 '22 20:10 chunhtai

We also have plan to improve overall deeplink setup process, the more flexible API we provide the less deeplink code we can automate for the developers. This requires more thoughts

chunhtai avatar Nov 03 '22 20:11 chunhtai

If app were to prevent deeplink access of a page, should the browser backward and forward button be prevented as well?

I think, the best would be to be able to do both depending on what we need. What I would do is in the redirect callback give access to a "RouterEvent" object witch contains informations about the event with properties like: inAppNavigation or urlNavigation, historyNavigation, deepLinkNavigation. Maybe using an enum or booleans.

DamienMrtl avatar Nov 04 '22 11:11 DamienMrtl

There is another issue to add support for handling deeplinks from different domains. https://github.com/flutter/flutter/issues/100624

We also need to expose domain information for deeplinks. One idea is that add a new string or something in the GoRouteState. If we do that, we can check the domain information and use that to determine if it is a deeplink.

chunhtai avatar Feb 01 '23 22:02 chunhtai

Hi, can we make automatic handling optional? I mean I want to run some preconditions and also want to push rather than go.

junaid1460 avatar Jul 14 '23 10:07 junaid1460

I mean a notification comes and user is in a flow he taps and everything gone

junaid1460 avatar Jul 14 '23 10:07 junaid1460

I think there are two steps to fix this issue

  1. https://github.com/flutter/packages/pull/4392/files This adds the ability to know whether a GoRouteState is a deeplink (by checking GoRouteState.uri.host and GoRouteState.uri.scheme)
  2. https://github.com/flutter/flutter/issues/102408 The onenter and onexit should have more flexible api to decide whether to push page or doing other operation.

chunhtai avatar Jul 21 '23 19:07 chunhtai

Hi, when is this scheduled to release ? I have the same requirement to restrict deeplink, without going through login page.

Harris-Thomas avatar Oct 28 '23 04:10 Harris-Thomas

I think there are two steps to fix this issue

  1. https://github.com/flutter/packages/pull/4392/files This adds the ability to know whether a GoRouteState is a deeplink (by checking GoRouteState.uri.host and GoRouteState.uri.scheme)
  2. [go_router] Replace redirect with onEnter and onExit #102408 The onenter and onexit should have more flexible api to decide whether to push page or doing other operation.

You mention that you can tell whether a GoRouteState is a deep link by checking GoRouteState.uri.host and GoRouteState.uri.scheme. Both of these values are "" when I check them in the redirect, does this sound right?

If so, the only way to tell if a state is a deep link or initiated from within the app is to add some extra denoting that it is not a deeplink, as others have suggested?

ChopinDavid avatar Dec 15 '23 21:12 ChopinDavid

@chunhtai I was able to get back a GoRouteState.uri.host and GoRouteState.uri.scheme in our redirect with, seemingly, no side-effects by making the following changes:

I can get moving on making a PR for this if this makes sense to you...

ChopinDavid avatar Dec 17 '23 21:12 ChopinDavid

You mention that you can tell whether a GoRouteState is a deep link by checking GoRouteState.uri.host and GoRouteState.uri.scheme. Both of these values are "" when I check them in the redirect, does this sound right?

This is expected as of now, since go_router can't migrate to use RouteInformation.uri yet.

Edit: looks like the stable has reach 3.12.0 and we should be able to use it now

chunhtai avatar Dec 18 '23 22:12 chunhtai

I can get moving on making a PR for this if this makes sense to you...

yes, feel free to send out a pr.

chunhtai avatar Dec 18 '23 22:12 chunhtai

Actually I was wrong package is supporting 3.10.0 and up, so the routeinformation.uri isn't yet available.

update: just consulted with eco team, it looks like we can bump the version to latest stable if we want to.

chunhtai avatar Dec 18 '23 22:12 chunhtai

@chuntai So, we would need a PR that:

  1. Upgrades go_router's min. flutter version to >=3.12.0 (so that RouteInformation.uri exists)
  2. Implements my aforementioned changes which make use of RouteInformation.uri rather than RouteInformation.location
  3. Adds new tests/makes sure that no tests are broken

correct?

ChopinDavid avatar Dec 19 '23 22:12 ChopinDavid

@ChopinDavid that sounds correct

chunhtai avatar Dec 20 '23 20:12 chunhtai

PR is open @chunhtai: https://github.com/flutter/packages/pull/5742

ChopinDavid avatar Dec 22 '23 20:12 ChopinDavid

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

github-actions[bot] avatar Feb 23 '24 18:02 github-actions[bot]