Drop interstitial by nikosdouvlis · Pull Request #2304 · clerk/javascript

nikosdouvlis · 2023-12-11T14:49:52Z

Description

❗ This needs to be merged into #2300 or after #2300 is merged

Drop interstitial from clerk/backend
Drop interstitial from clerk/shared
Drop interstitial from clerk/types
Drop interstitial from clerk/clerk-js (1 pending item)
Drop interstitial from clerk/nextjs
fix build for clerk/remix
fix build for gatsby-plugin-clerk
fix build for clerk/fastify
fix build for clerk/clerk-sdk-node

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

changeset-bot · 2023-12-11T14:49:56Z

⚠️ No Changeset found

Latest commit: 6651411

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

brkalow · 2023-12-11T16:38:04Z

+    } as AuthenticateRequestOptions;
+    const requestState = await clerkClient.authenticateRequest(req, authenticateRequestOptions);
+
+    if (requestState.status === AuthStatus.Handshake) {


@nikosdouvlis it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example). I think we need to keep the check to only consider the presence of a Location header.

Got it, makes sense.
I think we might want to introduce a new status for it or at least think of a different way to transfer this knowledge into the code as it's hard to follow the logic here otherwise

Check updated

brkalow

🤩 so much red 🔪

brkalow · 2023-12-12T03:40:58Z

+      // TODO @nikos
+      // if (!this.#options.isInterstitial) {
+      //   this.#clearJustSynced();
+      // }


I think we can do this:

Suggested change

// TODO @nikos

// if (!this.#options.isInterstitial) {

// this.#clearJustSynced();

// }

this.#clearJustSynced();

brkalow · 2023-12-12T03:42:49Z

-    // triggering a handshake redirect
    if (locationHeader) {
-      return new Response(null, { status: 307, headers: requestStateHeaders });
+      // triggering a handshake redirect


Okay so I realize that I misspoke earlier...after looking at the logic, we only redirect when the status is handshake, so the check you had initially added is the right approach 👍

dimkl

😮

As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example)

* feat(backend): Remove interstitial endpoints * feat(backend,types): Remove local interstitial script * feat(types): Clean retheme types * feat(backend): Remove interstitial and interstitial rules * feat(clerk-js): Remove interstitial from clerk-js * feat(nextjs): Remove interstitial from authMiddleware * feat(fastify): Remove interstitial * feat(gatsby-plugin-clerk): Remove interstitial * feat(remix): Remove interstitial * feat(clerk-sdk-node): Remove interstitial * fix(nextjs): Always respect redirect header if found As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example) * chore(repo): Fix sdk tests * fix(clerk-js): Fix tests related to db-jwt * fix(clerk-js): Keep hasJustSynced check

@ts-expect-error

