Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 5 additions & 0 deletions .changeset/hot-donkeys-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": minor
---

Stripe URL scheme from SIWE domain across all touchpoints
Comment thread
0xFirekeeper marked this conversation as resolved.
Outdated
35 changes: 34 additions & 1 deletion packages/thirdweb/src/auth/core/create-login-message.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterAll, beforeAll, describe, expect, test, vi } from "vitest";
import { createLoginMessage } from "./create-login-message.js";
import { createLoginMessage, stripUrlScheme } from "./create-login-message.js";

describe("createLoginMessage", () => {
beforeAll(() => {
Expand Down Expand Up @@ -72,4 +72,37 @@ describe("createLoginMessage", () => {
Not Before: 1634567800"
`);
});

test("should strip URL scheme from domain in the message", () => {
const payload = {
address: "0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B",
chain_id: "1",
domain: "https://example.com",
expiration_time: "1634567990",
invalid_before: "1634567800",
issued_at: "1634567890",
nonce: "123456",
statement: "This is a statement",
uri: "https://example.com",
version: "1.0",
};

const result = createLoginMessage(payload);
expect(result).toContain(
"example.com wants you to sign in with your Ethereum account:",
);
expect(result).not.toContain("https://example.com wants you to sign in");
});

test("stripUrlScheme should strip https scheme", () => {
expect(stripUrlScheme("https://example.com")).toBe("example.com");
});

test("stripUrlScheme should strip http scheme", () => {
expect(stripUrlScheme("http://example.com")).toBe("example.com");
});

test("stripUrlScheme should leave bare domains unchanged", () => {
expect(stripUrlScheme("example.com")).toBe("example.com");
});
});
12 changes: 11 additions & 1 deletion packages/thirdweb/src/auth/core/create-login-message.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import type { LoginPayload } from "./types.js";

/**
* Strips the URL scheme (e.g. "https://") from a domain string.
* Per EIP-4361, the domain field should be an RFC 3986 authority (host without scheme).
* @internal
*/
export function stripUrlScheme(domain: string): string {
return domain.replace(/^https?:\/\//, "");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The regex pattern is case-sensitive and will fail to strip uppercase scheme variants like HTTP:// or HTTPS://. According to RFC 3986, URI schemes are case-insensitive. This will cause EIP-4361 non-compliance when users provide domains with uppercase schemes.

Impact: Domains like "HTTP://example.com" will not be stripped, violating EIP-4361 which requires the domain field to be an authority without scheme.

Fix: Make the regex case-insensitive:

export function stripUrlScheme(domain: string): string {
  return domain.replace(/^https?:\/\//i, "");
}
Suggested change
export function stripUrlScheme(domain: string): string {
return domain.replace(/^https?:\/\//, "");
}
export function stripUrlScheme(domain: string): string {
return domain.replace(/^https?:\/\//i, "");
}

Spotted by Graphite

Fix in Graphite


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

Comment thread
0xFirekeeper marked this conversation as resolved.
Outdated

/**
* Create an EIP-4361 & CAIP-122 compliant message to sign based on the login payload
* @param payload - The login payload containing the necessary information.
Expand All @@ -8,7 +17,8 @@ import type { LoginPayload } from "./types.js";
*/
export function createLoginMessage(payload: LoginPayload): string {
const typeField = "Ethereum";
const header = `${payload.domain} wants you to sign in with your ${typeField} account:`;
const domain = stripUrlScheme(payload.domain);
const header = `${domain} wants you to sign in with your ${typeField} account:`;
let prefix = [header, payload.address].join("\n");
prefix = [prefix, payload.statement].join("\n\n");
if (payload.statement) {
Expand Down
25 changes: 25 additions & 0 deletions packages/thirdweb/src/auth/core/generate-login-payload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,29 @@ describe("generateLoginPayload", () => {
}
`);
});

test("should strip URL scheme from domain", async () => {
const options = {
client: TEST_CLIENT,
domain: "https://example.com",
login: {
nonce: {
generate() {
return "20cd4ddb-6857-4d36-8e44-9f6e026b8de9";
},
validate(uuid: string) {
return uuid === "20cd4ddb-6857-4d36-8e44-9f6e026b8de9";
},
},
},
};

const generatePayload = generateLoginPayload(options);
const result = await generatePayload({
address: "0xAb5801a7D398351b8bE11C439e05C5B3259aeC9B",
chainId: 1,
});

expect(result.domain).toBe("example.com");
});
});
3 changes: 2 additions & 1 deletion packages/thirdweb/src/auth/core/generate-login-payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
DEFAULT_LOGIN_STATEMENT,
DEFAULT_LOGIN_VERSION,
} from "./constants.js";
import { stripUrlScheme } from "./create-login-message.js";
import type { AuthOptions, LoginPayload } from "./types.js";

