Skip to content
Merged
5 changes: 5 additions & 0 deletions .changeset/autoconnect-redirect-state.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": minor
---

Redirect-based in-app wallet logins now include and verify a one-time `state` value before `AutoConnect` consumes auth material returned in the URL, tying the returned token back to a flow the page actually started. Added a `readUrlToken` option to `AutoConnect` / `useAutoConnect` to opt out of reading wallet auth material from the URL entirely.
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,9 @@ export default function RotateAdminKeyButton(props: {
Cancel
</Button>
<Button
disabled={rotateAdminKeyMutation.isPending || missingSecretKey}
disabled={
rotateAdminKeyMutation.isPending || missingSecretKey
}
onClick={() => rotateAdminKeyMutation.mutate()}
variant="destructive"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ describe.runIf(process.env.TW_SECRET_KEY)("bootstrap", () => {
expect(cloneFactory).not.toBeNull();
});

it("should return saved implementations for zksync chains", async () => {
// zkSync is no longer officially supported
it.skip("should return saved implementations for zksync chains", async () => {
let infra = await getOrDeployInfraForPublishedContract({
account: TEST_ACCOUNT_A,
chain: defineChain(300),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ describe.runIf(process.env.TW_SECRET_KEY)(
expect(results.length).toBe(8);
});

it("should return default constructor params for zksync chains", async () => {
// zkSync is no longer officially supported
it.skip("should return default constructor params for zksync chains", async () => {
const params = await getAllDefaultConstructorParamsForImplementation({
chain: defineChain(300),
client: TEST_CLIENT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { TEST_CLIENT } from "~test/test-clients.js";
import { TEST_ACCOUNT_A } from "~test/test-wallets.js";
import { createWalletAdapter } from "../../adapters/wallet-adapter.js";
import { ethereum } from "../../chains/chain-definitions/ethereum.js";
import type { AuthStoredTokenWithCookieReturnType } from "../in-app/core/authentication/types.js";
import { AUTH_TOKEN_LOCAL_STORAGE_NAME } from "../in-app/core/constants/settings.js";
import { getUrlToken } from "../in-app/web/lib/get-url-token.js";
import type { Wallet } from "../interfaces/wallet.js";
Expand Down Expand Up @@ -138,6 +139,75 @@ describe("useAutoConnectCore", () => {
expect(storedCookie).toBe(mockAuthCookie);
});

it("should ignore a URL authResult with no matching redirect state", async () => {
const wallet = createWalletAdapter({
adaptedAccount: TEST_ACCOUNT_A,
chain: ethereum,
client: TEST_CLIENT,
onDisconnect: () => {},
switchChain: () => {},
});
// A crafted URL supplies an authResult (and a cookie) with no state to back it.
// Because no redirect state was stored, the token must be rejected wholesale and
// the attacker-supplied cookie must NOT be persisted.
vi.mocked(getUrlToken).mockReturnValue({
authCookie: "should-not-be-saved",
authResult: {
storedToken: { cookieString: "attacker-token" },
} as unknown as AuthStoredTokenWithCookieReturnType,
walletId: wallet.id,
});

await autoConnectCore({
createWalletFn: () => wallet,
force: true,
manager,
props: {
client: TEST_CLIENT,
wallets: [wallet],
},
storage: mockStorage,
});

const storedCookie = await mockStorage.getItem(
AUTH_TOKEN_LOCAL_STORAGE_NAME(TEST_CLIENT.clientId),
);
expect(storedCookie).not.toBe("should-not-be-saved");
});

it("does not read the URL token when readUrlToken is false", async () => {
const wallet = createWalletAdapter({
adaptedAccount: TEST_ACCOUNT_A,
chain: ethereum,
client: TEST_CLIENT,
onDisconnect: () => {},
switchChain: () => {},
});
// With readUrlToken disabled, getUrlToken is not consulted, so an authCookie
// present in the URL is never persisted.
vi.mocked(getUrlToken).mockReturnValue({
authCookie: "url-cookie-should-be-ignored",
walletId: wallet.id,
});

await autoConnectCore({
createWalletFn: () => wallet,
force: true,
manager,
props: {
client: TEST_CLIENT,
readUrlToken: false,
wallets: [wallet],
},
storage: mockStorage,
});

const storedCookie = await mockStorage.getItem(
AUTH_TOKEN_LOCAL_STORAGE_NAME(TEST_CLIENT.clientId),
);
expect(storedCookie).not.toBe("url-cookie-should-be-ignored");
});

it("should handle error when manager connection fails", async () => {
const wallet1 = createWalletAdapter({
adaptedAccount: TEST_ACCOUNT_A,
Expand Down
15 changes: 14 additions & 1 deletion packages/thirdweb/src/wallets/connection/autoConnectCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
AuthStoredTokenWithCookieReturnType,
} from "../in-app/core/authentication/types.js";
import { isInAppSigner } from "../in-app/core/wallet/is-in-app-signer.js";
import { consumeRedirectState } from "../in-app/web/lib/auth/redirect-state.js";
import { getUrlToken } from "../in-app/web/lib/get-url-token.js";
import type { Wallet } from "../interfaces/wallet.js";
import {
Expand Down Expand Up @@ -82,7 +83,19 @@ const _autoConnectCore = async ({
getStoredActiveWalletId(storage),
]);

const urlToken = getUrlToken();
const rawUrlToken = props.readUrlToken === false ? undefined : getUrlToken();

// A token carrying an authResult only ever comes from an SDK-initiated redirect
// login, which persists a one-time state value. Require that state to match before
// trusting the URL-provided auth material, mirroring the origin check the popup
// login flow already performs. If it does not match, ignore the token entirely.
let urlToken = rawUrlToken;
if (rawUrlToken?.authResult) {
const validState = await consumeRedirectState(rawUrlToken.state);
if (!validState) {
urlToken = undefined;
}
}

// Handle linking flow: autoconnect with stored credentials, then link the new profile
if (urlToken?.authFlow === "link" && urlToken.authResult) {
Expand Down
14 changes: 14 additions & 0 deletions packages/thirdweb/src/wallets/connection/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ export type AutoConnectProps = {
*/
onTimeout?: () => void;

/**
* Whether to read wallet auth material (such as an auth token or cookie) from the
* current page URL when auto-connecting.
*
* The redirect-based in-app wallet login and the `SiteLink` / `SiteEmbed` components
* pass auth material via URL parameters, which `AutoConnect` reads to restore the
* session. Set this to `false` to disable reading auth material from the URL entirely
* — useful if your app only uses popup, OTP, or passkey login and never hands off a
* session between sites.
*
* @default true
*/
readUrlToken?: boolean;

/**
* @hidden
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { describe, expect, it } from "vitest";
import { TEST_CLIENT } from "~test/test-clients.js";
import { getLoginUrl } from "./getLoginPath.js";

describe("getLoginUrl", () => {
it("appends redirect params including state in redirect mode", () => {
const url = getLoginUrl({
authFlow: "link",
authOption: "google",
client: TEST_CLIENT,
mode: "redirect",
redirectUrl: "https://example.com/app",
state: "abc123",
});

const redirectUrl = decodeURIComponent(url.split("redirectUrl=")[1] ?? "");
expect(redirectUrl).toContain("walletId=inApp");
expect(redirectUrl).toContain("authProvider=google");
expect(redirectUrl).toContain("authFlow=link");
expect(redirectUrl).toContain("state=abc123");
});

it("does not append a redirect url in popup mode", () => {
const url = getLoginUrl({
authOption: "google",
client: TEST_CLIENT,
mode: "popup",
});
expect(url).not.toContain("redirectUrl=");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,19 @@ export const getLoginUrl = ({
mode = "popup",
redirectUrl,
authFlow,
state,
}: {
authOption: AuthOption;
client: ThirdwebClient;
ecosystem?: Ecosystem;
mode?: "popup" | "redirect" | "window";
redirectUrl?: string;
authFlow?: "connect" | "link";
/**
* One-time value tied to the browser session that started this flow. It is
* echoed back on the redirect and validated before the returned token is trusted.
*/
state?: string;
}) => {
if (mode === "popup" && redirectUrl) {
throw new Error("Redirect URL is not supported for popup mode");
Expand All @@ -54,6 +60,9 @@ export const getLoginUrl = ({
if (authFlow) {
formattedRedirectUrl.searchParams.set("authFlow", authFlow);
}
if (state) {
formattedRedirectUrl.searchParams.set("state", state);
}
baseUrl = `${baseUrl}&redirectUrl=${encodeURIComponent(formattedRedirectUrl.toString())}`;
}

Expand Down
6 changes: 6 additions & 0 deletions packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import type { AuthStoredTokenWithCookieReturnType } from "../../../core/authentication/types.js";
import type { Ecosystem } from "../../../core/wallet/types.js";
import { DEFAULT_POP_UP_SIZE } from "./constants.js";
import { storeRedirectState } from "./redirect-state.js";

const closeWindow = ({
isWindowOpenedByFn,
Expand Down Expand Up @@ -34,10 +35,15 @@
mode?: "redirect" | "popup" | "window";
authFlow?: "connect" | "link";
}): Promise<void> {
// Persist a one-time state bound to this browser and echo it on the redirect so
// the returned auth token can be tied back to a flow this page actually started.
const state =
options.mode === "popup" ? undefined : await storeRedirectState();

Check warning on line 41 in packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts#L40-L41

Added lines #L40 - L41 were not covered by tests
const loginUrl = getLoginUrl({
...options,
mode: options.mode || "redirect",
authFlow: options.authFlow,
state,

Check warning on line 46 in packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts#L46

Added line #L46 was not covered by tests
});
if (options.mode === "redirect") {
window.location.href = loginUrl;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { beforeEach, describe, expect, it } from "vitest";
import { consumeRedirectState, storeRedirectState } from "./redirect-state.js";

describe.runIf(typeof window !== "undefined")("redirect-state", () => {
beforeEach(() => {
window.localStorage.clear();
});

it("accepts the exact state it stored", async () => {
const state = await storeRedirectState();
expect(await consumeRedirectState(state)).toBe(true);
});

it("rejects a mismatched state", async () => {
await storeRedirectState();
expect(await consumeRedirectState("not-the-stored-state")).toBe(false);
});

it("rejects an undefined returned state", async () => {
await storeRedirectState();
expect(await consumeRedirectState(undefined)).toBe(false);
});

it("rejects when nothing was stored", async () => {
expect(await consumeRedirectState("anything")).toBe(false);
});

it("is single-use: a valid state cannot be replayed", async () => {
const state = await storeRedirectState();
expect(await consumeRedirectState(state)).toBe(true);
expect(await consumeRedirectState(state)).toBe(false);
});

it("leaves a pending state intact after a mismatched attempt", async () => {
const state = await storeRedirectState();
// a wrong attempt must NOT consume the legitimate pending state
expect(await consumeRedirectState("wrong")).toBe(false);
// ...so the real callback still validates when it returns
expect(await consumeRedirectState(state)).toBe(true);
});

it("supports concurrent flows without clobbering each other", async () => {
const first = await storeRedirectState();
const second = await storeRedirectState();
expect(first).not.toBe(second);
// both flows validate independently, in any order
expect(await consumeRedirectState(second)).toBe(true);
expect(await consumeRedirectState(first)).toBe(true);
});

it("a forged/unknown consume cannot evict other pending flows", async () => {
const first = await storeRedirectState();
const second = await storeRedirectState();
// an attacker-supplied state only ever touches its own (absent) key
expect(await consumeRedirectState("forged-state-value")).toBe(false);
// ...so both legitimate flows still validate
expect(await consumeRedirectState(first)).toBe(true);
expect(await consumeRedirectState(second)).toBe(true);
});

it("rejects an expired state", async () => {
window.localStorage.setItem(
"thirdweb:auth-redirect-state:stale",
String(Date.now() - 1000),
);
expect(await consumeRedirectState("stale")).toBe(false);
});

it("rejects a state whose stored expiry is malformed", async () => {
window.localStorage.setItem(
"thirdweb:auth-redirect-state:garbage",
"not-a-number",
);
expect(await consumeRedirectState("garbage")).toBe(false);
});

it("prunes expired states on the next store", async () => {
const staleKey = "thirdweb:auth-redirect-state:old";
window.localStorage.setItem(staleKey, String(Date.now() - 1000));
await storeRedirectState();
expect(window.localStorage.getItem(staleKey)).toBeNull();
});

it("no-ops safely when localStorage is unavailable", async () => {
const realLocalStorage = window.localStorage;
Object.defineProperty(window, "localStorage", {
configurable: true,
get() {
throw new Error("localStorage blocked");
},
});
try {
// store still returns a state, and consume reports false rather than throwing
expect(typeof (await storeRedirectState())).toBe("string");
expect(await consumeRedirectState("anything")).toBe(false);
} finally {
Object.defineProperty(window, "localStorage", {
configurable: true,
value: realLocalStorage,
writable: true,
});
}
});
});
Loading
Loading