Skip to content

Commit

Permalink
Fix infinite loops on nested private routes with roles (#9204)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
KrisCoulson and dac09 authored Sep 19, 2023
1 parent af54305 commit f15cecd
Show file tree
Hide file tree
Showing 4 changed files with 348 additions and 53 deletions.
109 changes: 91 additions & 18 deletions packages/router/src/__tests__/analyzeRoutes.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,12 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
whileLoadingPage: undefined,
wrappers: [WrapperX],
// Props passed through from set
setProps: {
id: 'set-one',
passThruProp: 'bazinga',
},
setProps: [
{
id: 'set-one',
passThruProp: 'bazinga',
},
],
})
)

Expand All @@ -158,11 +160,16 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
path: '/b',
whileLoadingPage: undefined,
wrappers: [WrapperX, WrapperY], // both wrappers
setProps: {
id: 'set-two', // the id gets overwritten by the second Set
theme: 'blue',
passThruProp: 'bazinga', // from the first set
},
setProps: [
{
id: 'set-one',
passThruProp: 'bazinga',
},
{
id: 'set-two',
theme: 'blue',
},
],
})
)

Expand All @@ -173,11 +180,16 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
path: '/c',
whileLoadingPage: undefined,
wrappers: [WrapperX, WrapperY], // both wrappers
setProps: {
id: 'set-two',
theme: 'blue',
passThruProp: 'bazinga', // from the first set
},
setProps: [
{
id: 'set-one',
passThruProp: 'bazinga',
},
{
id: 'set-two',
theme: 'blue',
},
],
})
)
})
Expand Down Expand Up @@ -276,10 +288,12 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
page: FakePage,
wrappers: [],
setId: 1,
setProps: {
private: true,
unauthenticated: 'home',
},
setProps: [
{
private: true,
unauthenticated: 'home',
},
],
})
})

Expand Down Expand Up @@ -310,4 +324,63 @@ describe('AnalyzeRoutes: with homePage and Children', () => {
expect(namedRoutesMap.simple()).toBe('/simple')
expect(namedRoutesMap.rdSimple()).toBe('/rdSimple')
})

test('Redirect routes analysis', () => {
const HomePage = () => <h1>Home Page</h1>
const PrivateAdminPage = () => <h1>Private Admin Page</h1>
const PrivateEmployeePage = () => <h1>Private Employee Page</h1>
const PrivateNoRolesAssigned = () => <h1>Private Employee Page</h1>

const RedirectedRoutes = (
<Router>
<Route path="/" page={HomePage} name="home" />
<Private unauthenticated="home">
<Route
path="/no-roles-assigned"
page={PrivateNoRolesAssigned}
name="noRolesAssigned"
/>
<Set
private
unauthenticated="noRolesAssigned"
roles={['ADMIN', 'EMPLOYEE']}
someProp="propFromNoRolesSet"
>
<Private unauthenticated="admin" roles={'EMPLOYEE'}>
<Route
path="/employee"
page={PrivateEmployeePage}
name="privateEmployee"
/>
</Private>

<Private unauthenticated="employee" roles={'ADMIN'}>
<Route
path="/admin"
page={PrivateAdminPage}
name="privateAdmin"
/>
</Private>
</Set>
</Private>
</Router>
)

const { pathRouteMap, namedRoutesMap } = analyzeRoutes(
RedirectedRoutes.props.children,
{
currentPathName: '/simple',
}
)

console.log(JSON.stringify({ pathRouteMap, namedRoutesMap }, null, 3))

// expect(pathRouteMap['/simple'].redirect).toBe('/rdSimple')
// expect(pathRouteMap['/rdSimple'].redirect).toBeFalsy()

// // @TODO true for now, but we may not allow names on a redirect route
// expect(Object.keys(namedRoutesMap).length).toBe(2)
// expect(namedRoutesMap.simple()).toBe('/simple')
// expect(namedRoutesMap.rdSimple()).toBe('/rdSimple')
})
})
Loading

0 comments on commit f15cecd

Please sign in to comment.