chore(express): Refactor requireAuth and clerkMiddleware middlewares

[wobsoriano]

Description

This PR refactors both clerkMiddleware and requireAuth Express middlewares to simplify DX and reduce confusion when choosing between middlewares:

1. clerkMiddleware

Removed option to pass a handler. It does not do anything at all, which can be replaced by a one-liner from userland:

// Before
app.use(clerkMiddleware((req, res, next) => {
  // req.auth here is undefined so it's not useful at all
}));

// After
app.use(clerkMiddleware());
app.use((req, res, next) => {
  // do stuff
})

2. requireAuth

Is now using the same logic as clerkMiddleware (use authenticateRequest and attach auth to request), except it will redirect to signInUrl if unauthenticated instead of calling next(new Error()). So you can have requireAuth middleware without clerkMiddleware:

// Before - sends error to error middleware
app.use(clerkMiddleware());
app.get('/protected', requireAuth, (req, res) => {
  res.send('This is a protected route');
});

app.use((err, req, res, next) => {
  if (err instanceof UnauthorizedError) {
    res.status(401).send('Unauthorized');
  } else {
    next(err);
  }
});


// After - redirects to signInUrl

// Apply centralized middleware
app.use(requireAuth());

// Apply middleware to a specific route
app.get('/protected', requireAuth({ signInUrl: '/sign-in }), (req, res) => {
  res.send('This is a protected route');
});

Other removed exports:

• UnauthorizedError (not used anywhere)
• ForbiddenError (not used anywhere)

Documentation for this one is still being worked on, we didn't announce the SDK formally, so I think minor update is good. The documentation will have migration guide from @clerk/clerk-sdk-node to @clerk/express. anyway.

Resolves ECO-201

Checklist

• npm test runs as expected.
• npm run build runs as expected.
• (If applicable) JSDoc comments have been added or updated for any package exports
• (If applicable) Documentation has been updated

Type of change

• 🐛 Bug fix
• 🌟 New feature
• 🔨 Breaking change
• 📖 Refactoring / dependency upgrade / documentation
• other:

[changeset-bot]

🦋 Changeset detected

Latest commit: a7eb3eb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@clerk/express	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[wobsoriano]

Removed this tests as requireAuth does not rely on clerkMiddleware anymore

[wobsoriano]

Removed as we removed option to pass a handler in clerkMiddleware

[jescalan]

This is beautiful I love it. Such a quick turnaround!

[jescalan]

Have we covered for the scenario where someone uses both middlewares? A little conditional at the top of this helper to just call next if auth is already on the request I think would take care of that. May be worth adding a test for this as well.

[wobsoriano]

Nope, but added just now and a test too!

[jescalan]

Do we need to error if there are both? Could we just add a conditional check for req.auth at the top of the function instead?

[wobsoriano]

Ah I get you now, we want to allow usage of both, but check for req.auth first then call next() if exists so we dont have to run the same logic etc 👍🏼 Perfect for scenario like

app.use(clerkMiddleware());

app.get('/protected', requireAuth(), (req, res) => {
  res.send('This is a protected route');
});

Updated

[LauraBeatris]

I love those changes, the DX is much better now ❤️

[jescalan]

This looks great to me! Nice work Rob 👏