* feat(backend): Try the new Client Handshake mechanism * feat(backend): Update authenticateRequest handler to support multi-domain handshake * feat(repo): Introduce tests for client handshake (#2265) * Test suite start * feat(backend,nextjs,utils): Fix jest * first test * Fix bug in jwks cache for multiple runtime keys * Add all the tests, including many failing * Add all the tests, including many failing * fix(shared): Correctly construct proxy URL --------- Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Bryce Kalow <bryce@clerk.dev> * chore(backend): Refactor authenticateRequest to clarify logic * fix(backend): Fix options passing to authenticateRequest * feat(backend): Add sec-fetch-dest check for satellite sync, adjust tests to support additional URLs * feat(backend): Account for clock skew in dev, but still log error * feat(backend): Refactor backend tests to account for recent refactoring to authenticateRequest * feat(backend): Treat handshake payload as a signed jwt * fix(backend): Add tests and adjust logic to ensure existing tests pass * chore(backend): Refactor tests to conform to new method signature * chore(repo): Add changeset * feat(*): Drop interstitial (#2304) * feat(backend): Remove interstitial endpoints * feat(backend,types): Remove local interstitial script * feat(types): Clean retheme types * feat(backend): Remove interstitial and interstitial rules * feat(clerk-js): Remove interstitial from clerk-js * feat(nextjs): Remove interstitial from authMiddleware * feat(fastify): Remove interstitial * feat(gatsby-plugin-clerk): Remove interstitial * feat(remix): Remove interstitial * feat(clerk-sdk-node): Remove interstitial * fix(nextjs): Always respect redirect header if found As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example) * chore(repo): Fix sdk tests * fix(clerk-js): Fix tests related to db-jwt * fix(clerk-js): Keep hasJustSynced check * chore(*): Fix linter * chore(backend): Remove unused AuthErrorReason properties, destructure from authenticateContext * chore(clerk-js): Remove unused @ts-expect-error directive * fix: Address build issues in sdk-node * fix(remix): Correct Remix build issues * chore(repo): Apply linting fixes --------- Co-authored-by: Sokratis Vidros <sokratis.vidros@gmail.com> Co-authored-by: Colin Sidoti <51144033+colinclerk@users.noreply.github.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Colin Sidoti <colin@clerk.dev>

@ts-expect-error

…#2331) * feat(backend): Try the new Client Handshake mechanism * feat(backend): Update authenticateRequest handler to support multi-domain handshake * feat(repo): Introduce tests for client handshake (#2265) * Test suite start * feat(backend,nextjs,utils): Fix jest * first test * Fix bug in jwks cache for multiple runtime keys * Add all the tests, including many failing * Add all the tests, including many failing * fix(shared): Correctly construct proxy URL --------- Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Bryce Kalow <bryce@clerk.dev> * chore(backend): Refactor authenticateRequest to clarify logic * fix(backend): Fix options passing to authenticateRequest * feat(backend): Add sec-fetch-dest check for satellite sync, adjust tests to support additional URLs * feat(backend): Account for clock skew in dev, but still log error * feat(backend): Refactor backend tests to account for recent refactoring to authenticateRequest * feat(backend): Treat handshake payload as a signed jwt * fix(backend): Add tests and adjust logic to ensure existing tests pass * chore(backend): Refactor tests to conform to new method signature * chore(repo): Add changeset * feat(*): Drop interstitial (#2304) * feat(backend): Remove interstitial endpoints * feat(backend,types): Remove local interstitial script * feat(types): Clean retheme types * feat(backend): Remove interstitial and interstitial rules * feat(clerk-js): Remove interstitial from clerk-js * feat(nextjs): Remove interstitial from authMiddleware * feat(fastify): Remove interstitial * feat(gatsby-plugin-clerk): Remove interstitial * feat(remix): Remove interstitial * feat(clerk-sdk-node): Remove interstitial * fix(nextjs): Always respect redirect header if found As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example) * chore(repo): Fix sdk tests * fix(clerk-js): Fix tests related to db-jwt * fix(clerk-js): Keep hasJustSynced check * chore(*): Fix linter * chore(backend): Remove unused AuthErrorReason properties, destructure from authenticateContext * chore(clerk-js): Remove unused @ts-expect-error directive * feat(repo): Migrate handshake tests to e2e test suite * fix(nextjs): Ensure we respect location header from authenticateRequest() regardless of auth status * Create bright-ways-provide.md * chore(clerk-sdk-node): Properly export createClerkClient * chore(clerk-sdk-node): Properly export createClerkClient * fix(nextjs): Fix undefined headers in tests --------- Co-authored-by: Sokratis Vidros <sokratis.vidros@gmail.com> Co-authored-by: Colin Sidoti <51144033+colinclerk@users.noreply.github.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Colin Sidoti <colin@clerk.dev>

@ts-expect-error

* feat(backend): Try the new Client Handshake mechanism * feat(backend): Update authenticateRequest handler to support multi-domain handshake * feat(repo): Introduce tests for client handshake (#2265) * Test suite start * feat(backend,nextjs,utils): Fix jest * first test * Fix bug in jwks cache for multiple runtime keys * Add all the tests, including many failing * Add all the tests, including many failing * fix(shared): Correctly construct proxy URL --------- Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Bryce Kalow <bryce@clerk.dev> * chore(backend): Refactor authenticateRequest to clarify logic * fix(backend): Fix options passing to authenticateRequest * feat(backend): Add sec-fetch-dest check for satellite sync, adjust tests to support additional URLs * feat(backend): Account for clock skew in dev, but still log error * feat(backend): Refactor backend tests to account for recent refactoring to authenticateRequest * feat(backend): Treat handshake payload as a signed jwt * fix(backend): Add tests and adjust logic to ensure existing tests pass * chore(backend): Refactor tests to conform to new method signature * chore(repo): Add changeset * feat(*): Drop interstitial (#2304) * feat(backend): Remove interstitial endpoints * feat(backend,types): Remove local interstitial script * feat(types): Clean retheme types * feat(backend): Remove interstitial and interstitial rules * feat(clerk-js): Remove interstitial from clerk-js * feat(nextjs): Remove interstitial from authMiddleware * feat(fastify): Remove interstitial * feat(gatsby-plugin-clerk): Remove interstitial * feat(remix): Remove interstitial * feat(clerk-sdk-node): Remove interstitial * fix(nextjs): Always respect redirect header if found As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example) * chore(repo): Fix sdk tests * fix(clerk-js): Fix tests related to db-jwt * fix(clerk-js): Keep hasJustSynced check * chore(*): Fix linter * chore(backend): Remove unused AuthErrorReason properties, destructure from authenticateContext * chore(clerk-js): Remove unused @ts-expect-error directive * fix: Address build issues in sdk-node * fix(remix): Correct Remix build issues * chore(repo): Apply linting fixes --------- Co-authored-by: Sokratis Vidros <sokratis.vidros@gmail.com> Co-authored-by: Colin Sidoti <51144033+colinclerk@users.noreply.github.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Colin Sidoti <colin@clerk.dev>

@ts-expect-error

…#2331) * feat(backend): Try the new Client Handshake mechanism * feat(backend): Update authenticateRequest handler to support multi-domain handshake * feat(repo): Introduce tests for client handshake (#2265) * Test suite start * feat(backend,nextjs,utils): Fix jest * first test * Fix bug in jwks cache for multiple runtime keys * Add all the tests, including many failing * Add all the tests, including many failing * fix(shared): Correctly construct proxy URL --------- Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Bryce Kalow <bryce@clerk.dev> * chore(backend): Refactor authenticateRequest to clarify logic * fix(backend): Fix options passing to authenticateRequest * feat(backend): Add sec-fetch-dest check for satellite sync, adjust tests to support additional URLs * feat(backend): Account for clock skew in dev, but still log error * feat(backend): Refactor backend tests to account for recent refactoring to authenticateRequest * feat(backend): Treat handshake payload as a signed jwt * fix(backend): Add tests and adjust logic to ensure existing tests pass * chore(backend): Refactor tests to conform to new method signature * chore(repo): Add changeset * feat(*): Drop interstitial (#2304) * feat(backend): Remove interstitial endpoints * feat(backend,types): Remove local interstitial script * feat(types): Clean retheme types * feat(backend): Remove interstitial and interstitial rules * feat(clerk-js): Remove interstitial from clerk-js * feat(nextjs): Remove interstitial from authMiddleware * feat(fastify): Remove interstitial * feat(gatsby-plugin-clerk): Remove interstitial * feat(remix): Remove interstitial * feat(clerk-sdk-node): Remove interstitial * fix(nextjs): Always respect redirect header if found As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example) * chore(repo): Fix sdk tests * fix(clerk-js): Fix tests related to db-jwt * fix(clerk-js): Keep hasJustSynced check * chore(*): Fix linter * chore(backend): Remove unused AuthErrorReason properties, destructure from authenticateContext * chore(clerk-js): Remove unused @ts-expect-error directive * feat(repo): Migrate handshake tests to e2e test suite * fix(nextjs): Ensure we respect location header from authenticateRequest() regardless of auth status * Create bright-ways-provide.md * chore(clerk-sdk-node): Properly export createClerkClient * chore(clerk-sdk-node): Properly export createClerkClient * fix(nextjs): Fix undefined headers in tests --------- Co-authored-by: Sokratis Vidros <sokratis.vidros@gmail.com> Co-authored-by: Colin Sidoti <51144033+colinclerk@users.noreply.github.com> Co-authored-by: Nikos Douvlis <nikosdouvlis@gmail.com> Co-authored-by: Colin Sidoti <colin@clerk.dev>

clerk-cookie added types clerk-js nextjs backend retheme labels Dec 11, 2023

nikosdouvlis force-pushed the nikos/rm-interstitial-handshake-refactor branch from 040a4f6 to 77b699c Compare December 11, 2023 15:00

brkalow reviewed Dec 11, 2023

View reviewed changes

SokratisVidros approved these changes Dec 11, 2023

View reviewed changes

clerk-cookie added remix fastify gatsby labels Dec 11, 2023

nikosdouvlis force-pushed the nikos/rm-interstitial-handshake-refactor branch from fc043fa to dcf531b Compare December 11, 2023 23:29

nikosdouvlis marked this pull request as ready for review December 11, 2023 23:36

clerk-cookie added the sdk-node label Dec 11, 2023

brkalow approved these changes Dec 12, 2023

View reviewed changes

dimkl approved these changes Dec 12, 2023

View reviewed changes

nikosdouvlis added 12 commits December 12, 2023 12:44

feat(backend): Remove interstitial endpoints

5694b00

feat(backend,types): Remove local interstitial script

a83c7fb

feat(types): Clean retheme types

ed43c8c

feat(backend): Remove interstitial and interstitial rules

e9aa61d

feat(clerk-js): Remove interstitial from clerk-js

fd837ce

feat(nextjs): Remove interstitial from authMiddleware

954da39

feat(fastify): Remove interstitial

f396575

feat(gatsby-plugin-clerk): Remove interstitial

cc982c7

feat(remix): Remove interstitial

a67e3bf

feat(clerk-sdk-node): Remove interstitial

be6d699

fix(nextjs): Always respect redirect header if found

c3e8989

As it's possible that we trigger a redirect from authenticateRequest that isn't a handshake status (dev multi-domain sync, for example)

chore(repo): Fix sdk tests

7c8906c

nikosdouvlis force-pushed the nikos/rm-interstitial-handshake-refactor branch from b321473 to 7c8906c Compare December 12, 2023 12:05

fix(clerk-js): Fix tests related to db-jwt

b1f0cb8

nikosdouvlis force-pushed the nikos/rm-interstitial-handshake-refactor branch from 1dadc54 to b1f0cb8 Compare December 12, 2023 12:27

fix(clerk-js): Keep hasJustSynced check

6651411

nikosdouvlis merged commit 4e49b74 into client_handshake_Refactor Dec 12, 2023

nikosdouvlis deleted the nikos/rm-interstitial-handshake-refactor branch December 12, 2023 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Drop interstitial#2304

Drop interstitial#2304
nikosdouvlis merged 14 commits into
client_handshake_Refactorfrom
nikos/rm-interstitial-handshake-refactor

nikosdouvlis commented Dec 11, 2023 •

edited

Loading

Uh oh!

changeset-bot Bot commented Dec 11, 2023 •

edited

Loading

Uh oh!

brkalow Dec 11, 2023

Uh oh!

nikosdouvlis Dec 11, 2023

Uh oh!

nikosdouvlis Dec 11, 2023

Uh oh!

brkalow left a comment

Uh oh!

brkalow Dec 12, 2023

Uh oh!

brkalow Dec 12, 2023

Uh oh!

dimkl left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

nikosdouvlis commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Type of change

Packages affected

Uh oh!

changeset-bot Bot commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

brkalow Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

nikosdouvlis Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

nikosdouvlis Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

brkalow left a comment

Choose a reason for hiding this comment

Uh oh!

brkalow Dec 12, 2023

Choose a reason for hiding this comment

Uh oh!

brkalow Dec 12, 2023

Choose a reason for hiding this comment

Uh oh!

dimkl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nikosdouvlis commented Dec 11, 2023 •

edited

Loading

changeset-bot Bot commented Dec 11, 2023 •

edited

Loading