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

[Bug?]: Routes infinitely redirect users after upgrading to Redwood v6 #9131

Closed
1 task
ahoopes16 opened this issue Sep 3, 2023 · 6 comments
Closed
1 task
Labels
bug/confirmed We have confirmed this is a bug

Comments

@ahoopes16
Copy link

What's not working?

Hello there,

I started my project using Redwood v5, and my routes are setup like this:

const Routes = () => {
  return (
    <Router useAuth={useAuth}>
      <Route path="/" page={LoginPage} name="login" />
      <Route path="/forgot-password" page={ForgotPasswordPage} name="forgotPassword" />
      <Route path="/reset-password" page={ResetPasswordPage} name="resetPassword" />
      <Route path="/verify" page={VerifyPage} name="verify" />

      <Set private unauthenticated="login" wrap={MainLayout}>
        <Route path="/no-roles-assigned" page={NoRolesAssignedPage} name="noRolesAssigned" />

        <Set private unauthenticated="noRolesAssigned" roles={[Role.Administrator, Role.Employee]}>
          <Private unauthenticated="admin" roles={Role.Employee}>
            {/** Employee Routes */}
          </Private>

          <Private unauthenticated="employee" roles={Role.Administrator}>
            {/** Administrator Routes */}
          </Private>
        </Set>
      </Set>
      <Route notfound page={NotFoundPage} />
    </Router>
  )
}

While I was still using RedwoodJS v5, the unauthenticated and roles parameters worked exactly as expected. If a user was logged in with only the "Employee" or only the "Administrator" role, they could only access their respective routes, and if they tried to access one of the other roles' routes they would automatically get redirected to their pages. If a user was logged in but had no roles, they would be navigated to the No Roles Assigned Page. If a user was not logged in but tried to access any of the inner routes they would automatically get redirected to the login page.

After upgrading to Redwood v6, if I try to access the /employee route without being logged in, it gets stuck in an infinite loop redirecting between /employee and /admin.

Is there a different way that I need to organize these routes in Redwood v6 to get the same behavior I used to have? Or is this a true defect in how the routing is handled now?

How do we reproduce the bug?

It seems like as long as you have your routes in a similar configuration to the one that I posted above, you should run into this issue.

  1. Have a "home" route that users can access without being authenticated
  2. Have a <Set private unauthenticated="home" /> set of routes
  3. Have a route for when no roles are assigned to your authenticated users called noRolesAssigned inside the Set from step 2
  4. Have a nested <Set private unauthenticated="noRolesAssigned /> inside that first set
  5. Inside the nested Set, add two <Private /> components, one for each of 2 roles in your system
  6. On each Private tag, add props that set the roles prop to one of the roles and then set the unauthenticated prop to a page inside the other Private tag
  7. Without logging in, attempt to navigate to one of those Private routes
  8. Observe the browser get locked up as it infinitely loops between the two rather than redirecting you to the home page

What's your environment? (If it applies)

System:
    OS: macOS 13.3.1
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.14.1 - /private/var/folders/v4/678lwd551ds0yz879_vvtdy40000gn/T/xfs-732c8753/node
    Yarn: 3.5.0 - /private/var/folders/v4/678lwd551ds0yz879_vvtdy40000gn/T/xfs-732c8753/yarn
  Databases:
    SQLite: 3.39.5 - /usr/bin/sqlite3
  Browsers:
    Chrome: 116.0.5845.140
    Edge: 116.0.1938.54
    Safari: 16.4
  npmPackages:
    @redwoodjs/auth-dbauth-setup: 6.1.0 => 6.1.0 
    @redwoodjs/core: 6.1.0 => 6.1.0

Are you interested in working on this?

  • I'm interested in working on this
@ahoopes16 ahoopes16 added the bug/needs-info More information is needed for reproduction label Sep 3, 2023
@jtoar
Copy link
Contributor

jtoar commented Sep 12, 2023

Hey @ahoopes16, thanks for reporting and the reproduction steps, I can reproduce it! Investigating now.

@jtoar jtoar added bug/confirmed We have confirmed this is a bug and removed bug/needs-info More information is needed for reproduction labels Sep 12, 2023
@ahoopes16
Copy link
Author

I'm so glad I'm not going crazy! Thank you for taking a look.

@jtoar
Copy link
Contributor

jtoar commented Sep 12, 2023

@ahoopes16 brought this up to the team today—we identified the reason for the bug. In v6, many router components like Set and Private became "virtual" in the sense that they aren't rendered as written, but are used as a kind of DSL though that description is probably taking it too far. (Their wrap props get rendered of course.) But in doing this we ignored the nested sequence of authentication/role checks. As a result, only the nearest Set/Private component is being used, instead of them being processed in sequence as originally intended. We'll fix this in a patch!

@ahoopes16
Copy link
Author

@jtoar Fantastic, thanks so much for the quick triage information! I look forward to the patch.

jtoar pushed a commit that referenced this issue Sep 19, 2023
Fix for #9131

This handles use cases where there are multiple nested routes with
different levels of authentication and redirect contexts

---------

Co-authored-by: Daniel Choudhury <dannychoudhury@gmail.com>
jtoar pushed a commit that referenced this issue Sep 20, 2023
Fix for #9131

This handles use cases where there are multiple nested routes with
different levels of authentication and redirect contexts

---------

Co-authored-by: Daniel Choudhury <dannychoudhury@gmail.com>
jtoar pushed a commit that referenced this issue Nov 2, 2023
Fix for #9131

This handles use cases where there are multiple nested routes with
different levels of authentication and redirect contexts

---------

Co-authored-by: Daniel Choudhury <dannychoudhury@gmail.com>
@jtoar
Copy link
Contributor

jtoar commented Nov 16, 2023

@ahoopes16 sorry it took much longer than anticipated, but this fix just landed in v6.4

@jtoar jtoar closed this as completed Nov 16, 2023
@ahoopes16
Copy link
Author

No worries, thanks so much for getting it fixed at all! My users will be thrilled. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug
Projects
None yet
Development

No branches or pull requests

2 participants