chore(backend): Limit exposed token verification related errors by tmilewski · Pull Request #2189 · clerk/javascript

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski · 2023-11-22T17:38:05Z

Description

Removes both TokenVerificationErrorAction & TokenVerificationErrorCode from @clerk/backend exports as both are internal.

It also happens to support @flawnn's work/use-case in #1980, once we backport to release/v4.

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

Packages affected

Co-authored-by: flawnn <fl4wn@protonmail.com>

changeset-bot · 2023-11-22T17:38:09Z

🦋 Changeset detected

Latest commit: 5410436

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	Major
@clerk/backend	Major
@clerk/nextjs	Major
@clerk/remix	Major
gatsby-plugin-clerk	Patch
@clerk/fastify	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

dimkl

❓ How is dropping those 2 exports going to resolve the use case of user of catching and handling those errors? I think i got confused. 😓

tmilewski · 2023-11-22T20:14:30Z

@dimkl It doesn't. A comment in the other PR suggested we wanted to keep some of these exports internal. This PR aims to narrow the scope of the exports on main. When backporting this to release/v4, it will enable access only to these narrowed exports which aren't currently exported at all.

dimkl · 2023-11-23T07:58:36Z

I would suggest we close this PR because in the context of the Public APIs RFC we are going to change the exports of the @clerk/backend and probably export a @clerk/backend/errors subpath with the errors that we want to expose.
Also, I would suggest we keep those errors exported (from the /errors subpath) since the customer use case seems valid.

nikosdouvlis · 2023-11-23T12:14:30Z

 export { AuthStatus } from './authStatus';
 export type { RequestState } from './authStatus';
-export * from './errors';
+export { TokenVerificationError, TokenVerificationErrorReason } from './errors';


@tmilewski As the discussion about the error-related exports for v5 is still ongoing, I think we need to do the following:
export the requested error from release/v4
remove export * from './errors' from here
revisit this then we make a decision

Co-authored-by: flawnn <fl4wn@protonmail.com>

chore(backend): Limit exposed token verification related errors
5410436
Co-authored-by: flawnn <fl4wn@protonmail.com>

tmilewski self-assigned this Nov 22, 2023

clerk-cookie added nextjs backend sdk-node labels Nov 22, 2023

dimkl reviewed Nov 22, 2023
View reviewed changes

nikosdouvlis requested changes Nov 23, 2023
View reviewed changes

nikosdouvlis mentioned this pull request Nov 23, 2023
feat(backend): Also export token-related errors #1980
Closed
22 tasks

nikosdouvlis approved these changes Dec 7, 2023
View reviewed changes

tmilewski added this pull request to the merge queue Dec 7, 2023

Merged via the queue into main with commit e1f7eaeDec 7, 2023

tmilewski deleted the chore/limit-token-error-exports branch December 7, 2023 15:10

clerk-cookie mentioned this pull request Dec 7, 2023
Version Packages (alpha-v5) #2276
Merged

octoper pushed a commit that referenced this pull request Dec 13, 2023
chore(backend): Limit exposed token verification related errors (#2189)
0d446f4
Co-authored-by: flawnn <fl4wn@protonmail.com>

coderabbitaiBot mentioned this pull request May 21, 2026
Remote access via Cloudflare relay + polished CLI auth UX TraderSamwise/aimux#4
Merged
4 tasks