feat(clerk-js): Authenticate with popup #5239

dstaley · 2025-02-26T18:06:37Z

Description

This PR implements the ability to authenticate with a popup instead of a redirect for external connections. This is useful in environments like Loveable and Bolt where we're unable to render pages like Google inside of an iframe.

It accomplishes this by adding a new prop to SignIn and SignUp called oauthFlow, with the values "redirect" | "popup" | "auto". The "auto" value uses a new utility, originPrefersPopup, to determine whether the given origin is one that likely needs to use the popup flow. This means that generative AI coding tools do not need to make any specific changes to their code for OAuth flows to work as expected. Users are, of course, able to opt into the "popup" flow specifically if they desire, however we maintain that the "redirect" option is a better UX.

Checklist

pnpm test runs as expected.
pnpm 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

vercel · 2025-02-26T18:06:41Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
clerk-js-sandbox	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 25, 2025 9:30pm

changeset-bot · 2025-03-03T19:17:05Z

🦋 Changeset detected

Latest commit: 367577b

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

This PR includes changesets to release 22 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/agent-toolkit	Patch
@clerk/astro	Patch
@clerk/backend	Patch
@clerk/elements	Patch
@clerk/expo-passkeys	Patch
@clerk/express	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/nextjs	Patch
@clerk/nuxt	Patch
@clerk/react-router	Patch
@clerk/clerk-react	Patch
@clerk/remix	Patch
@clerk/shared	Patch
@clerk/tanstack-react-start	Patch
@clerk/testing	Patch
@clerk/themes	Patch
@clerk/vue	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

jacekradko · 2025-03-21T01:14:53Z

packages/clerk-js/src/core/clerk.ts

+      let shouldRemoveListener = false;
+
+      if (event.data.session) {
+        const existingSession = this.client?.sessions.find(x => x.id === event.data.session) || null;


[nit] we are already checking for existence of this.client on L1856 so no need for the optional chaining here

I think it's because this is an async function that can run with delay which may result to this.client not being defined.

packages/clerk-js/src/core/resources/SignIn.ts

packages/clerk-js/src/ui/components/SignIn/SignInSocialButtons.tsx

panteliselef · 2025-03-21T13:30:14Z

packages/clerk-js/src/core/clerk.ts

+        console.log('This can be safely ignored:');
+        console.error(err);


Is there a significant reason to leave the logs ?

packages/clerk-js/src/core/clerk.ts

panteliselef · 2025-03-21T13:33:15Z

packages/clerk-js/src/core/clerk.ts

+    const r = new URL(redirectUrl);
+    r.searchParams.set('sign_in_force_redirect_url', params.redirectUrlComplete);
+    r.searchParams.set('sign_up_force_redirect_url', params.redirectUrlComplete);
+    // All URLs are decorated with the dev browser token in development mode since we're moving between AP and the app.


panteliselef · 2025-03-21T13:36:28Z

packages/clerk-js/src/core/clerk.ts

+      let shouldRemoveListener = false;
+
+      if (event.data.session) {
+        const existingSession = this.client?.sessions.find(x => x.id === event.data.session) || null;


I think it's because this is an async function that can run with delay which may result to this.client not being defined.

panteliselef · 2025-03-21T13:37:29Z

packages/clerk-js/src/core/clerk.ts

+        if (!existingSession) {
+          try {
+            await this.client?.reload();
+          } catch (e) {
+            console.error(e);
+          }
+        }


Is this another attempt to recover if stale ?

Yup, I noticed in a few instances we were receiving a session ID that wasn't in the return value from this.client.session.

panteliselef · 2025-03-21T13:39:28Z

packages/clerk-js/src/core/clerk.ts

+        await this.setActive({
+          session: event.data.session,
+          redirectUrl: params.redirectUrlComplete,
+        });


Is it on purpose that we existingSession may not exist, but we are still calling set active with the event data that was provided ?

packages/clerk-js/src/ui/contexts/components/SignIn.ts

panteliselef · 2025-03-21T13:49:17Z

packages/clerk-js/src/ui/utils/originPrefersPopup.ts

+const POPUP_PREFERRED_ORIGINS = ['.lovable.app', '.lovableproject.com', '.webcontainer-api.io'];
+
+/**
+ * Returns `true` if the current origin is one that is typically embedded via an iframe, which would benefit from the
+ * popup flow.
+ * @returns {boolean} Whether the current origin prefers the popup flow.
+ */
+export function originPrefersPopup(): boolean {
+  return POPUP_PREFERRED_ORIGINS.some(origin => window.location.origin.endsWith(origin));
+}


potentially something that could be fetched from /environment, but this feels ok as well.

Your suggesting we set the POPUP_PREFERRED_DOMAINS on the backend? (if so, I do kind of like that doing it from there would give us the ability to modify without releasing - I guess our hotloading kind of gives us the same capability though)

Agreed, that's why I don't feel strongly about it. Only someone with a pinned version would not benefit from hotloading, but I assume users of these tools won't have a reason to do that.

Yup, Bryce and I discussed this. Ultimately we also concurred that it's likely not that big of a deal where this logic lives. There was some hesitancy from my side on whether it's easier to cut a clerk-js release versus updating the return value from a FAPI endpoint.

We're also maintaining a very similar list of origins for AP frame ancestors.

Not blocking for this PR, but I'll capture the "store and reference in a central place" as an enhancement.

packages/clerk-js/src/ui/components/SignIn/SignInSocialButtons.tsx

packages/clerk-js/src/core/clerk.ts

resources

jacekradko · 2025-03-25T16:38:02Z

packages/clerk-js/src/utils/authenticateWithPopup.ts

+
+import type { Clerk } from '../core/clerk';
+
+export async function _authenticateWithPopup(


Curious why we are exporting a function with a leading _ ?

In our SignIn and SignUp resources, we have a public authenticateWithPopup method available at signIn.authenticateWithPopup. These methods share code, so that's pulled out into the shared _authenticateWithPopup method. I would love a better name if you have any suggestions, but the underscore is to distinguish it from the actual public method.

jacekradko

No major concerns

wip

1a9ebc7

clerk-cookie added types clerk-js labels Feb 26, 2025

vercel bot deployed to Preview February 26, 2025 18:07 View deployment