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

DRAFT: feat: Enforce authentication when rendering RSC page with the RSC Handler #10647

Closed
wants to merge 18 commits into from

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented May 18, 2024

This PR further the goal to enforce authentication and role permissions when rendering RSC client side.

PR changes:

  • in rsc client, pass the route in the rw-rsc route
    // http://localhost:8910/rw-rsc//AboutPage?props=%7B%7D
  • in rsc handler, extract routePath, lookup route form manifest and use that to enforce auth
  • isRouteAllowed is mocked for moment

-- OLD PR

  • when auto loading the routes, construct the page import renders (from dist/ssr or from rsc) by passing the rscId and the roles
  • builds from a map of pages and the route built from project routes instead of pages directory
  • updates the render function to accept a set of route info
export function renderFromRscServer<TProps>(
  rscId: string,
  routes: { name: string; path: string }[],
) {
  // @TODO: Enforce auth and permissions on the client side
  // 1. Load the routes from the route manifest
  // 2. look up by the route name, the isPrivate, roles and unauthenticated redirect
  // 3. check if the user is authenticated
  // 4. check if the user has the required roles
  // 5. if the user is not authenticated, redirect to the unauthenticated redirect or error?

May not need the path, but added in the case if need some url matching later server-side.

Also, requires: #10572 as that PR adds the auth info to the routes manifest which will be loaded by the client to check auth

Notes:

  • the packages/vite/src/plugins/vite-plugin-rsc-routes-auto-loader.ts test fails as was mocked from pages

@dthyresson dthyresson added this to the RSC milestone May 18, 2024
@dthyresson dthyresson added the release:feature This PR introduces a new feature label May 18, 2024
@dthyresson
Copy link
Contributor Author

@dac09 This is the output of the rsc-routes-auto-loader.

import { renderFromRscServer } from "@redwoodjs/vite/client";
  const UserExampleUserExamplesPage = renderFromRscServer("UserExampleUserExamplesPage", {
  routes: [{
  name: "userExamples",
  path: "/user-examples"
  }]
  });
  const UserExampleUserExamplePage = renderFromRscServer("UserExampleUserExamplePage", {
  routes: [{
  name: "userExample",
  path: "/user-examples/{id:Int}"
  }]
  });
  const UserExampleEditUserExamplePage = renderFromRscServer("UserExampleEditUserExamplePage", {
  routes: [{
  name: "editUserExample",
  path: "/user-examples/{id:Int}/edit"
  }]
  });
  const UserExampleNewUserExamplePage = renderFromRscServer("UserExampleNewUserExamplePage", {
  routes: [{
  name: "newUserExample",
  path: "/user-examples/new"
  }]
  });
  const EmptyUserEmptyUsersPage = renderFromRscServer("EmptyUserEmptyUsersPage", {
  routes: [{
  name: "emptyUsers",
  path: "/empty-users"
  }]
  });
  const EmptyUserEmptyUserPage = renderFromRscServer("EmptyUserEmptyUserPage", {
  routes: [{
  name: "emptyUser",
  path: "/empty-users/{id:Int}"
  }]
  });
  const EmptyUserEditEmptyUserPage = renderFromRscServer("EmptyUserEditEmptyUserPage", {
  routes: [{
  name: "editEmptyUser",
  path: "/empty-users/{id:Int}/edit"
  }]
  });
  const EmptyUserNewEmptyUserPage = renderFromRscServer("EmptyUserNewEmptyUserPage", {
  routes: [{
  name: "newEmptyUser",
  path: "/empty-users/new"
  }]
  });
  const MultiCellPage = renderFromRscServer("MultiCellPage", {
  routes: [{
  name: "multiCell",
  path: "/multi-cell"
  }]
  });
  const AboutPage = renderFromRscServer("AboutPage", {
  routes: [{
  name: "about",
  path: "/about"
  }, {
  name: "info",
  path: "/info"
  }]
  });
  const HomePage = renderFromRscServer("HomePage", {
  routes: [{
  name: "home",
  path: "/"
  }]
  });
  import { jsx, jsxs } from "react/jsx-runtime";
  import { Router, Route, Set } from "@redwoodjs/router";
  import NavigationLayout from "./layouts/NavigationLayout/NavigationLayout";
  import ScaffoldLayout from "./layouts/ScaffoldLayout/ScaffoldLayout";
  import NotFoundPage from "./pages/NotFoundPage/NotFoundPage";
  const Routes = () => {
  return /* @__PURE__ */jsxs(Router, {
  children: [/* @__PURE__ */jsxs(Set, {
  wrap: NavigationLayout,
  children: [/* @__PURE__ */jsx(Route, {
  path: "/",
  page: HomePage,
  name: "home"
  }), /* @__PURE__ */jsx(Route, {
  path: "/about",
  page: AboutPage,
  name: "about"
  }), /* @__PURE__ */jsx(Route, {
  path: "/info",
  page: AboutPage,
  name: "info"
  }), /* @__PURE__ */jsx(Route, {
  path: "/multi-cell",
  page: MultiCellPage,
  name: "multiCell"
  }), /* @__PURE__ */jsxs(Set, {
  wrap: ScaffoldLayout,
  title: "EmptyUsers",
  titleTo: "emptyUsers",
  buttonLabel: "New EmptyUser",
  buttonTo: "newEmptyUser",
  children: [/* @__PURE__ */jsx(Route, {
  path: "/empty-users/new",
  page: EmptyUserNewEmptyUserPage,
  name: "newEmptyUser"
  }), /* @__PURE__ */jsx(Route, {
  path: "/empty-users/{id:Int}/edit",
  page: EmptyUserEditEmptyUserPage,
  name: "editEmptyUser"
  }), /* @__PURE__ */jsx(Route, {
  path: "/empty-users/{id:Int}",
  page: EmptyUserEmptyUserPage,
  name: "emptyUser"
  }), /* @__PURE__ */jsx(Route, {
  path: "/empty-users",
  page: EmptyUserEmptyUsersPage,
  name: "emptyUsers"
  })]
  }), /* @__PURE__ */jsxs(Set, {
  wrap: ScaffoldLayout,
  title: "UserExamples",
  titleTo: "userExamples",
  buttonLabel: "New UserExample",
  buttonTo: "newUserExample",
  children: [/* @__PURE__ */jsx(Route, {
  path: "/user-examples/new",
  page: UserExampleNewUserExamplePage,
  name: "newUserExample"
  }), /* @__PURE__ */jsx(Route, {
  path: "/user-examples/{id:Int}/edit",
  page: UserExampleEditUserExamplePage,
  name: "editUserExample"
  }), /* @__PURE__ */jsx(Route, {
  path: "/user-examples/{id:Int}",
  page: UserExampleUserExamplePage,
  name: "userExample"
  }), /* @__PURE__ */jsx(Route, {
  path: "/user-examples",
  page: UserExampleUserExamplesPage,
  name: "userExamples"
  })]
  })]
  }), /* @__PURE__ */jsx(Route, {
  notfound: true,
  page: NotFoundPage
  })]
  });
  };
  export default Routes;

Can see that:

  const AboutPage = renderFromRscServer("AboutPage", {
  routes: [{
  name: "about",
  path: "/about"
  }, {
  name: "info",
  path: "/info"
  }]
  });

can cal renderFromRscServer with the component (rscId) and the route info. But again we find ourselves in where the entry maps to two route potentially.

Because the page component is used later:

, /* @__PURE__ */jsx(Route, {
  path: "/info",
  page: AboutPage,
  name: "info"

and

, /* @__PURE__ */jsx(Route, {
  path: "/about",
  page: AboutPage,
  name: "about"
  }), /* @__PURE_

I cannot easily make two imports -- one for about with the about route and one for the same page and the info route.

The Route component needs the result of the renderFromRscServer but renderFromRscServer needs two possible calls.

It will be easier to explain on a call where I think we still have a hole in the logic.

I have some ideas for concessions, but better to chat.

@dthyresson
Copy link
Contributor Author

Also @dac09 when you look at how the component renders:

export function renderFromRscServer<TProps>(
  rscId: string,
  routes: { name: string; path: string }[],
) {
  console.log(
    '>>>> serve rscId (renderFromRscServer)',
    rscId,
    'routes',
    routes,
    'url',
    window.location.pathname,
  )

  if (typeof window === 'undefined') {
    throw new Error(
      'renderFromRscServer should only be used in a real browser ' +
        'environment. Did you mean to use renderFromDist in clientSsr.ts ' +
        'instead?',
    )
  }

  // @TODO: Enforce auth and permissions on the client side
  // 1. Load the routes from the route manifest
  // 2. look up by the route name, the isPrivate, roles and unauthenticated redirect
  // 3. check if the user is authenticated
  // 4. check if the user has the required roles
  // 5. if the user is not authenticated, redirect to the unauthenticated redirect or error?

  const cachedFetchRSC = cache(fetchRSC) <---

How is caching done if the request is to render an authenticated component? If cannot cache the render of something will info for another auth'd request.

Does cache consider the auth state? Does it never cache if the component is auth'd.

@dthyresson
Copy link
Contributor Author

dthyresson commented May 20, 2024

Will rely on #10572.

@dac09 After chatting with @Josh-Walker-GM we realized that instead of passing the rscId and route to toe rendered (which could be spoofed), the client can reform the request url to include the route:

  // http://localhost:8910/rw-rsc/<routePath>/AboutPage?props=%7B%7D

The we can extract the route path, lookup from route manifest, and enforce auth rule in the rsc handler.

May be able to also do on client or even in some middleware.

Also - maybe we don't even have a base path /rw-rsc and just rely on the header to know if a rsc render request.

@dac09 dac09 changed the title DRAFT: feat: Enforce authentication when rendering RSC client side DRAFT: feat: Enforce authentication when rendering RSC page with the RSC Handler May 22, 2024
// Getting a warning on GitHub about this
// https://github.com/redwoodjs/redwood/security/code-scanning/211
// Handle according to TODO below
res.end(String(err))

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
.
@dthyresson
Copy link
Contributor Author

No longer need this PR per @dac09 and @Tobbe since logic to enforce auth will be in client and server router.

@dthyresson dthyresson closed this May 23, 2024
@Josh-Walker-GM Josh-Walker-GM removed this from the RSC milestone Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants