Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
DamienMrtl opened this issue May 12, 2022 · 24 comments · Fixed by flutter/packages#5742
Closed

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

DamienMrtl opened this issue May 12, 2022 · 24 comments · Fixed by flutter/packages#5742
Assignees
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter p: go_router The go_router package P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team

Comments

@DamienMrtl
Copy link

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

@darshankawar darshankawar added in triage Presently being triaged by the triage team p: first party package flutter/packages repository. See also p: labels. c: proposal A detailed proposal for a change to Flutter p: go_router The go_router package c: new feature Nothing broken; request for a new capability and removed in triage Presently being triaged by the triage team labels May 13, 2022
@stuartmorgan stuartmorgan added the P3 Issues that are less important to the Flutter project label May 17, 2022
@chunhtai
Copy link
Contributor

Can you explain more on your use case?

@AlexanderFarkas
Copy link

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

@DamienMrtl
Copy link
Author

@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.

@chunhtai
Copy link
Contributor

chunhtai commented May 23, 2022

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

@DamienMrtl
Copy link
Author

@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.

@horo99
Copy link

horo99 commented Jun 13, 2022

@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

@leoshusar
Copy link

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).

@chunhtai
Copy link
Contributor

chunhtai commented Oct 20, 2022

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?

@johnpryan johnpryan changed the title [go_router] Detect deeplinks in redirects / Prevent access to route using deeplinks [go_router] Prevent access to route when navigated to via a deep link Oct 26, 2022
@chunhtai chunhtai self-assigned this Nov 3, 2022
@chunhtai
Copy link
Contributor

chunhtai commented Nov 3, 2022

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

@DamienMrtl
Copy link
Author

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.

@chunhtai
Copy link
Contributor

chunhtai commented Feb 1, 2023

There is another issue to add support for handling deeplinks from different domains. #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 chunhtai added team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team labels Jul 6, 2023
@junaid1460
Copy link

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

@junaid1460
Copy link

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

@chunhtai
Copy link
Contributor

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.

@Harris-Thomas
Copy link

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

@ChopinDavid
Copy link

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
Copy link

ChopinDavid commented Dec 17, 2023

@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...

@chunhtai
Copy link
Contributor

chunhtai commented Dec 18, 2023

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
Copy link
Contributor

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
Copy link
Contributor

chunhtai commented Dec 18, 2023

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.

@ChopinDavid
Copy link

ChopinDavid commented Dec 19, 2023

@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?

@chunhtai
Copy link
Contributor

@ChopinDavid that sounds correct

@ChopinDavid
Copy link

PR is open @chunhtai: flutter/packages#5742

auto-submit bot pushed a commit to flutter/packages that referenced this issue Feb 9, 2024
#5742)

A number of developers have voiced the desire to be able to know whether a `GoRouterState` is the result of a deep-link or in-app navigation from within their `GoRouter`'s `redirect` method. This can be accomplished by exposing the `Uri`'s `scheme` and `host` on the `GoRouterState`. This way, we can know whether the `GoRouterState` is the result of a deep-link by checking `state.uri.scheme != null` or `state.uri.host != null`.

This PR would close [#103659](flutter/flutter#103659 (comment)).

No tests were broken as a result of this change, and we have added coverage for this change through new tests.
Copy link

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 github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter p: go_router The go_router package P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team
Projects
No open projects
Status: No status
10 participants