Chrome (web) secured content seen for a brief moment

[teneon]

Hi there,

i have Flutter project with Riverpod, go_router. Take a look at this Go router "redirect" part of the code:

  /// Redirects the user when our authentication changes
  String? redirect(BuildContext context, GoRouterState state) {
    if (this.state.isLoading || this.state.hasError) return null;

    final isSplash = state.location == SplashPage.path;

    if (isSplash) {
      return isAuth ? HomePage.path : LoginPage.path;
    }

    final isLoggingIn = state.location == LoginPage.path;
    if (isLoggingIn) return isAuth ? HomePage.path : null;

    final isSigningUp = state.location == SignupPage.path;
    if (isSigningUp) return isAuth ? HomePage.path : null;

    return isAuth ? null : SplashPage.path;
  }

I am testing flutter web at the moment. Let's say when i start the app via VSCode Chrome opens this location: http://localhost:37471/#/splash for a brief moment and then it redirects to http://localhost:37471/#/login which is expected behaviour.

The problem is that if i open new tab in chrome and enter manually this URL: http://localhost:37471/#/home

i can see the whole "HomePage" for a brief moment (and i shouldn't see it since i am not logged in) and only then it redirects me to http://localhost:37471/#/login

Unauthenticated users should never be able to see "HomePage" or other protected pages if they change URL in Chrome manually, that would be a huge security issue, so i have changed the "redirect" part of the code like this:

String? redirect(BuildContext context, GoRouterState state) {
  if (this.state.isLoading || this.state.hasError) return SplashPage.path; 

  final isSplash = state.location == SplashPage.path;
  if (isSplash) {
    return isAuth ? HomePage.path : LoginPage.path;
  }

  final isLoggingIn = state.location == LoginPage.path;
  if (isLoggingIn) return isAuth ? HomePage.path : null;

  final isSigningUp = state.location == SignupPage.path;
  if (isSigningUp) return isAuth ? HomePage.path : null;

  return isAuth ? null : SplashPage.path;
}

...and now it works as expected, at least it looks like so. I am wondering what do you guys thinks. Is this the correct approach or did i break some other scenario(s)?

Best regards!

[lucavenir]

You're probably right, I never acknowledged this since my "HomePage" (or any protected route) usually depends on authState in some way (providers will eventually depend on that), thus these routes never show protected content for me (indeed, since authState is loading, every protected state will be loading as well).

But this isn't enough and I totally get your suggestion. I'd like to thank you for writing such a detailed issue with a solution into it.

Would you like to open a PR to fix this? I would still make a small change on the first line though.

  if (this.state.isLoading) return SplashPage.path;
  if (this.state.hasError) return LoginPage.path;
  ...

This is necessary, or else you wouldn't be able to retry a login if your authState fails somehow. But the main point is: not redirecting in these cases has an intrinsic fallacy. Apparently.

To actually certify this statement, I'd kindly ask you to write tests that verify the expected behaviors. This project used to have tests, but when I refactored / updated the content to the latest APIs I had to delete them (they were basically meaningless).

Let me know if you're up to do this small piece of work, else I'll leave this open and address this on my spare time

[teneon]

Hi ;) Thank you for your reply and specially for your good work. I have been busy with something else, so my reply took way too long, sry about that.

I am not yet familiar enough with Riverpod and GoRouter to be able to write unit tests. I will keep playing with your example to figure everything out first. If i will find out something interesting i will let you know!

Best regards!

[lucavenir]

I think we might close this for now; with the new changes, it should be fixed. Note that go_router has plenty of open and often deal breaking issues, such as this one, which may impact this issue in the future.