ronantakizawa/codereview-bench huggingface dataset:
Repository: better-auth/better-auth
File: packages/better-auth/src/client/config.ts (TypeScript)

Code before:
	const $store = {
		notify: (
			signal?: (Omit<string, "$sessionSignal"> | "$sessionSignal") | undefined,
		) => {
			pluginsAtoms[signal as keyof typeof pluginsAtoms]!.set(
				!pluginsAtoms[signal as keyof typeof pluginsAtoms]!.get(),
			);
		},
		listen: (
			signal: Omit<string, "$sessionSignal"> | "$sessionSignal",
			listener: (value: boolean, oldValue?: boolean | undefined) => void,
		) => {
			pluginsAtoms[signal as keyof typeof pluginsAtoms]!.subscribe(listener);
		},
		atoms: pluginsAtoms,
	};

	for (const plugin of plugins) {
		if (plugin.getActions) {
			Object.assign(
				pluginsActions,
				plugin.getActions?.($fetch, $store, options),
			);
		}
	}
	return {
		get baseURL() {
			return baseURL;
		},
		pluginsActions,
		pluginsAtoms,
		pluginPathMethods,
		atomListeners,
		$fetch,
		$store,
	};
};


Review comment (suggestion):
I don't think it's ideal to deep merge each entry. Instead we should do a single merge:

```ts
const pluginsActions = defu(
	{},
	...plugins.map((p) => p.getActions?.($fetch, $store, options)),
) as Record<string, any>;
```

Since defu reads from right to left, this will also make the last plugin win for conflicting fields (expected behavior), instead of the first one (current behavior).

Code after:

	const $store = {
		notify: (
			signal?: (Omit<string, "$sessionSignal"> | "$sessionSignal") | undefined,
		) => {
			pluginsAtoms[signal as keyof typeof pluginsAtoms]!.set(
				!pluginsAtoms[signal as keyof typeof pluginsAtoms]!.get(),
			);
		},
		listen: (
			signal: Omit<string, "$sessionSignal"> | "$sessionSignal",
			listener: (value: boolean, oldValue?: boolean | undefined) => void,
		) => {
			pluginsAtoms[signal as keyof typeof pluginsAtoms]!.subscribe(listener);
		},
		atoms: pluginsAtoms,
	};

	for (const plugin of plugins) {
		if (plugin.getActions) {
			pluginsActions = defu(
				plugin.getActions?.($fetch, $store, options) ?? {},
				pluginsActions,
			);
		}
	}
	return {
		get baseURL() {
			return baseURL;
		},
		pluginsActions,
		pluginsAtoms,
		pluginPathMethods,
		atomListeners,
		$fetch,
		$store,
	};
};
Repository: better-auth/better-auth
File: packages/sso/src/saml.test.ts (TypeScript)

Code before:
			onSuccess: async (context) => {
				samlResponse = await context.data;
			},
		});

		const response = await auth.handler(
			new Request(
				"http://localhost:3000/api/auth/sso/saml2/callback/single-assertion-provider",
				{
					method: "POST",
					headers: {
						"Content-Type": "application/x-www-form-urlencoded",
					},
					body: new URLSearchParams({
						SAMLResponse: samlResponse.samlResponse,
						RelayState: "http://localhost:3000/dashboard",
					}),
				},
			),
		);

		expect(response.status).toBe(302);
		expect(response.headers.get("location")).not.toContain("error");
	});
});


Review comment (security):
I think we could benefit from enhacing this test, something like 
```suggestion
const session = await auth.api.getSession({ headers: getCookies(response) });
expect(session.user.email).toBe("testuser@example.com"); // lowercase!
```

