Introduce Forbidden and the UI Error abstraction

[panteliselef]

Background

Expanding from this PR. This PR is a refactor of the current notFound() utility and its dependencies. In order to make future additions easier this PR proposes the concept of a UI Error. UI Errors like notFound and forbidden provide an isomorphic utility that triggers (throws) the error, but it is handled differently based on context.

What?

Similar to the notFound() function and the notFound.js file, this PR follows the convention and introduces the forbidden() function along side the forbiddden.js file.

Why?

While the available notFound and redirect functions can get you a long way, there is only so much you can do with them.

Developers need a way to handle the authorization state of a user across the multiple "contexts" next has, by that I am referring to components, api routes, and server actions. For this case the 403 Forbidden status code can be used.

(We would like to support 401 errors as well)

How?

As a guide, I used the existing implementation of notFound and redirect.

• Handle 403 errors in components, api routes, server actions, and parallel routes.
• Handle missing forbidden files. In this case we display a default one.
• ForbiddenErrorBoundary and Forbidden have been introduced in order to catch and display the 403 error.

To verify that the changes are working the PR includes a minimal set of e2e tests.

How to use ?

Protect a page from a user who does not have the privilege to access it.

// app/team/subscription/page.tsx
import { forbidden } from "next/navigation";

export default function Page(){
  const user = auth() // grap user from headers()

  if(!user.isAdmin) {
    forbidden()
  }

  return <></>;
}

// app/team/subscription/forbidden.tsx
export default function Forbidden(){
  return (
    <>
      <p>Not an admin</p>
     <>
  );
}

Protect an API route from a user who does not have the privilege to access it.

import { forbidden } from "next/navigation";

export const GET = () => {
  const user = auth() // grap user from headers()

  if(!user.isAdmin) {
    forbidden() // -> Returns a 403 response with empty body
  }
  
  return new Response(JSON.strigify({ hasAccess: true }), {statusCode: 200})
};

What's next

• More e2e tests have been written but not pushed in order to make the review easier.
• See how the unauthorized helper will look like if this PR is accepted.
• I've added a few comments with questions. I kindly ask the reviewers to take a look at those as they might be important for implementing the feature correctly.

[ijjk]

Allow CI Workflow Run

• approve CI run for commit: d480602

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

[leerob]

Could we add some documentation here please?

We'll likely need updates in:

• https://nextjs.org/docs/app/building-your-application/routing/error-handling
• https://nextjs.org/docs/app/api-reference/file-conventions/forbidden (new?)

[panteliselef]

@leerob I'll start working on those.
Seems like https://nextjs.org/docs/app/building-your-application/routing/error-handling does not refer to not-found so probably i can skip that.

Instead i think we need a similar page for the forbidden() function

[gnoff]

So the approach of this PR is to generalize the not found and forbidden in an abstraction but this doesn't quite achieve what I was hoping we could/would. There is still a separate boundary for each of these functions when what we really want to achieve is a single boundary. We also ideally eventually would roll redirect boundary behavior into this component.

I went through the PR with @huozhi and he will end up doing more review and providing additional feedback. But one thing that this PR also needs before we can land is a more comprehensive test suite. The current one doesn't actually use forbidden.ts nor does it exercise the new methods in server actions and route handlers.

What I'd like to see is a removal of abstractions rather than the creation of new ones (ui-error-boundary and friends). But maybe before jumping into this work let's give @huozhi a chance to provide guidance or communicate a plan on what we will do internally before/after landing this

[gnoff]

The recent changes here go a long way to unifying the error handlers for notFound and forbidden but one goal I was hoping to see achieved was to not expand the number of boundaries we layer within each Segment in the App. While the implementation is now abstracted and shared we are still creating a new component in the hierarchy. If we add support for notAuthorized too we'd have to add another. Can we unify these two into a single boundary? Maybe even include the redirect boundary as well so we eliminate some component tree depth

[panteliselef]

Combining all boundaries into one should be possible. It's something that I experimented with, but dropped it. It definitely makes sense and I'll include it.

[panteliselef]

Hey @gnoff, thanks for reviewing, the single boundary makes sense, I'll make this change.

As the I mention in the description, there are more tests available here, I have just not included them in this PR in order to make the review easier, I will include those in this PR shortly. I will also make sure the server actions and route handlers are included.

What I'd like to see is a removal of abstractions rather than the creation of new ones (ui-error-boundary and friends). But maybe before jumping into this work let's give @huozhi a chance to provide guidance or communicate a plan on what we will do internally before/after landing this.

@huozhi please let me know how to do feel about the "UI error" concept. Should I drop it ? Should we refactor ?

[panteliselef]

@gnoff Regarding the introductions to the Public API with forbidden() and forbidden.jsx|tsx are we ok ?

[huozhi]

👋 Hey, thanks for the PR!
We've reviewed the forbidden() API implementation internally, and here are our thoughts:

We noticed some duplication of not-found logic when implementing the new forbidden() API. There's a significant potential to reuse underlaying error boundaries and error handling logic from notFound to manage the new HTTP errors like 403 Forbidden or 401 Unauthorized. We aim for a more lightweight approach that covers both server rendering and also client error boundaries side efficiently.

Currently, the PR encapsulates these errors as UI errors, creating a new layer similar to notFound errors. However, these could be merged into a general error boundary. Additionally, on the server side it's still missing handling for 403 HTTP responses.

Next.js have various scenarios and compositions need to be thoroughly tested, such as dynamic / static rendering, API routes, pages router, PPR, the custom forbidden error boundary forbidden.js . The tests from the PR primarily cover the default forbidden, but leaving a few critical scenarios. We assume it requires more refinement and iterations there.

Given these points, we believe it would beneficial if we could take over the work and and refine the implementation later to ensure these HTTP errors handling can be extended in the future effortlessly. I'll get another PR for this feature 🙏

We greatly appreciate your contribution and your efforts in pushing the standards forward!a

[panteliselef]

Hey @huozhi thanks for the reply. Letting Vercel take the lead on this one makes sense.

A single error boundary was introduced here.

I also never got to push [these](test: Add forbidden e2e test)test: Add forbidden e2e test and these tests, they do cover more cases, but not sure if it's every single one you are interested in.

Will you be interested in opening a GH discussion, and let people have a peek into the API you are thinking to ship ?