nextjs-auth0 icon indicating copy to clipboard operation
nextjs-auth0 copied to clipboard

withPageAuthRequired on layout component

Open Patrick-Ullrich opened this issue 1 year ago • 3 comments

Checklist

  • [X] The issue can be reproduced in the nextjs-auth0 sample app (or N/A).
  • [X] I have looked into the Readme, Examples, and FAQ and have not found a suitable solution or answer.
  • [X] I have looked into the API documentation and have not found a suitable solution or answer.
  • [X] I have searched the issues and have not found a suitable solution or answer.
  • [X] I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • [X] I agree to the terms within the Auth0 Code of Conduct.

Description

Related to some of the other App Router bugs open.

Instead of doing the withPageAuthRequired on each Page, I'd like to just put it on the layout.

e.g.:

export default withPageAuthRequired(async function BaseLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  const session = await getSession();

  return (
    <div>
      Hello {session?.user.name},
      <br />
      <a href="/api/auth/logout">Logout</a>
      <br />
      <a href="/api/auth/login">Login</a>
      
      <div>{children}</div>
    </div>
  );
});

This code seems to work as expected but will throw a typescript error: Screenshot 2024-01-21 at 17 30 31

Seems like only the typing would need to be enhanced.

It also be nice to be able to access the current route in the returnTo func which makes this use case even better:

export default withPageAuthRequired(async function BaseLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  ...

  return (
    <div>
       ...
    </div>
  );
}, {
  // Pass in pathname on top of dynamic pieces
  returnTo: ({ pathname } => pathname);
});

Reproduction

Use above code example.

Additional context

No response

nextjs-auth0 version

3.5.0

Next.js version

14.1

Node.js version

18.17.1

Patrick-Ullrich avatar Jan 21 '24 23:01 Patrick-Ullrich

+1, I also encountered this today and feel it would be convenient to be able to do this once in a layout rather than developers needing to add it on every page for many use cases (not to mention safer, since developers may forget and accidentally leave a page exposed).

EthanML avatar Jan 25 '24 13:01 EthanML

Edit: This broke npm run build. Looking into it more. Edit 2: This hack breaks typechecks when used in Pages, since they're not allowed to have children. So this seems like a bigger problem than just a small one-line type fix.

~I tried for ages writing a custom declaration file that would overwrite the built in type to fix this, but couldn't. I ended up using patch-package (https://www.npmjs.com/package/patch-package) to adjust the built in type instead. Works fine in runtime, it's just the type that's wrong.~

~How to do the same:~

  • ~Run npm install -D patch-package to install the patch package~
  • ~Add children?: React.ReactNode; to the type AppRouterPageRouteOpts in /node_modules/@auth0/nextjs-auth0/dist/helpers/with-page-auth-required.d.ts~
  • ~Run npx patch-package to detect the change and save as a patch file~
  • ~Add "postinstall": "patch-package" to the scripts property of package.json to automatically apply the patch when running npm install~

~I've also made PR with this change. Hopefully I didn't make something completely unreasonable. 😄 https://github.com/auth0/nextjs-auth0/pull/1678~

perenstrom avatar Feb 24 '24 19:02 perenstrom

FWIW this is what I've done, feels pretty bad but does seem to work:

export default withPageAuthRequired(
    Layout as AppRouterPageRoute,
) as React.FC;

rsslldnphy avatar May 11 '24 12:05 rsslldnphy