Code after:

		let samlResponse: any;
		await betterFetch("http://localhost:8081/api/sso/saml2/idp/post", {
			onSuccess: async (context) => {
				samlResponse = await context.data;
			},
		});

		const response = await auth.handler(
			new Request(
				"http://localhost:3000/api/auth/sso/saml2/callback/single-assertion-provider",
				{
					method: "POST",
					headers: {
						"Content-Type": "application/x-www-form-urlencoded",
					},
					body: new URLSearchParams({
						SAMLResponse: samlResponse.samlResponse,
						RelayState: "http://localhost:3000/dashboard",
					}),
				},
			),
		);

		expect(response.status).toBe(302);
		expect(response.headers.get("location")).not.toContain("error");
	});

	it("should normalize email to lowercase in SAML authentication to prevent duplicate creation", async () => {
		// Tests repeated logins with mixed-case emails to prevent duplicate user creation
		// First login with "TestUser@Example.com" should create user and second login
		// with same email should find existing user and not create duplicate
		const { auth, signInWithTestUser } = await getTestInstance({
			plugins: [sso()],
		});
		const { headers } = await signInWithTestUser();

		await auth.api.registerSSOProvider({
			body: {
				providerId: "email-case-provider",
				issuer: "http://localhost:8081",
				domain: "example.com",
				samlConfig: {
					entryPoint: "http://localhost:8081/api/sso/saml2/idp/post",
					cert: certificate,
					callbackUrl: "http://localhost:3000/dashboard",
					wantAssertionsSigned: false,
					signatureAlgorithm: "sha256",
					digestAlgorithm: "sha256",
					idpMetadata: {
						metadata: idpMetadata,
Repository: better-auth/better-auth
File: packages/expo/src/focus-manager.ts (TypeScript)

Code before:
import type { FocusListener, FocusManager } from "better-auth/client";
import { kFocusManager } from "better-auth/client";
import type { AppStateStatus } from "react-native";
import { AppState } from "react-native";

class ExpoFocusManager implements FocusManager {
	listeners = new Set<FocusListener>();
	subscription?: ReturnType<typeof AppState.addEventListener>;
	isFocused = true;

	subscribe(listener: FocusListener) {
		this.listeners.add(listener);
		return () => {
			this.listeners.delete(listener);
		};
	}

	setFocused(focused: boolean) {
		if (this.isFocused === focused) return;
		this.isFocused = focused;
		this.listeners.forEach((listener) => listener(focused));
	}

	setup() {
		this.subscription = AppState.addEventListener(
			"change",
			(state: AppStateStatus) => {
				this.setFocused(state === "active");
			},
		);

		return () => {
			this.subscription?.remove();
		};

Review comment (refactor):
Wouldn't this make it hard to distinguish the initial state from a duplicate state? How about using undefined as the initial value?

```ts
isFocused: boolean | undefined;
```

Code after:
import type { FocusListener, FocusManager } from "better-auth/client";
import { kFocusManager } from "better-auth/client";
import type { AppStateStatus } from "react-native";
import { AppState } from "react-native";

class ExpoFocusManager implements FocusManager {
	listeners = new Set<FocusListener>();
	subscription?: ReturnType<typeof AppState.addEventListener>;
	isFocused: boolean | undefined;

	subscribe(listener: FocusListener) {
		this.listeners.add(listener);
		return () => {
			this.listeners.delete(listener);
		};
	}

	setFocused(focused: boolean) {
		if (this.isFocused === focused) return;
		this.isFocused = focused;
		this.listeners.forEach((listener) => listener(focused));
	}

	setup() {
		this.subscription = AppState.addEventListener(
			"change",
			(state: AppStateStatus) => {
				this.setFocused(state === "active");
			},
		);

		return () => {
			this.subscription?.remove();
		};
Repository: better-auth/better-auth
File: packages/better-auth/src/cookies/index.ts (TypeScript)

Code before:
	ctx: GenericEndpointContext,
	session: {
		session: Session & Record<string, any>;
		user: User;
	},
	dontRememberMe: boolean,
) {
	if (!ctx.context.options.session?.cookieCache?.enabled) {
		return;
	}

	const filteredSession = filterOutputFields(
		session.session,
		ctx.context.options.session?.additionalFields,
	);

	const filteredUser = parseUserOutput(ctx.context.options, session.user);

	const versionConfig = ctx.context.options.session?.cookieCache?.version;
	let version = "1";
	if (versionConfig) {
		if (typeof versionConfig === "string") {
			version = versionConfig;
		} else if (typeof versionConfig === "function") {
			const result = versionConfig(session.session, session.user);
			version = result instanceof Promise ? await result : result;
		}
	}

	const sessionData: SessionData = {
		session: filteredSession,
		user: filteredUser,
		updatedAt: Date.now(),
		version,
	};

	const options = {
		...ctx.context.authCookies.sessionData.attributes,
		maxAge: dontRememberMe
			? undefined
			: ctx.context.authCookies.sessionData.attributes.maxAge,
	};

	const expiresAtDate = getDate(options.maxAge || 60, "sec").getTime();
	const strategy =
		ctx.context.options.session?.cookieCache?.strategy || "compact";

	let data: string;

	if (strategy === "jwe") {
		// Use JWE strategy (JSON Web Encryption) with A256CBC-HS512 + HKDF

Review comment (suggestion):
```suggestion
			version = isPromise(result) ? await result : result;
```

Code after:
export async function setCookieCache(
	ctx: GenericEndpointContext,
	session: {
		session: Session & Record<string, any>;
		user: User;
	},
	dontRememberMe: boolean,
) {
	if (!ctx.context.options.session?.cookieCache?.enabled) {
		return;
	}

	const filteredSession = filterOutputFields(
		session.session,
		ctx.context.options.session?.additionalFields,
	);

	const filteredUser = parseUserOutput(ctx.context.options, session.user);

	const versionConfig = ctx.context.options.session?.cookieCache?.version;
	let version = "1";
	if (versionConfig) {
		if (typeof versionConfig === "string") {
			version = versionConfig;
		} else if (typeof versionConfig === "function") {
			const result = versionConfig(session.session, session.user);
			version = isPromise(result) ? await result : result;
		}
	}

	const sessionData = {
		session: filteredSession,
		user: filteredUser,
		updatedAt: Date.now(),
		version,
	};

	const options = {
		...ctx.context.authCookies.sessionData.attributes,
		maxAge: dontRememberMe
			? undefined
			: ctx.context.authCookies.sessionData.attributes.maxAge,
	};

	const expiresAtDate = getDate(options.maxAge || 60, "sec").getTime();
	const strategy =
		ctx.context.options.session?.cookieCache?.strategy || "compact";

	let data: string;

	if (strategy === "jwe") {
Repository: better-auth/better-auth
File: packages/better-auth/src/cookies/index.ts (TypeScript)

Code before:
		const [name, value] = cookie.split(/=(.*)/s);
		cookieMap.set(name!, value!);
	});
	return cookieMap;
}

export type EligibleCookies = (string & {}) | (keyof BetterAuthCookies & {});

export const getSessionCookie = (
	request: Request | Headers,
	config?:
		| {
				cookiePrefix?: string;
				cookieName?: string;
				path?: string;
		  }
		| undefined,
) => {
	if (config?.cookiePrefix) {
		if (config.cookieName) {
			config.cookiePrefix = `${config.cookiePrefix}-`;
		} else {
			config.cookiePrefix = `${config.cookiePrefix}.`;
		}
	}
	const headers = request instanceof Headers ? request : request.headers;
	const cookies = headers.get("cookie");
	if (!cookies) {
		return null;
	}
	const { cookieName = "session_token", cookiePrefix = "better-auth." } =
		config || {};
	const name = `${cookiePrefix}${cookieName}`;
	const secureCookieName = `${SECURE_COOKIE_PREFIX}${name}`;
	const parsedCookie = parseCookies(cookies);
	const sessionToken =
		parsedCookie.get(name) || parsedCookie.get(secureCookieName);
	if (sessionToken) {
		return sessionToken;
	}

	return null;
};

export const getCookieCache = async <
	S extends {
		session: Session & Record<string, any>;
		user: User & Record<string, any>;
		updatedAt: number;
		version?: string;
	},

Review comment (suggestion):
According to the next.js impl. Please do `request instanceof Headers` before this syntax will fix the issue

https://github.com/vercel/next.js/blob/02d8ff2cf3b3cdb0101981321ee7a31730d6a843/packages/next/src/server/web/spec-extension/adapters/headers.ts#L36-L37

Code after:
		const [name, value] = cookie.split(/=(.*)/s);
		cookieMap.set(name!, value!);
	});
	return cookieMap;
}

export type EligibleCookies = (string & {}) | (keyof BetterAuthCookies & {});

export const getSessionCookie = (
	request: Request | Headers,
	config?:
		| {
				cookiePrefix?: string;
				cookieName?: string;
				path?: string;
		  }
		| undefined,
) => {
	if (config?.cookiePrefix) {
		if (config.cookieName) {
			config.cookiePrefix = `${config.cookiePrefix}-`;
		} else {
			config.cookiePrefix = `${config.cookiePrefix}.`;
		}
	}
	const headers =
		request instanceof Headers || !("headers" in request)
			? request
			: request.headers;
	const cookies = headers.get("cookie");
	if (!cookies) {
		return null;
	}
	const { cookieName = "session_token", cookiePrefix = "better-auth." } =
		config || {};
	const name = `${cookiePrefix}${cookieName}`;
	const secureCookieName = `${SECURE_COOKIE_PREFIX}${name}`;
	const parsedCookie = parseCookies(cookies);
	const sessionToken =
		parsedCookie.get(name) || parsedCookie.get(secureCookieName);
	if (sessionToken) {
		return sessionToken;
	}

	return null;
};

export const getCookieCache = async <
	S extends {
		session: Session & Record<string, any>;
		user: User & Record<string, any>;
Repository: better-auth/better-auth
File: packages/better-auth/src/plugins/organization/routes/crud-access-control.ts (TypeScript)

Code before:
						},
						condition,
					],
				});
			if (!existingRoleInDB) {
				ctx.context.logger.error(
					`[Dynamic Access Control] The role name/id does not exist in the database.`,
					{
						...("roleName" in ctx.body
							? { roleName: ctx.body.roleName }
							: { roleId: ctx.body.roleId }),
						organizationId,
					},
				);
				throw APIError.from(
					"BAD_REQUEST",
					ORGANIZATION_ERROR_CODES.ROLE_NOT_FOUND,
				);
			}

			existingRoleInDB.permission = JSON.parse(
				existingRoleInDB.permission as never as string,
			);

			// Check if any members are assigned to this role
			const membersInOrg = await ctx.context.adapter.findMany<Member>({
				model: "member",
				where: [
					{
						field: "organizationId",
						value: organizationId,
						operator: "eq",
					},
				],
			});
			const roleToDelete = existingRoleInDB.role;
			const memberWithRole = membersInOrg.find((member) => {
				const memberRoles = member.role.split(",").map((r) => r.trim());
				return memberRoles.includes(roleToDelete);
			});
			if (memberWithRole) {
				ctx.context.logger.error(
					`[Dynamic Access Control] Cannot delete a role that is assigned to members.`,
					{
						role: existingRoleInDB.role,
						organizationId,
					},
				);
				throw APIError.from(
					"BAD_REQUEST",
					ORGANIZATION_ERROR_CODES.ROLE_IS_ASSIGNED_TO_MEMBERS,

Review comment (performance):
I think this would have performance when there are many members. Can we use like foreign key to avoid the deletion?

Code after:
						},
						condition,
					],
				});
			if (!existingRoleInDB) {
				ctx.context.logger.error(
					`[Dynamic Access Control] The role name/id does not exist in the database.`,
					{
						...("roleName" in ctx.body
							? { roleName: ctx.body.roleName }
							: { roleId: ctx.body.roleId }),
						organizationId,
					},
				);
				throw APIError.from(
					"BAD_REQUEST",
					ORGANIZATION_ERROR_CODES.ROLE_NOT_FOUND,
				);
			}

			existingRoleInDB.permission = JSON.parse(
				existingRoleInDB.permission as never as string,
			);

			// Check if any members are assigned to this role
			const roleToDelete = existingRoleInDB.role;
			const members = await ctx.context.adapter.findMany<Member>({
				model: "member",
				where: [
					{
						field: "organizationId",
						value: organizationId,
						operator: "eq",
						connector: "AND",
					},
					{
						field: "role",
						value: roleToDelete,
						operator: "contains",
					},
				],
			});
			const memberWithRole = members.find((member) => {
				const memberRoles = member.role.split(",").map((r) => r.trim());
				return memberRoles.includes(roleToDelete);
			});
			if (memberWithRole) {
				ctx.context.logger.error(
					`[Dynamic Access Control] Cannot delete a role that is assigned to members.`,
					{
						role: existingRoleInDB.role,
Repository: better-auth/better-auth
File: packages/better-auth/src/api/routes/account.ts (TypeScript)

Code before:
			);

			if (hasBeenLinked) {
				return c.json({
					url: "", // this is for type inference
					status: true,
					redirect: false,
				});
			}

			const trustedProviders =
				c.context.options.account?.accountLinking?.trustedProviders;

			const isTrustedProvider = trustedProviders?.includes(provider.id);
			if (
				(!isTrustedProvider && !linkingUserInfo.user.emailVerified) ||
				c.context.options.account?.accountLinking?.enabled === false
			) {
				throw APIError.from("UNAUTHORIZED", {
					message: "Account not linked - linking not allowed",
					code: "LINKING_NOT_ALLOWED",
				});
			}

			if (
				linkingUserInfo.user.email?.toLowerCase() !== session.user.email &&
				c.context.options.account?.accountLinking?.allowDifferentEmails !== true
			) {
				throw APIError.from("UNAUTHORIZED", {
					message: "Account not linked - different emails not allowed",
					code: "LINKING_DIFFERENT_EMAILS_NOT_ALLOWED",
				});
			}

			try {
				await c.context.internalAdapter.createAccount({
					userId: session.user.id,
					providerId: provider.id,
					accountId: linkingUserId,
					accessToken: c.body.idToken.accessToken,
					idToken: token,
					refreshToken: c.body.idToken.refreshToken,
					scope: c.body.idToken.scopes?.join(","),
				});
			} catch (_e: any) {
				throw APIError.from("EXPECTATION_FAILED", {
					message: "Account not linked - unable to create account",
					code: "LINKING_FAILED",
				});
			}


Review comment (suggestion):
```suggestion
				linkingUserInfo.user.email?.toLowerCase() !== session.user.email.toLowerCase() &&
```

Code after:
			);

			if (hasBeenLinked) {
				return c.json({
					url: "", // this is for type inference
					status: true,
					redirect: false,
				});
			}

			const trustedProviders =
				c.context.options.account?.accountLinking?.trustedProviders;

			const isTrustedProvider = trustedProviders?.includes(provider.id);
			if (
				(!isTrustedProvider && !linkingUserInfo.user.emailVerified) ||
				c.context.options.account?.accountLinking?.enabled === false
			) {
				throw APIError.from("UNAUTHORIZED", {
					message: "Account not linked - linking not allowed",
					code: "LINKING_NOT_ALLOWED",
				});
			}

			if (
				linkingUserInfo.user.email?.toLowerCase() !==
					session.user.email.toLowerCase() &&
				c.context.options.account?.accountLinking?.allowDifferentEmails !== true
			) {
				throw APIError.from("UNAUTHORIZED", {
					message: "Account not linked - different emails not allowed",
					code: "LINKING_DIFFERENT_EMAILS_NOT_ALLOWED",
				});
			}

			try {
				await c.context.internalAdapter.createAccount({
					userId: session.user.id,
					providerId: provider.id,
					accountId: linkingUserId,
					accessToken: c.body.idToken.accessToken,
					idToken: token,
					refreshToken: c.body.idToken.refreshToken,
					scope: c.body.idToken.scopes?.join(","),
				});
			} catch (_e: any) {
				throw APIError.from("EXPECTATION_FAILED", {
					message: "Account not linked - unable to create account",
					code: "LINKING_FAILED",
				});
			}
Repository: better-auth/better-auth
File: packages/better-auth/src/oauth2/link-account.test.ts (TypeScript)

Code before:

		const userId = signUpRes.data!.user.id;

		// Manually set emailVerified to true
		await ctx.adapter.update({
			model: "user",
			where: [{ field: "id", value: userId }],
			update: { emailVerified: true },
		});

		// Link with Google account (also verified)
		await linkGoogleAccount();

		// Verify email remains verified
		const user = await ctx.adapter.findOne<User>({
			model: "user",
			where: [{ field: "id", value: userId }],
		});
		expect(user?.emailVerified).toBe(true);
	});
});

