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

withPageAuthRequired on layout component #1643

Open
6 tasks done
Patrick-Ullrich opened this issue Jan 21, 2024 · 3 comments
Open
6 tasks done

withPageAuthRequired on layout component #1643

Patrick-Ullrich opened this issue Jan 21, 2024 · 3 comments

Comments

@Patrick-Ullrich
Copy link

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
Copy link

EthanML commented Jan 25, 2024

+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
Copy link

perenstrom commented Feb 24, 2024

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. 😄 #1678

@rsslldnphy
Copy link

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants