Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/thirdweb/src/react/web/ui/SiteLink.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,25 @@ describe("SiteLink", () => {
expect(anchor).toBeTruthy();
await waitFor(() => expect(anchor?.href).toContain("walletId=inApp"));
});

it("preserves hash fragment for hash-routed URLs", async () => {
const testUrl = "https://snapshot.org/#/s:wampei.eth";
const { container } = render(
<SiteLink client={TEST_CLIENT} href={testUrl}>
Test Link
</SiteLink>,
{
setConnectedWallet: true,
},
);

const anchor = container.querySelector("a");
expect(anchor).toBeTruthy();
await waitFor(() => {
const href = anchor?.href ?? "";
// Hash fragment must be preserved in the URL
expect(href).toContain("#/s:wampei.eth");
expect(href).toContain("walletId=");
});
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { getUrlToken } from "./get-url-token.js";

describe.runIf(global.window !== undefined)("getUrlToken", () => {
Expand Down Expand Up @@ -48,9 +48,9 @@ describe.runIf(global.window !== undefined)("getUrlToken", () => {
const result = getUrlToken();

expect(result).toEqual({
authCookie: null,
authFlow: null,
authProvider: null,
authCookie: undefined,
authFlow: undefined,
authProvider: undefined,
authResult: { token: "abc" },
walletId: "123",
});
Expand All @@ -63,8 +63,8 @@ describe.runIf(global.window !== undefined)("getUrlToken", () => {

expect(result).toEqual({
authCookie: "myCookie",
authFlow: null,
authProvider: null,
authFlow: undefined,
authProvider: undefined,
authResult: undefined,
walletId: "123",
});
Expand All @@ -81,7 +81,7 @@ describe.runIf(global.window !== undefined)("getUrlToken", () => {

expect(result).toEqual({
authCookie: "myCookie",
authFlow: null,
authFlow: undefined,
authProvider: "provider1",
authResult: { token: "xyz" },
walletId: "123",
Expand All @@ -92,4 +92,60 @@ describe.runIf(global.window !== undefined)("getUrlToken", () => {
"?walletId=123&authResult=%7B%22token%22%3A%22xyz%22%7D&authProvider=provider1&authCookie=myCookie",
);
});

it("should preserve hash fragment when cleaning up URL", () => {
Object.defineProperty(window, "location", {
value: {
...window.location,
search: "?walletId=123&authCookie=myCookie",
hash: "#/s:wampei.eth",
pathname: "/",
},
writable: true,
});

const pushStateSpy = vi.spyOn(window.history, "pushState");

const result = getUrlToken();

expect(result).toEqual({
authCookie: "myCookie",
authFlow: undefined,
authProvider: undefined,
authResult: undefined,
walletId: "123",
});

// Verify pushState was called with the hash preserved
expect(pushStateSpy).toHaveBeenCalledWith({}, "", "/#/s:wampei.eth");
pushStateSpy.mockRestore();
});

it("should parse auth params embedded inside the hash fragment", () => {
Object.defineProperty(window, "location", {
value: {
...window.location,
search: "",
hash: "#/s:wampei.eth?walletId=123&authCookie=myCookie",
pathname: "/",
},
writable: true,
});

const pushStateSpy = vi.spyOn(window.history, "pushState");

const result = getUrlToken();

expect(result).toEqual({
authCookie: "myCookie",
authFlow: undefined,
authProvider: undefined,
authResult: undefined,
walletId: "123",
});

// Verify pushState preserves hash path but strips auth params from it
expect(pushStateSpy).toHaveBeenCalledWith({}, "", "/#/s:wampei.eth");
pushStateSpy.mockRestore();
});
});
55 changes: 47 additions & 8 deletions packages/thirdweb/src/wallets/in-app/web/lib/get-url-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,68 @@
return undefined;
}

const queryString = window.location.search;
const params = new URLSearchParams(queryString);
const authResultString = params.get("authResult");
const walletId = params.get("walletId") as WalletId | undefined;
const authProvider = params.get("authProvider") as AuthOption | undefined;
const authCookie = params.get("authCookie") as string | undefined;
const authFlow = params.get("authFlow") as "connect" | "link" | undefined;
// Read params from the standard query string
const params = new URLSearchParams(window.location.search);

// Also check for params embedded inside the hash fragment (e.g. #/route?walletId=...)
// This supports hash-routed apps where params may be placed after the hash path
let hashParams: URLSearchParams | undefined;
const hash = window.location.hash || "";
let cleanHash = hash;
const hashQueryIndex = hash.indexOf("?");
if (hashQueryIndex !== -1) {
hashParams = new URLSearchParams(hash.substring(hashQueryIndex));
cleanHash = hash.substring(0, hashQueryIndex);
}

const walletId = (params.get("walletId") ??
hashParams?.get("walletId") ??
undefined) as WalletId | undefined;
const authResultString =
params.get("authResult") ?? hashParams?.get("authResult") ?? undefined;
const authProvider = (params.get("authProvider") ??
hashParams?.get("authProvider") ??
undefined) as AuthOption | undefined;
const authCookie = (params.get("authCookie") ??
hashParams?.get("authCookie") ??
undefined) as string | undefined;
const authFlow = (params.get("authFlow") ??
hashParams?.get("authFlow") ??
undefined) as "connect" | "link" | undefined;

if ((authCookie || authResultString) && walletId) {
const authResult = (() => {
if (authResultString) {
params.delete("authResult");
hashParams?.delete("authResult");
return JSON.parse(decodeURIComponent(authResultString));
}
})();
params.delete("walletId");
params.delete("authProvider");
params.delete("authCookie");
params.delete("authFlow");
hashParams?.delete("walletId");
hashParams?.delete("authProvider");
hashParams?.delete("authCookie");
hashParams?.delete("authFlow");
Comment on lines +37 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing params.delete("walletId") in the cleanup logic. The code deletes walletId from hashParams (line 63) but not from the standard params. If walletId is in the standard query string, it will remain in the reconstructed URL after authentication.

Impact: walletId will not be cleaned up from the query string, potentially causing it to persist in the URL after auth completion.

Fix: Add params.delete("walletId"); before line 63:

params.delete("authProvider");
params.delete("authCookie");
params.delete("authFlow");
params.delete("walletId");  // Add this line
hashParams?.delete("walletId");
Suggested change
hashParams?.get("walletId") ??
undefined) as WalletId | undefined;
const authResultString =
params.get("authResult") ?? hashParams?.get("authResult") ?? undefined;
const authProvider = (params.get("authProvider") ??
hashParams?.get("authProvider") ??
undefined) as AuthOption | undefined;
const authCookie = (params.get("authCookie") ??
hashParams?.get("authCookie") ??
undefined) as string | undefined;
const authFlow = (params.get("authFlow") ??
hashParams?.get("authFlow") ??
undefined) as "connect" | "link" | undefined;
if ((authCookie || authResultString) && walletId) {
const authResult = (() => {
if (authResultString) {
params.delete("authResult");
hashParams?.delete("authResult");
return JSON.parse(decodeURIComponent(authResultString));
}
})();
params.delete("walletId");
params.delete("authProvider");
params.delete("authCookie");
params.delete("authFlow");
hashParams?.delete("walletId");
hashParams?.delete("authProvider");
hashParams?.delete("authCookie");
hashParams?.delete("authFlow");
hashParams?.get("walletId") ??
undefined) as WalletId | undefined;
const authResultString =
params.get("authResult") ?? hashParams?.get("authResult") ?? undefined;
const authProvider = (params.get("authProvider") ??
hashParams?.get("authProvider") ??
undefined) as AuthOption | undefined;
const authCookie = (params.get("authCookie") ??
hashParams?.get("authCookie") ??
undefined) as string | undefined;
const authFlow = (params.get("authFlow") ??
hashParams?.get("authFlow") ??
undefined) as "connect" | "link" | undefined;
if ((authCookie || authResultString) && walletId) {
const authResult = (() => {
if (authResultString) {
params.delete("authResult");
hashParams?.delete("authResult");
return JSON.parse(decodeURIComponent(authResultString));
}
})();
params.delete("authProvider");
params.delete("authCookie");
params.delete("authFlow");
params.delete("walletId");
hashParams?.delete("walletId");
hashParams?.delete("authProvider");
hashParams?.delete("authCookie");
hashParams?.delete("authFlow");

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


const remainingSearch = params.toString();
const searchString = remainingSearch ? `?${remainingSearch}` : "";

// Reconstruct hash, preserving the hash path and any remaining non-auth params
let hashString = cleanHash;
if (hashParams) {
const remainingHashParams = hashParams.toString();
if (remainingHashParams) {
hashString = `${cleanHash}?${remainingHashParams}`;
}

Check warning on line 77 in packages/thirdweb/src/wallets/in-app/web/lib/get-url-token.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/in-app/web/lib/get-url-token.ts#L76-L77

Added lines #L76 - L77 were not covered by tests
}

window.history.pushState(
{},
"",
`${window.location.pathname}?${params.toString()}`,
`${window.location.pathname}${searchString}${hashString}`,
);
return { authCookie, authFlow, authProvider, authResult, walletId };
}
Expand Down
Loading