describe("oauth2 - account linking with case insensitive email", async () => {
	const { auth, client, cookieSetter } = await getTestInstance({
		socialProviders: {
			google: {
				clientId: "test",
				clientSecret: "test",
				enabled: true,
				verifyIdToken: async () => true,
			},
		},
		emailAndPassword: {
			enabled: true,
		},
		account: {
			accountLinking: {
				enabled: true,
				trustedProviders: ["google"],
			},
		},
	});

	const ctx = await auth.$context;

	it("should link account when email casing differs using callback", async () => {
		const testEmail = "casing-test@example.com";
		const googleEmail = "Casing-Test@Example.com";

		// Create user with lowercase email
		const sessionHeaders = new Headers();

Review comment (security):
Could you add this comment on the describe block? 🙏
```ts
/**
 * @see https://github.com/better-auth/better-auth/issues/7806
 */
```

Code after:

		const userId = signUpRes.data!.user.id;

		// Manually set emailVerified to true
		await ctx.adapter.update({
			model: "user",
			where: [{ field: "id", value: userId }],
			update: { emailVerified: true },
		});

		// Link with Google account (also verified)
		await linkGoogleAccount();

		// Verify email remains verified
		const user = await ctx.adapter.findOne<User>({
			model: "user",
			where: [{ field: "id", value: userId }],
		});
		expect(user?.emailVerified).toBe(true);
	});
});

