Skip to content

feat(backend,clerk-sdk-node): Simplify the authenticateRequest signature#1329

Merged
nikosdouvlis merged 1 commit into
mainfrom
stefanos/js-455-improve-the-dx-of-the-signature-of-authenticaterequest-for
Jun 19, 2023
Merged

feat(backend,clerk-sdk-node): Simplify the authenticateRequest signature#1329
nikosdouvlis merged 1 commit into
mainfrom
stefanos/js-455-improve-the-dx-of-the-signature-of-authenticaterequest-for

Conversation

@anagstef

@anagstef anagstef commented Jun 9, 2023

Copy link
Copy Markdown
Contributor

Type of change

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

Packages affected

  • @clerk/clerk-js
  • @clerk/clerk-react
  • @clerk/nextjs
  • @clerk/remix
  • @clerk/types
  • @clerk/themes
  • @clerk/localizations
  • @clerk/clerk-expo
  • @clerk/backend
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/fastify
  • @clerk/chrome-extension
  • gatsby-plugin-clerk
  • build/tooling/chore

Description

  • npm test runs as expected.
  • npm run build runs as expected.

This PR simplifies the signature of authenticateRequest for better DX on both @clerk/backend and @clerk/clerk-sdk-node packages:

  • One pair of legacy or new instance keys are required now and not all 4 of them
  • @clerk/backend now can handle the "Bearer " prefix in Authorization header for better DX
  • host parameter is now optional in @clerk/backend

Example of using authenticateRequest

Before:

import clerkClient from '@clerk/clerk-sdk-node';

// `Bearer asdgfjdkhgmsfngl`
const authHeader = request.headers.get('authorization') || '';
const authToken = authHeader.split(` `)[1];

const res = await clerkClient.authenticateRequest({
  headerToken: authToken,
  publishableKey: process.env.CLERK_PUB_KEY,
  secretKey: process.env.CLERK_SECRET_KEY,
  apiKey: ``,
  frontendApi: ``,
  host: ``,
});

...

After this PR:

import clerkClient from '@clerk/clerk-sdk-node';

const res = await clerkClient.authenticateRequest({
  headerToken: request.headers.get('authorization'),
  publishableKey: process.env.CLERK_PUB_KEY,
  secretKey: process.env.CLERK_SECRET_KEY
});

...

@anagstef
anagstef requested a review from dimkl June 9, 2023 00:08
@anagstef
anagstef requested a review from SokratisVidros as a code owner June 9, 2023 00:08
@anagstef anagstef self-assigned this Jun 9, 2023
@anagstef
anagstef requested a review from nikosdouvlis as a code owner June 9, 2023 00:08
@changeset-bot

changeset-bot Bot commented Jun 9, 2023

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c2f124f

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

This PR includes changesets to release 6 packages
Name Type
@clerk/clerk-sdk-node Patch
@clerk/backend Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/fastify Patch
@clerk/remix Patch

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

@jit-ci jit-ci Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

@anagstef
anagstef force-pushed the stefanos/js-455-improve-the-dx-of-the-signature-of-authenticaterequest-for branch 2 times, most recently from 6da8e5e to af9b738 Compare June 9, 2023 12:14
@nikosdouvlis

Copy link
Copy Markdown
Member

!snapshot

@clerk-cookie

Copy link
Copy Markdown
Collaborator

Hey @nikosdouvlis - the snapshot version command generated the following package versions:

Package Version
@clerk/backend 0.22.0-snap.af9b738
@clerk/chrome-extension 0.3.8-snap.af9b738
@clerk/clerk-js 4.45.1-snap.af9b738
eslint-config-custom 0.3.0
@clerk/clerk-expo 0.17.8-snap.af9b738
@clerk/fastify 0.4.6-snap.af9b738
gatsby-plugin-clerk 4.3.7-snap.af9b738
@clerk/localizations 1.17.1-snap.af9b738
@clerk/nextjs 4.20.0-snap.af9b738
@clerk/clerk-react 4.19.0-snap.af9b738
@clerk/remix 2.6.6-snap.af9b738
@clerk/clerk-sdk-node 4.10.4-snap.af9b738
@clerk/shared 0.18.0-snap.af9b738
@clerk/themes 1.7.5
@clerk/types 3.41.1-snap.af9b738