/**
Expand Down Expand Up @@ -31,7 +32,7 @@ export function generateLoginPayload(options: AuthOptions) {
return {
address,
chain_id: chainId ? chainId.toString() : undefined,
domain: options.domain,
domain: stripUrlScheme(options.domain),
expiration_time: new Date(now + expirationTime).toISOString(),
invalid_before: new Date(now - expirationTime).toISOString(),
issued_at: new Date(now).toISOString(),
Expand Down
42 changes: 42 additions & 0 deletions packages/thirdweb/src/auth/core/verify-login-payload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,46 @@ describe("verifyLoginPayload", () => {

expect(verificationResult.valid).toBe(false);
});

test("should work when domain has URL scheme (backward compat)", async () => {
const options = {
client: TEST_CLIENT,
domain: "https://example.com",
login: {
nonce: {
generate() {
return "20cd4ddb-6857-4d36-8e44-9f6e026b8de9";
},
validate(uuid: string) {
return uuid === "20cd4ddb-6857-4d36-8e44-9f6e026b8de9";
},
},
payloadExpirationTimeSeconds: 3600,
statement: "This is a statement",
},
};

const generatePayload = generateLoginPayload(options);
const payloadToSign = await generatePayload({
address: TEST_ACCOUNT_A.address,
});

// sign the payload
const signatureResult = await signLoginPayload({
account: TEST_ACCOUNT_A,
payload: payloadToSign,
});

// verify the payload
const verifyPayload = verifyLoginPayload(options);

const verificationResult = await verifyPayload(signatureResult);

expect(verificationResult.valid).toBe(true);
if (verificationResult.valid) {
expect(verificationResult.payload.address).toBe(TEST_ACCOUNT_A.address);
// domain in payload should have scheme stripped
expect(verificationResult.payload.domain).toBe("example.com");
}
});
Comment thread
0xFirekeeper marked this conversation as resolved.
});
5 changes: 3 additions & 2 deletions packages/thirdweb/src/auth/core/verify-login-payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { trackLogin } from "../../analytics/track/siwe.js";
import { getCachedChain } from "../../chains/utils.js";
import { verifySignature } from "../verify-signature.js";
import { DEFAULT_LOGIN_STATEMENT, DEFAULT_LOGIN_VERSION } from "./constants.js";
import { createLoginMessage } from "./create-login-message.js";
import { createLoginMessage, stripUrlScheme } from "./create-login-message.js";
import type { AuthOptions, LoginPayload } from "./types.js";

/**
Expand Down Expand Up @@ -48,7 +48,8 @@ export function verifyLoginPayload(options: AuthOptions) {
signature,
}: VerifyLoginPayloadParams): Promise<VerifyLoginPayloadResult> => {
// check that the intended domain matches the domain of the payload
if (payload.domain !== options.domain) {
// normalize both sides by stripping URL scheme for backward compatibility
if (stripUrlScheme(payload.domain) !== stripUrlScheme(options.domain)) {
Comment thread
0xFirekeeper marked this conversation as resolved.
return {
error: `Expected domain '${options.domain}' does not match domain on payload '${payload.domain}'`,
valid: false,
Expand Down
Loading