/**
 * @see https://github.com/better-auth/better-auth/issues/7806
 */
describe("oauth2 - account linking with case insensitive email", async () => {
	const { auth, client, cookieSetter } = await getTestInstance({
		socialProviders: {
			google: {
				clientId: "test",
				clientSecret: "test",
				enabled: true,
				verifyIdToken: async () => true,
			},
		},
		emailAndPassword: {
			enabled: true,
		},
		account: {
			accountLinking: {
				enabled: true,
				trustedProviders: ["google"],
			},
		},
	});

	const ctx = await auth.$context;

	it("should link account when email casing differs using callback", async () => {
		const testEmail = "casing-test@example.com";
		const googleEmail = "Casing-Test@Example.com";
Repository: better-auth/better-auth
File: packages/better-auth/src/api/routes/update-session.ts (TypeScript)

Code before:
												type: "object",
												$ref: "#/components/schemas/Session",
											},
										},
									},
								},
							},
						},
					},
				},
			},
		},
		async (ctx) => {
			const body = ctx.body as Record<string, any>;

			if (typeof body !== "object" || Array.isArray(body)) {
				throw APIError.from(
					"BAD_REQUEST",
					BASE_ERROR_CODES.BODY_MUST_BE_AN_OBJECT,
				);
			}

			const coreFieldsInBody = Object.keys(body).filter((key) =>
				CORE_SESSION_FIELDS.has(key),
			);
			if (coreFieldsInBody.length > 0) {
				throw APIError.fromStatus("BAD_REQUEST", {
					message: `Cannot update core session field(s): ${coreFieldsInBody.join(", ")}`,
				});
			}

			const session = ctx.context.session;
			const additionalFields = parseSessionInput(
				ctx.context.options,
				body,
				"update",
			);

			if (Object.keys(additionalFields).length === 0) {
				throw APIError.fromStatus("BAD_REQUEST", {
					message: "No fields to update",
				});
			}

			const updatedSession = await ctx.context.internalAdapter.updateSession(
				session.session.token,
				{
					...additionalFields,
					updatedAt: new Date(),
				},
			);

Review comment (security):
We should use `getAuthSchema` and check if a field has `input: false` or not, this will help with plugin-added fields that should be protected too.

Code after:
		},
		async (ctx) => {
			const body = ctx.body as Record<string, any>;

			if (typeof body !== "object" || Array.isArray(body)) {
				throw APIError.from(
					"BAD_REQUEST",
					BASE_ERROR_CODES.BODY_MUST_BE_AN_OBJECT,
				);
			}

			const session = ctx.context.session;
			const additionalFields = parseSessionInput(
				ctx.context.options,
				body,
				"update",
			);

			if (Object.keys(additionalFields).length === 0) {
				throw APIError.fromStatus("BAD_REQUEST", {
					message: "No fields to update",
				});
			}

			const updatedSession = await ctx.context.internalAdapter.updateSession(
				session.session.token,
				{
					...additionalFields,
					updatedAt: new Date(),
				},
			);

			const newSession = updatedSession ?? {
				...session.session,
				...additionalFields,
				updatedAt: new Date(),
			};

			await setSessionCookie(ctx, {
				session: newSession,
				user: session.user,
			});

			return ctx.json({
				session: parseSessionOutput(ctx.context.options, newSession),
			});
		},
	);