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

feat: Reworks RSC server entries and route manifest building to derive from routes and include if route info related to authentication #10572

Merged
merged 22 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ecb5faf
Builds server entries from routes rather than pages
dthyresson May 15, 2024
3e42a4c
A route is private if it has a parent PrivateSet
dthyresson May 15, 2024
2b6ebf1
Adds isPrivate attribute to Route manifest
dthyresson May 15, 2024
674a725
Add changeset
dthyresson May 15, 2024
dfdea38
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 15, 2024
6416eba
Favor PrivateSet over Private in fixtures to authenticate routes. Tes…
dthyresson May 15, 2024
569b510
Adds unauthenticated path to RWRoute model
dthyresson May 16, 2024
74a9694
private redirect fix
dthyresson May 16, 2024
f7e35e8
update route manifest
dthyresson May 16, 2024
01f47f4
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
0f5b4f2
Had extraneous exp typo import
dthyresson May 16, 2024
9e9f062
remove console
dthyresson May 16, 2024
400580f
Add roles to building route manifest
dthyresson May 16, 2024
92659c3
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
778929a
improve changeset
dthyresson May 16, 2024
0ace4af
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
2e7bf8b
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 16, 2024
ba02c65
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 17, 2024
4584164
fix windows ci?
dthyresson May 17, 2024
6418794
Rename RWPage's const to constName
dthyresson May 21, 2024
7184362
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 21, 2024
a2ac9fc
Merge branch 'main' into dt-refactor-server-entries-from-routes
dthyresson May 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changesets/10572.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- feat: feat: Reworks RSC server entries and route manifest building to derive from routes and include if route info related to authentication (#10572) by @dthyresson

This PR is in furtherance of authentication support in RSC.

It refactors:

- How server entries are built -- not from "processing the pages dir" (which is a deprecated function) but rather the routes ... and the page info for that route. Note here that a page can be used in multiple routes, so the auth info cannot really be determined here.

- The route manifest building to include per route:

* isPrivate - is the route private, i.e, is it wrapped in a PrivateSet
* unauthenticated - what route to navigate to if the user in not authenticated
* roles - the roles to check to see if user has the require RBAC permission to navigate to the route

Now if some page, route request is being handled by RSC we might be able to check if it "isPrivate" and enforce auth with the roles and even where tp redirect to if not authenticated.
12 changes: 9 additions & 3 deletions __fixtures__/example-todo-main/web/src/Routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// 'src/pages/HomePage/HomePage.js' -> HomePage
// 'src/pages/Admin/BooksPage/BooksPage.js' -> AdminBooksPage

import { Private, Router, Route } from '@redwoodjs/router'
import { PrivateSet, Router, Route } from '@redwoodjs/router'
import SetLayout from 'src/layouts/SetLayout'

import FooPage from 'src/pages/FooPage'
Expand All @@ -22,9 +22,15 @@ const Routes = () => {
<Route path="/foo" page={FooPage} name="fooPage" />
<Route path="/bar" page={BarPage} name="barPage" />
</Set>
<Private prerender unauthenticated="home">
<PrivateSet unauthenticated="home" prerender>
<Route path="/private-page" page={PrivatePage} name="privatePage" />
</Private>
</PrivateSet>
<PrivateSet unauthenticated="home" roles="admin" prerender>
<Route path="/private-page-admin" page={PrivatePage} name="privatePageAdmin" />
</PrivateSet>
<PrivateSet unauthenticated="home" roles={['owner', 'superuser']} prerender>
<Route path="/private-page-admin-super" page={PrivatePage} name="privatePageAdminSuper" />
</PrivateSet>
<Route notfound page={NotFoundPage} />
</Router>
)
Expand Down
6 changes: 3 additions & 3 deletions __fixtures__/fragment-test-project/web/src/Routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// 'src/pages/HomePage/HomePage.js' -> HomePage
// 'src/pages/Admin/BooksPage/BooksPage.js' -> AdminBooksPage

import { Router, Route, Private, Set } from '@redwoodjs/router'
import { Router, Route, PrivateSet, Set } from '@redwoodjs/router'

import BlogLayout from 'src/layouts/BlogLayout'
import ScaffoldLayout from 'src/layouts/ScaffoldLayout'
Expand Down Expand Up @@ -38,9 +38,9 @@ const Routes = () => {
</Set>
<Set wrap={BlogLayout}>
<Route path="/waterfall/{id:Int}" page={WaterfallPage} prerender name="waterfall" />
<Private unauthenticated="login">
<PrivateSet unauthenticated="login">
<Route path="/profile" page={ProfilePage} name="profile" />
</Private>
</PrivateSet>
<Route path="/blog-post/{id:Int}" page={BlogPostPage} name="blogPost" prerender />
<Route path="/contact" page={ContactUsPage} name="contactUs" />
<Route path="/about" page={AboutPage} name="about" prerender />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ exports[`generates global page imports source maps 1`] = `
exports[`generates source maps for the router routes 1`] = `
{
"file": "web-routerRoutes.d.ts",
"mappings": ";;;;;;IAiBM;IACA;IACA;IAEE;IACA;IAGA",
"mappings": ";;;;;;IAiBM;IACA;IACA;IAEE;IACA;IAGA;IAGA;IAGA",
"names": [],
"sources": [
"../../../web/src/Routes.js",
Expand Down
8 changes: 8 additions & 0 deletions packages/internal/src/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,18 @@ export interface RWRouteManifestItem {
hasParams: boolean
relativeFilePath: string
redirect: { to: string; permanent: boolean } | null
isPrivate: boolean
unauthenticated: string | null
roles: string | string[] | null
// Probably want isNotFound here, so we can attach a separate 404 handler
}

export interface RouteSpec extends RWRouteManifestItem {
id: string
isNotFound: boolean
filePath: string | undefined
isPrivate: boolean
unauthenticated: string | null
relativeFilePath: string
}

Expand Down Expand Up @@ -111,6 +116,9 @@ export const getProjectRoutes = (): RouteSpec[] => {
redirect: route.redirect
? { to: route.redirect, permanent: false }
: null,
isPrivate: route.isPrivate,
unauthenticated: route.unauthenticated,
roles: route.roles,
}
})
}
4 changes: 2 additions & 2 deletions packages/project-config/src/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export interface PagesDependency {
/** the variable to which the import is assigned */
importName: string
/** @alias importName */
const: string
constName: string
/** absolute path without extension */
importPath: string
/** absolute path with extension */
Expand Down Expand Up @@ -346,7 +346,7 @@ export const processPagesDir = (
const importStatement = `const ${importName} = { name: '${importName}', loader: import('${importPath}') }`
return {
importName,
const: importName,
constName: importName,
importPath,
path: path.join(webPagesDir, pagePath),
importStatement,
Expand Down
4 changes: 2 additions & 2 deletions packages/structure/src/model/RWPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { RWProject } from './RWProject'

export class RWPage extends FileNode {
constructor(
public const_: string,
public constName: string,
public path: string,
public parent: RWProject,
) {
Expand All @@ -20,7 +20,7 @@ export class RWPage extends FileNode {
}
@lazy() get route() {
return this.parent.router.routes.find(
(r) => r.page_identifier_str === this.const_,
(r) => r.page_identifier_str === this.constName,
)
}
@lazy() get layoutName(): string | undefined {
Expand Down
4 changes: 3 additions & 1 deletion packages/structure/src/model/RWProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ export class RWProject extends BaseNode {
}
}
@lazy() get pages(): RWPage[] {
return this.processPagesDir.map((p) => new RWPage(p.const, p.path, this))
return this.processPagesDir.map(
(p) => new RWPage(p.constName, p.path, this),
)
}
@lazy() get router() {
return this.getRouter()
Expand Down
87 changes: 85 additions & 2 deletions packages/structure/src/model/RWRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,90 @@ export class RWRoute extends BaseNode {
?.getOpeningElement()
?.getTagNameNode()
?.getText()
return tagText === 'Private'
return tagText === 'Private' || tagText === 'PrivateSet'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to keep "Private"? I think it has to be "PrivateSet" now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say lets remove Private in a separate PR, and remove it from exports too!

}

@lazy() get unauthenticated() {
if (!this.isPrivate) {
return undefined
}

const a = this.jsxNode
.getParentIfKind(tsm.SyntaxKind.JsxElement)
?.getOpeningElement()
.getAttribute('unauthenticated')

if (!a) {
return undefined
}
if (tsm.Node.isJsxAttribute(a)) {
const init = a.getInitializer()
if (tsm.Node.isStringLiteral(init!)) {
return init.getLiteralValue()
}
}
return undefined
}

@lazy()
get roles() {
if (!this.isPrivate) {
return undefined
}

const a = this.jsxNode
.getParentIfKind(tsm.SyntaxKind.JsxElement)
?.getOpeningElement()
.getAttribute('roles')

if (!a) {
return undefined
}

if (tsm.Node.isJsxAttribute(a)) {
const init = a.getInitializer()

// Handle string literals
if (tsm.Node.isStringLiteral(init)) {
let literalValue = init.getLiteralValue()

// Check if the string looks like an array with single quotes
if (literalValue.startsWith('[') && literalValue.endsWith(']')) {
try {
// Unescape the string by replacing single quotes with double quotes
const correctedLiteralValue = literalValue.replace(/'/g, '"')
// Attempt to parse as JSON array
const parsedValue = JSON.parse(correctedLiteralValue)
if (Array.isArray(parsedValue)) {
return parsedValue
}
} catch (e) {
// If parsing fails, return undefined
return undefined
}
}

// If not an array, return the string value
return literalValue
}

// Handle JSX expressions with array literals
if (tsm.Node.isJsxExpression(init)) {
const expr = init.getExpression()
if (tsm.Node.isArrayLiteralExpression(expr)) {
return expr
.getElements()
.map((element) => {
if (tsm.Node.isStringLiteral(element)) {
return element.getLiteralValue()
}
return undefined
})
.filter((val) => val !== undefined)
}
}
}
return undefined
}

@lazy() get hasParameters(): boolean {
Expand Down Expand Up @@ -98,7 +181,7 @@ export class RWRoute extends BaseNode {
return undefined
}
return this.parent.parent.pages.find(
(p) => p.const_ === this.page_identifier_str,
(p) => p.constName === this.page_identifier_str,
)
}
/**
Expand Down
6 changes: 5 additions & 1 deletion packages/structure/src/model/RWRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ export class RWRouter extends FileNode {
.getDescendantsOfKind(tsm.SyntaxKind.JsxElement)
.filter((x) => {
const tagName = x.getOpeningElement().getTagNameNode().getText()
return tagName === 'Set' || tagName === 'Private'
return (
tagName === 'Set' ||
tagName === 'Private' ||
tagName === 'PrivateSet'
)
})

const prerenderSets = sets.filter((set) =>
Expand Down
105 changes: 104 additions & 1 deletion packages/structure/src/model/__tests__/model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,28 @@ describe.skip('env vars', () => {
})
})

describe('Redwood Page detection', () => {
it('detects pages', async () => {
const projectRoot = getFixtureDir('example-todo-main')
const project = new RWProject({ projectRoot, host: new DefaultHost() })
const routes = project.getRouter().routes
const pages = routes.map((r) => r.page).sort()
const pageConstants = pages.map((p) => p?.constName)
// Note: Pages can be duplicated if used by multiple routes, we use a Set
expect(pageConstants).toEqual([
'HomePage',
'TypeScriptPage',
'FooPage',
'BarPage',
'PrivatePage',
'PrivatePage',
'PrivatePage',
'NotFoundPage',
undefined,
])
})
})

describe('Redwood Route detection', () => {
it('detects routes with the prerender prop', async () => {
const projectRoot = getFixtureDir('example-todo-main')
Expand All @@ -123,7 +145,7 @@ describe('Redwood Route detection', () => {
// interested in
.map(({ name, path }) => ({ name, path }))

expect(prerenderRoutes.length).toBe(6)
expect(prerenderRoutes.length).toBe(8)
expect(prerenderRoutes).toContainEqual({ name: 'home', path: '/' })
expect(prerenderRoutes).toContainEqual({
name: 'typescriptPage',
Expand All @@ -140,6 +162,87 @@ describe('Redwood Route detection', () => {
path: '/private-page',
})
})
it('detects authenticated routes', async () => {
const projectRoot = getFixtureDir('example-todo-main')
const project = new RWProject({ projectRoot, host: new DefaultHost() })
const routes = project.getRouter().routes

const authenticatedRoutes = routes
.filter((r) => r.isPrivate)
.map(({ name, path, unauthenticated, roles }) => ({
name,
path,
unauthenticated,
roles,
}))

expect(authenticatedRoutes.length).toBe(3)
})

it('detects name and path for an authenticated route', async () => {
const projectRoot = getFixtureDir('example-todo-main')
const project = new RWProject({ projectRoot, host: new DefaultHost() })
const routes = project.getRouter().routes

const authenticatedRoutes = routes
.filter((r) => r.isPrivate)
.map(({ name, path, unauthenticated, roles }) => ({
name,
path,
unauthenticated,
roles,
}))

expect(authenticatedRoutes[1].name).toBe('privatePageAdmin')
expect(authenticatedRoutes[1].path).toBe('/private-page-admin')
expect(authenticatedRoutes[1].unauthenticated).toBe('home')
expect(authenticatedRoutes[1].roles).toBeTypeOf('string')
expect(authenticatedRoutes[1].roles).toContain('admin')
})

it('detects roles for an authenticated route when roles is a string of a single role', async () => {
const projectRoot = getFixtureDir('example-todo-main')
const project = new RWProject({ projectRoot, host: new DefaultHost() })
const routes = project.getRouter().routes

const authenticatedRoutes = routes
.filter((r) => r.isPrivate)
.map(({ name, path, unauthenticated, roles }) => ({
name,
path,
unauthenticated,
roles,
}))

expect(authenticatedRoutes[1].name).toBe('privatePageAdmin')
expect(authenticatedRoutes[1].path).toBe('/private-page-admin')
expect(authenticatedRoutes[1].unauthenticated).toBe('home')
expect(authenticatedRoutes[1].roles).toBeTypeOf('string')
expect(authenticatedRoutes[1].roles).toContain('admin')
})

it('detects roles for an authenticated route when roles is an array of a roles', async () => {
const projectRoot = getFixtureDir('example-todo-main')
const project = new RWProject({ projectRoot, host: new DefaultHost() })
const routes = project.getRouter().routes

const authenticatedRoutes = routes
.filter((r) => r.isPrivate)
.map(({ name, path, unauthenticated, roles }) => ({
name,
path,
unauthenticated,
roles,
}))

expect(authenticatedRoutes[2].name).toBe('privatePageAdminSuper')
expect(authenticatedRoutes[2].path).toBe('/private-page-admin-super')
expect(authenticatedRoutes[2].unauthenticated).toBe('home')
expect(authenticatedRoutes[2].roles).toBeInstanceOf(Array)
expect(authenticatedRoutes[2].roles).toContain('owner')
expect(authenticatedRoutes[2].roles).toContain('superuser')
expect(authenticatedRoutes[2].roles).not.toContain('member')
})
})

function getFixtureDir(
Expand Down
3 changes: 3 additions & 0 deletions packages/vite/src/buildRouteManifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ export async function buildRouteManifest() {
}
: null,
relativeFilePath: route.relativeFilePath,
isPrivate: route.isPrivate,
unauthenticated: route.unauthenticated,
roles: route.roles,
}

return acc
Expand Down
Loading
Loading