Tip: use the snippet copy button below to quickly install the required packages.

# @clerk/backend
npm i @clerk/backend@0.22.0-snap.af9b738
# @clerk/chrome-extension
npm i @clerk/chrome-extension@0.3.8-snap.af9b738
# @clerk/clerk-js
npm i @clerk/clerk-js@4.45.1-snap.af9b738
# eslint-config-custom
npm i eslint-config-custom@0.3.0
# @clerk/clerk-expo
npm i @clerk/clerk-expo@0.17.8-snap.af9b738
# @clerk/fastify
npm i @clerk/fastify@0.4.6-snap.af9b738
# gatsby-plugin-clerk
npm i gatsby-plugin-clerk@4.3.7-snap.af9b738
# @clerk/localizations
npm i @clerk/localizations@1.17.1-snap.af9b738
# @clerk/nextjs
npm i @clerk/nextjs@4.20.0-snap.af9b738
# @clerk/clerk-react
npm i @clerk/clerk-react@4.19.0-snap.af9b738
# @clerk/remix
npm i @clerk/remix@2.6.6-snap.af9b738
# @clerk/clerk-sdk-node
npm i @clerk/clerk-sdk-node@4.10.4-snap.af9b738
# @clerk/shared
npm i @clerk/shared@0.18.0-snap.af9b738
# @clerk/themes
npm i @clerk/themes@1.7.5
# @clerk/types
npm i @clerk/types@3.41.1-snap.af9b738

Comment thread packages/sdk-node/src/authenticateRequest.ts
Comment thread packages/backend/src/util/request.ts Outdated

@nikosdouvlis nikosdouvlis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @anagstef :)

Comment thread .changeset/pink-carpets-matter.md Outdated
Comment thread packages/backend/src/index.ts
Comment on lines +56 to +63
publishableKey: string;
/**
* @deprecated Use `publishableKey` instead.
*/
frontendApi: string;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can we have both?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have wayy more context that me here so please ignore if it doesn't make sense, however, the usual way to write this would be:

type ExclusiveAOrB = {
 a: string; 
 b?: never; 
} | {
 a?: never;
 b: string;
}

(notice the ?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can we have both?

@nikosdouvlis Isn't this a breaking change (on the type-level) of anyone passing both?

Comment thread packages/sdk-node/src/authenticateRequest.ts
Comment thread packages/sdk-node/src/authenticateRequest.ts
Comment thread packages/sdk-node/src/types.ts
@anagstef
anagstef force-pushed the stefanos/js-455-improve-the-dx-of-the-signature-of-authenticaterequest-for branch from af9b738 to 680f728 Compare June 19, 2023 11:46
- One pair of legacy or new instance keys are required  instead of all 4 of them in `authenticateRequest`
- `@clerk/backend` now can handle the `Bearer` prefix in Authorization header for better DX
- `host` parameter is now optional in `@clerk/backend`
@anagstef
anagstef force-pushed the stefanos/js-455-improve-the-dx-of-the-signature-of-authenticaterequest-for branch from 680f728 to c2f124f Compare June 19, 2023 15:40
@nikosdouvlis
nikosdouvlis merged commit de2347f into main Jun 19, 2023
@nikosdouvlis
nikosdouvlis deleted the stefanos/js-455-improve-the-dx-of-the-signature-of-authenticaterequest-for branch June 19, 2023 16:48
@clerk-cookie clerk-cookie mentioned this pull request Jun 19, 2023
@clerk-cookie

Copy link
Copy Markdown
Collaborator

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@clerk clerk locked as resolved and limited conversation to collaborators Jun 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants