Skip to content

Commit ba20840

Browse files
broodybroody
authored
fix: preserve custom-scheme redirect_uri in standalone session flow (#2481)
## Summary - preserve custom-scheme redirect targets when deriving the standalone session app origin instead of collapsing them to the literal `"null"` - restore standalone verification to check both `redirect_url` and `redirect_uri`, matching the session flow inputs - harden the session consent UI so non-HTTP origins do not crash `/session` - add regression coverage for custom-scheme standalone redirects and redirect param fallback ## Context - this fixes standalone `/session` flows for apps using custom redirect schemes such as `cagecalls://open` and `jokers://open` - this also restores the earlier domain-verification fix for integrations that only provide `redirect_uri` ## Test plan - [x] `pnpm --filter @cartridge/keychain exec vitest run src/hooks/connection.test.ts src/components/session.test.tsx src/components/ConnectRoute.test.tsx` - [x] Pre-commit checks via `git commit` (lint, tests, build, graphql codegen) - [ ] Verify `/session` completes with `redirect_uri=cagecalls://open` - [ ] Verify `/session` completes with `redirect_uri=jokers://open`
1 parent 00275a5 commit ba20840

File tree

5 files changed

+325
-58
lines changed

5 files changed

+325
-58
lines changed

‎packages/keychain/src/components/connect/SessionConsent.tsx‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,17 @@ export function SessionConsent({
1111
variant?: "default" | "slot" | "signup";
1212
}) {
1313
const { origin } = useConnection();
14-
const hostname = useMemo(
15-
() => (origin ? new URL(origin).hostname : undefined),
16-
[origin],
17-
);
14+
const hostname = useMemo(() => {
15+
if (!origin) {
16+
return undefined;
17+
}
18+
19+
try {
20+
return new URL(origin).hostname;
21+
} catch {
22+
return undefined;
23+
}
24+
}, [origin]);
1825

1926
switch (variant) {
2027
case "slot":

‎packages/keychain/src/components/provider/index.test.tsx‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
import { render, screen } from "@testing-library/react";
2+
import { PropsWithChildren } from "react";
3+
import { beforeEach, describe, expect, it, vi } from "vitest";
4+
import { Provider } from "./index";
5+
6+
const mockUseConnectionValue = vi.fn();
7+
8+
vi.mock("@/hooks/connection", () => ({
9+
useConnectionValue: () => mockUseConnectionValue(),
10+
}));
11+
12+
vi.mock("@/components/provider/upgrade", () => ({
13+
UpgradeProvider: ({ children }: PropsWithChildren) => <>{children}</>,
14+
}));
15+
16+
vi.mock("@/hooks/wallets", () => ({
17+
WalletsProvider: ({ children }: PropsWithChildren) => <>{children}</>,
18+
}));
19+
20+
vi.mock("@/utils/graphql", () => ({
21+
ENDPOINT: "https://example.com",
22+
}));
23+
24+
vi.mock("@auth0/auth0-react", () => ({
25+
Auth0Provider: ({ children }: PropsWithChildren) => <>{children}</>,
26+
}));
27+
28+
vi.mock("@starknet-react/chains", () => ({
29+
mainnet: {},
30+
sepolia: {},
31+
}));
32+
33+
vi.mock("@starknet-react/core", () => ({
34+
cartridge: {},
35+
jsonRpcProvider: vi.fn(() => ({})),
36+
StarknetConfig: ({ children }: PropsWithChildren) => <>{children}</>,
37+
}));
38+
39+
vi.mock("@turnkey/sdk-react", () => ({
40+
TurnkeyProvider: ({ children }: PropsWithChildren) => <>{children}</>,
41+
}));
42+
43+
vi.mock("react-query", () => ({
44+
QueryClient: class {},
45+
QueryClientProvider: ({ children }: PropsWithChildren) => <>{children}</>,
46+
}));
47+
48+
vi.mock("starknet", () => ({
49+
constants: {
50+
StarknetChainId: {
51+
SN_MAIN: "SN_MAIN",
52+
SN_SEPOLIA: "SN_SEPOLIA",
53+
},
54+
},
55+
num: {
56+
toBigInt: () => 0n,
57+
},
58+
}));
59+
60+
vi.mock("./posthog", () => ({
61+
PostHogProvider: ({ children }: PropsWithChildren) => <>{children}</>,
62+
}));
63+
64+
vi.mock("./tokens", () => ({
65+
TokensProvider: ({ children }: PropsWithChildren) => <>{children}</>,
66+
}));
67+
68+
vi.mock("./ui", () => ({
69+
UIProvider: ({ children }: PropsWithChildren) => <>{children}</>,
70+
}));
71+
72+
vi.mock("@/hooks/features", () => ({
73+
FeatureProvider: ({ children }: PropsWithChildren) => <>{children}</>,
74+
}));
75+
76+
vi.mock("@/components/provider/arcade", () => ({
77+
ArcadeProvider: ({ children }: PropsWithChildren) => <>{children}</>,
78+
}));
79+
80+
vi.mock("@/components/provider/data", () => ({
81+
DataProvider: ({ children }: PropsWithChildren) => <>{children}</>,
82+
}));
83+
84+
vi.mock("@/context", () => ({
85+
ToastProvider: ({ children }: PropsWithChildren) => <>{children}</>,
86+
}));
87+
88+
vi.mock("@/context/quest", () => ({
89+
QuestProvider: ({ children }: PropsWithChildren) => <>{children}</>,
90+
}));
91+
92+
vi.mock("@cartridge/ui/utils/api/indexer", () => ({
93+
IndexerAPIProvider: ({ children }: PropsWithChildren) => <>{children}</>,
94+
}));
95+
96+
vi.mock("@cartridge/ui/utils/api/cartridge", () => ({
97+
CartridgeAPIProvider: ({ children }: PropsWithChildren) => <>{children}</>,
98+
}));
99+
100+
vi.mock("../ErrorBoundary", () => ({
101+
ErrorBoundary: ({ children }: PropsWithChildren) => <>{children}</>,
102+
}));
103+
104+
vi.mock("@cartridge/arcade/marketplace/react", () => ({
105+
MarketplaceClientProvider: ({ children }: PropsWithChildren) => (
106+
<>{children}</>
107+
),
108+
}));
109+
110+
vi.mock("@cartridge/ui", () => ({
111+
SpinnerIcon: () => <div data-testid="loading-spinner" />,
112+
}));
113+
114+
const baseConnection = {
115+
parent: undefined,
116+
controller: undefined,
117+
origin: "",
118+
rpcUrl: "https://rpc.example.com",
119+
project: null,
120+
namespace: null,
121+
propagateError: false,
122+
webauthnPopup: {
123+
create: false,
124+
get: false,
125+
},
126+
preset: null,
127+
policiesStr: null,
128+
theme: {
129+
name: "default",
130+
verified: true,
131+
},
132+
isConfigLoading: false,
133+
isPoliciesResolved: true,
134+
isMainnet: false,
135+
verified: true,
136+
chainId: undefined,
137+
setController: vi.fn(),
138+
controllerVersion: undefined,
139+
setRpcUrl: vi.fn(),
140+
openModal: vi.fn().mockResolvedValue(undefined),
141+
logout: vi.fn().mockResolvedValue(undefined),
142+
openSettings: vi.fn(),
143+
externalDetectWallets: vi.fn().mockResolvedValue([]),
144+
externalConnectWallet: vi.fn(),
145+
externalSignTypedData: vi.fn(),
146+
externalSignMessage: vi.fn(),
147+
externalSendTransaction: vi.fn(),
148+
externalGetBalance: vi.fn(),
149+
externalWaitForTransaction: vi.fn(),
150+
};
151+
152+
describe("Provider", () => {
153+
beforeEach(() => {
154+
vi.clearAllMocks();
155+
mockUseConnectionValue.mockReturnValue(baseConnection);
156+
});
157+
158+
it("shows a loading spinner while preset config is loading", () => {
159+
mockUseConnectionValue.mockReturnValue({
160+
...baseConnection,
161+
isConfigLoading: true,
162+
});
163+
164+
render(
165+
<Provider>
166+
<div>child content</div>
167+
</Provider>,
168+
);
169+
170+
expect(screen.getByTestId("loading-spinner")).toBeInTheDocument();
171+
expect(screen.queryByText("child content")).not.toBeInTheDocument();
172+
});
173+
174+
it("renders children once preset config loading has finished", () => {
175+
render(
176+
<Provider>
177+
<div>child content</div>
178+
</Provider>,
179+
);
180+
181+
expect(screen.getByText("child content")).toBeInTheDocument();
182+
expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument();
183+
});
184+
});

‎packages/keychain/src/components/provider/index.tsx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { IndexerAPIProvider } from "@cartridge/ui/utils/api/indexer";
2626
import { CartridgeAPIProvider } from "@cartridge/ui/utils/api/cartridge";
2727
import { ErrorBoundary } from "../ErrorBoundary";
2828
import { MarketplaceClientProvider } from "@cartridge/arcade/marketplace/react";
29+
import { SpinnerIcon } from "@cartridge/ui";
2930

3031
export function Provider({ children }: PropsWithChildren) {
3132
const connection = useConnectionValue();
@@ -59,6 +60,14 @@ export function Provider({ children }: PropsWithChildren) {
5960
[connection.controller, connection.project],
6061
);
6162

63+
if (connection.isConfigLoading) {
64+
return (
65+
<div className="flex h-screen w-screen items-center justify-center bg-background">
66+
<SpinnerIcon className="animate-spin text-muted-foreground" size="lg" />
67+
</div>
68+
);
69+
}
70+
6271
return (
6372
<FeatureProvider>
6473
<CartridgeAPIProvider url={ENDPOINT}>

‎packages/keychain/src/hooks/connection.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import {
2+
getStandaloneAppOrigin,
3+
getStandaloneRedirectUrl,
24
isNestedIframe,
35
isOriginVerified,
46
resolvePolicies,
@@ -79,6 +81,43 @@ describe("isOriginVerified", () => {
7981
});
8082
});
8183

84+
describe("getStandaloneAppOrigin", () => {
85+
it("should use the URL origin for http redirects", () => {
86+
expect(getStandaloneAppOrigin("https://example.com/callback?foo=bar")).toBe(
87+
"https://example.com",
88+
);
89+
});
90+
91+
it("should preserve custom-scheme redirects instead of collapsing to null", () => {
92+
expect(getStandaloneAppOrigin("cagecalls://open")).toBe("cagecalls://open");
93+
});
94+
});
95+
96+
describe("getStandaloneRedirectUrl", () => {
97+
it("prefers redirect_url when present", () => {
98+
const searchParams = new URLSearchParams({
99+
redirect_url: "https://example.com/callback",
100+
redirect_uri: "jokers://open",
101+
});
102+
103+
expect(getStandaloneRedirectUrl(searchParams)).toBe(
104+
"https://example.com/callback",
105+
);
106+
});
107+
108+
it("falls back to redirect_uri when redirect_url is absent", () => {
109+
const searchParams = new URLSearchParams({
110+
redirect_uri: "jokers://open",
111+
});
112+
113+
expect(getStandaloneRedirectUrl(searchParams)).toBe("jokers://open");
114+
});
115+
116+
it("returns null when no standalone redirect target is present", () => {
117+
expect(getStandaloneRedirectUrl(new URLSearchParams())).toBeNull();
118+
});
119+
});
120+
82121
describe("isNestedIframe", () => {
83122
it("returns false for the top-level window", () => {
84123
const top = {};

0 commit comments

Comments
 (0)