| author | |
| committer | |
| log | a7c5ad5db167b3a172ccb26408b0638c99b2a459 |
| tree | dedfb66ef1b3f21298b30028af693f914f6baa8f |
| parent | 6e6d1b49412d064ccbde7daae2e287f9b62efd7d |
| signature | Signed by PGP key B5690EEEBB952194 |
5 files changed, 68 insertions(+), 11 deletions(-)
apps/web/src/cloud/connectCliAuth.test.ts+24| ... | @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vite-plus/test"; | ... | @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vite-plus/test"; |
| 2 | 2 | ||
| 3 | import { | 3 | import { |
| 4 | buildConnectCliClerkAuthorizeUrl, | 4 | buildConnectCliClerkAuthorizeUrl, |
| 5 | connectCliSignInRedirectUrl, | ||
| 5 | hasConnectCliAuthConfig, | 6 | hasConnectCliAuthConfig, |
| 6 | readConnectCliCallbackResult, | 7 | readConnectCliCallbackResult, |
| 7 | } from "./connectCliAuth"; | 8 | } from "./connectCliAuth"; |
| ... | @@ -69,6 +70,29 @@ describe("connectCliAuth", () => { | ... | @@ -69,6 +70,29 @@ describe("connectCliAuth", () => { |
| 69 | ).toBeNull(); | 70 | ).toBeNull(); |
| 70 | }); | 71 | }); |
| 71 | 72 | ||
| 73 | it("sends the sign-in redirect to the authorize endpoint, not back to /connect", () => { | ||
| 74 | vi.stubEnv("VITE_CLERK_PUBLISHABLE_KEY", TEST_PUBLISHABLE_KEY); | ||
| 75 | vi.stubEnv("VITE_CLERK_CLI_OAUTH_CLIENT_ID", "oauthapp_123"); | ||
| 76 | |||
| 77 | const connectUrl = "https://app.t3.codes/connect#state=state-1&challenge=challenge-1"; | ||
| 78 | const redirectUrl = connectCliSignInRedirectUrl( | ||
| 79 | { state: "state-1", challenge: "challenge-1" }, | ||
| 80 | connectUrl, | ||
| 81 | ); | ||
| 82 | |||
| 83 | expect(redirectUrl).not.toBe(connectUrl); | ||
| 84 | expect(new URL(redirectUrl).pathname).toBe("/oauth/authorize"); | ||
| 85 | }); | ||
| 86 | |||
| 87 | it("falls back to the current URL when the authorize URL cannot be built", () => { | ||
| 88 | vi.stubEnv("VITE_CLERK_PUBLISHABLE_KEY", TEST_PUBLISHABLE_KEY); | ||
| 89 | |||
| 90 | const connectUrl = "https://app.t3.codes/connect#state=state-1&challenge=challenge-1"; | ||
| 91 | expect( | ||
| 92 | connectCliSignInRedirectUrl({ state: "state-1", challenge: "challenge-1" }, connectUrl), | ||
| 93 | ).toBe(connectUrl); | ||
| 94 | }); | ||
| 95 | |||
| 72 | it("reads the code and state Clerk echoes back to the callback", () => { | 96 | it("reads the code and state Clerk echoes back to the callback", () => { |
| 73 | expect( | 97 | expect( |
| 74 | readConnectCliCallbackResult( | 98 | readConnectCliCallbackResult( |
apps/web/src/cloud/connectCliAuth.ts+17| ... | @@ -60,6 +60,23 @@ export function buildConnectCliClerkAuthorizeUrl(request: ConnectAuthorizeReques | ... | @@ -60,6 +60,23 @@ export function buildConnectCliClerkAuthorizeUrl(request: ConnectAuthorizeReques |
| 60 | }); | 60 | }); |
| 61 | } | 61 | } |
| 62 | 62 | ||
| 63 | /** | ||
| 64 | * Where Clerk sends the browser once the sign-in modal on /connect completes. | ||
| 65 | * It has to be the authorize endpoint rather than this page: /connect carries | ||
| 66 | * the CLI request in its fragment, so navigating back to the same URL is a | ||
| 67 | * same-document fragment navigation the browser never reloads — and Clerk | ||
| 68 | * treats any post-sign-in navigation as a page unload and skips the state emit | ||
| 69 | * that would otherwise re-render the surface, so the session never arrives | ||
| 70 | * either. Falls back to the current URL when the authorize URL cannot be | ||
| 71 | * built, which only happens on a deployment without the CLI OAuth config. | ||
| 72 | */ | ||
| 73 | export function connectCliSignInRedirectUrl( | ||
| 74 | request: ConnectAuthorizeRequest, | ||
| 75 | currentHref: string, | ||
| 76 | ): string { | ||
| 77 | return buildConnectCliClerkAuthorizeUrl(request) ?? currentHref; | ||
| 78 | } | ||
| 79 | |||
| 63 | export function rememberConnectCliAuthState(state: string): void { | 80 | export function rememberConnectCliAuthState(state: string): void { |
| 64 | try { | 81 | try { |
| 65 | window.sessionStorage.setItem(CONNECT_CLI_AUTH_STATE_STORAGE_KEY, state); | 82 | window.sessionStorage.setItem(CONNECT_CLI_AUTH_STATE_STORAGE_KEY, state); |
apps/web/src/components/clerk/authRedirect.test.ts+4-1| ... | @@ -5,7 +5,10 @@ import { resolveClerkSignInProps } from "./authRedirect"; | ... | @@ -5,7 +5,10 @@ import { resolveClerkSignInProps } from "./authRedirect"; |
| 5 | describe("resolveClerkSignInProps", () => { | 5 | describe("resolveClerkSignInProps", () => { |
| 6 | it("returns to the current browser URL on the web", () => { | 6 | it("returns to the current browser URL on the web", () => { |
| 7 | const href = "https://app.t3.codes/connect?state=state-1#details"; | 7 | const href = "https://app.t3.codes/connect?state=state-1#details"; |
| 8 | expect(resolveClerkSignInProps(href, false)).toEqual({ forceRedirectUrl: href }); | 8 | expect(resolveClerkSignInProps(href, false)).toEqual({ |
| 9 | forceRedirectUrl: href, | ||
| 10 | signUpForceRedirectUrl: href, | ||
| 11 | }); | ||
| 9 | }); | 12 | }); |
| 10 | 13 | ||
| 11 | it("removes a Clerk virtual pathname and callback params while preserving the desktop route", () => { | 14 | it("removes a Clerk virtual pathname and callback params while preserving the desktop route", () => { |
apps/web/src/components/clerk/authRedirect.ts+3-1| ... | @@ -15,5 +15,7 @@ export function resolveClerkSignInProps(href: string, isElectron: boolean): Cler | ... | @@ -15,5 +15,7 @@ export function resolveClerkSignInProps(href: string, isElectron: boolean): Cler |
| 15 | signUpForceRedirectUrl: redirectUrl.toString(), | 15 | signUpForceRedirectUrl: redirectUrl.toString(), |
| 16 | }; | 16 | }; |
| 17 | } | 17 | } |
| 18 | return { forceRedirectUrl: href }; | 18 | // The sign-in modal can switch to sign-up, which follows its own redirect |
| 19 | // target; without one Clerk falls back to the URL the modal was opened from. | ||
| 20 | return { forceRedirectUrl: href, signUpForceRedirectUrl: href }; | ||
| 19 | } | 21 | } |
apps/web/src/components/cloud/ConnectCliAuthSurface.tsx+20-9| ... | @@ -1,9 +1,10 @@ | ... | @@ -1,9 +1,10 @@ |
| 1 | import { useAuth, useClerk, useUser } from "@clerk/react"; | 1 | import { useAuth, useClerk, useUser } from "@clerk/react"; |
| 2 | import { encodeConnectAuthCode, readConnectAuthorizeRequest } from "@t3tools/shared/connectAuth"; | 2 | import { encodeConnectAuthCode, readConnectAuthorizeRequest } from "@t3tools/shared/connectAuth"; |
| 3 | import { useEffect, useRef, useState } from "react"; | 3 | import { useCallback, useEffect, useRef, useState } from "react"; |
| 4 | 4 | ||
| 5 | import { | 5 | import { |
| 6 | buildConnectCliClerkAuthorizeUrl, | 6 | buildConnectCliClerkAuthorizeUrl, |
| 7 | connectCliSignInRedirectUrl, | ||
| 7 | readConnectCliAuthState, | 8 | readConnectCliAuthState, |
| 8 | readConnectCliCallbackResult, | 9 | readConnectCliCallbackResult, |
| 9 | rememberConnectCliAuthState, | 10 | rememberConnectCliAuthState, |
| ... | @@ -56,6 +57,21 @@ export function ConnectCliAuthorizeSurface() { | ... | @@ -56,6 +57,21 @@ export function ConnectCliAuthorizeSurface() { |
| 56 | const signInOpened = useRef(false); | 57 | const signInOpened = useRef(false); |
| 57 | const redirecting = useRef(false); | 58 | const redirecting = useRef(false); |
| 58 | 59 | ||
| 60 | const openSignIn = useCallback(() => { | ||
| 61 | if (!request) { | ||
| 62 | return; | ||
| 63 | } | ||
| 64 | // Clerk redirects to the authorize endpoint itself once sign-in completes, | ||
| 65 | // so the callback's state check has to be armed before handing off. | ||
| 66 | rememberConnectCliAuthState(request.state); | ||
| 67 | clerk.openSignIn( | ||
| 68 | resolveClerkSignInProps( | ||
| 69 | connectCliSignInRedirectUrl(request, window.location.href), | ||
| 70 | isElectron, | ||
| 71 | ), | ||
| 72 | ); | ||
| 73 | }, [clerk, request]); | ||
| 74 | |||
| 59 | useEffect(() => { | 75 | useEffect(() => { |
| 60 | if (!request || !isLoaded || redirecting.current) { | 76 | if (!request || !isLoaded || redirecting.current) { |
| 61 | return; | 77 | return; |
| ... | @@ -63,7 +79,7 @@ export function ConnectCliAuthorizeSurface() { | ... | @@ -63,7 +79,7 @@ export function ConnectCliAuthorizeSurface() { |
| 63 | if (!isSignedIn) { | 79 | if (!isSignedIn) { |
| 64 | if (!signInOpened.current) { | 80 | if (!signInOpened.current) { |
| 65 | signInOpened.current = true; | 81 | signInOpened.current = true; |
| 66 | clerk.openSignIn(resolveClerkSignInProps(window.location.href, isElectron)); | 82 | openSignIn(); |
| 67 | } | 83 | } |
| 68 | return; | 84 | return; |
| 69 | } | 85 | } |
| ... | @@ -74,7 +90,7 @@ export function ConnectCliAuthorizeSurface() { | ... | @@ -74,7 +90,7 @@ export function ConnectCliAuthorizeSurface() { |
| 74 | redirecting.current = true; | 90 | redirecting.current = true; |
| 75 | rememberConnectCliAuthState(request.state); | 91 | rememberConnectCliAuthState(request.state); |
| 76 | window.location.assign(authorizeUrl); | 92 | window.location.assign(authorizeUrl); |
| 77 | }, [clerk, isLoaded, isSignedIn, request]); | 93 | }, [isLoaded, isSignedIn, openSignIn, request]); |
| 78 | 94 | ||
| 79 | if (!request) { | 95 | if (!request) { |
| 80 | return ( | 96 | return ( |
| ... | @@ -101,12 +117,7 @@ export function ConnectCliAuthorizeSurface() { | ... | @@ -101,12 +117,7 @@ export function ConnectCliAuthorizeSurface() { |
| 101 | /> | 117 | /> |
| 102 | {isLoaded && !isSignedIn ? ( | 118 | {isLoaded && !isSignedIn ? ( |
| 103 | <div className="mt-6"> | 119 | <div className="mt-6"> |
| 104 | <Button | 120 | <Button type="button" onClick={openSignIn}> |
| 105 | type="button" | ||
| 106 | onClick={() => | ||
| 107 | clerk.openSignIn(resolveClerkSignInProps(window.location.href, isElectron)) | ||
| 108 | } | ||
| 109 | > | ||
| 110 | Sign in | 121 | Sign in |
| 111 | </Button> | 122 | </Button> |
| 112 | </div> | 123 | </div> |