auth0 / nextjs-auth0

Next.js SDK for signing in with Auth0
MIT License
2.01k stars 381 forks source link

withPageAuthRequired on layout component #1643

Open Patrick-Ullrich opened 7 months ago

Patrick-Ullrich commented 7 months ago

Checklist

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

EthanML commented 7 months ago

+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).

perenstrom commented 6 months ago

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:~

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

rsslldnphy commented 3 months ago

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

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