javascript icon indicating copy to clipboard operation
javascript copied to clipboard

fix(clerk-sdk-node): Inherit verifyToken options from clerkClient

Open panteliselef opened this issue 1 year ago • 4 comments

Description

This PR allows clerkClient.verifyToken from "@clerk/clerk-sdk-node" to inherit the VerifyTokenOptions set when clerkClient is instantiated.

Added an additional error when both jwtKey and secretKey are missing.

fixes #3283

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:

panteliselef avatar May 01 '24 11:05 panteliselef

🦋 Changeset detected

Latest commit: 5782b0065b869569c0be3232ed61dcdf7d1b9bcc

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
@clerk/express Patch
@clerk/fastify Patch
@clerk/nextjs 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

changeset-bot[bot] avatar May 01 '24 11:05 changeset-bot[bot]

!snapshot

panteliselef avatar May 01 '24 11:05 panteliselef

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

Package Version
@clerk/backend 1.1.2-snapshot.v9a7208b
@clerk/chrome-extension 1.0.4-snapshot.v9a7208b
@clerk/clerk-js 5.2.1-snapshot.v9a7208b
@clerk/clerk-expo 1.0.4-snapshot.v9a7208b
@clerk/express 0.0.5-snapshot.v9a7208b
@clerk/fastify 1.0.4-snapshot.v9a7208b
gatsby-plugin-clerk 5.0.0-beta.45
@clerk/nextjs 5.0.5-snapshot.v9a7208b
@clerk/remix 4.0.4-snapshot.v9a7208b
@clerk/clerk-sdk-node 5.0.4-snapshot.v9a7208b

Tip: Use the snippet copy button below to quickly install the required packages. @clerk/backend

npm i @clerk/[email protected] --save-exact

@clerk/chrome-extension

npm i @clerk/[email protected] --save-exact

@clerk/clerk-js

npm i @clerk/[email protected] --save-exact

@clerk/clerk-expo

npm i @clerk/[email protected] --save-exact

@clerk/express

npm i @clerk/[email protected] --save-exact

@clerk/fastify

npm i @clerk/[email protected] --save-exact

gatsby-plugin-clerk

npm i [email protected] --save-exact

@clerk/nextjs

npm i @clerk/[email protected] --save-exact

@clerk/remix

npm i @clerk/[email protected] --save-exact

@clerk/clerk-sdk-node

npm i @clerk/[email protected] --save-exact

clerk-cookie avatar May 01 '24 11:05 clerk-cookie

@dimkl Regarding case 1, Completely dropping the 2nd params, wouldn't that be a breaking change ?

If we are going the extra mile here, let's still support the 2nd param, but update the signature so that 2nd param is optional. It is the least disruptive solution.

Your suggestions make sense, but I would skip them for this package and only apply them to the new express package

panteliselef avatar May 09 '24 06:05 panteliselef

☁️ I understand that the customer use case is valid and that we probably need to make a change to clerkClient.verifyToken(token, options) to support execution without passing options but i am not 100% sure that we should make it this way. I would expect to either keep it as is and inform the customers using it explicitly that it's just a re-export from the @clerk/backend and all the arguments required should be passed. or support the following:

import { clerkClient } from "@clerk/clerk-sdk-node";

// case #1: use options passed in clerkClient
clerkClient.verifyToken(token)

// case #2: options should be non-conflicting with the ones passed in clerkClient
// as it does not make sense to have a different jwtKey set in the clerkClient and a different one used in verifyToken
clerkClient.verifyToken(token, options)

If they need to pass explicitly the options i would advise to either create a different clerkClient or use the verifytoken from the @clerk/backend. I think we should go for ONLY the 1st case. We should check if the @clerk/express supports the same API and make the appropriate changes there. cc: @nikosdouvlis

Hi, I use this function to create auth middleware for the "graphql-yoga" server. All options are default and secret from env (which doesn't work now), so "case #1" fits better.

If I use different options that conflict with the current "clerkClient," I'll use "createClerkClient" to create a new one (that also doesn't work now)

shadoworion avatar May 10 '24 11:05 shadoworion