fix(oauth-provider): return JSON redirects from post-login OAuth continuation (#8815)

This commit is contained in:
Gustavo Valverde
2026-03-28 20:05:29 +01:00
committed by GitHub
parent c25eb2f39b
commit 8afe2a7cea
4 changed files with 577 additions and 38 deletions

View File

@@ -175,13 +175,15 @@ export async function authorizeEndpoint(
});
if (!query.client_id) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(ctx, "invalid_client", "client_id is required"),
);
}
if (!query.response_type) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(ctx, "invalid_request", "response_type is required"),
);
}
@@ -191,7 +193,8 @@ export async function authorizeEndpoint(
: undefined;
const promptNone = promptSet?.has("none") ?? false;
if (promptSet?.has("select_account") && !opts.selectAccount?.page) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(
ctx,
`unsupported_prompt_select_account`,
@@ -201,7 +204,8 @@ export async function authorizeEndpoint(
}
if (!(query.response_type === "code")) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(
ctx,
"unsupported_response_type",
@@ -213,12 +217,14 @@ export async function authorizeEndpoint(
// Check client
const client = await getClient(ctx, opts, query.client_id);
if (!client) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(ctx, "invalid_client", "client_id is required"),
);
}
if (client.disabled) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(ctx, "client_disabled", "client is disabled"),
);
}
@@ -227,7 +233,8 @@ export async function authorizeEndpoint(
(url) => url === query.redirect_uri,
);
if (!redirectUri || !query.redirect_uri) {
throw ctx.redirect(
return handleRedirect(
ctx,
getErrorURL(ctx, "invalid_redirect", "invalid redirect uri"),
);
}
@@ -240,7 +247,8 @@ export async function authorizeEndpoint(
return !validScopes?.has(scope);
});
if (invalidScopes.length) {
throw ctx.redirect(
return handleRedirect(
ctx,
formatErrorURL(
query.redirect_uri,
"invalid_scope",
@@ -263,7 +271,8 @@ export async function authorizeEndpoint(
// Validate PKCE parameters if required
if (pkceRequired) {
if (!query.code_challenge || !query.code_challenge_method) {
throw ctx.redirect(
return handleRedirect(
ctx,
formatErrorURL(
query.redirect_uri,
"invalid_request",
@@ -279,7 +288,8 @@ export async function authorizeEndpoint(
if (query.code_challenge || query.code_challenge_method) {
// Both parameters must be provided together
if (!query.code_challenge || !query.code_challenge_method) {
throw ctx.redirect(
return handleRedirect(
ctx,
formatErrorURL(
query.redirect_uri,
"invalid_request",
@@ -293,7 +303,8 @@ export async function authorizeEndpoint(
// Check code challenge method is supported (only S256)
const codeChallengesSupported = ["S256"];
if (!codeChallengesSupported.includes(query.code_challenge_method)) {
throw ctx.redirect(
return handleRedirect(
ctx,
formatErrorURL(
query.redirect_uri,
"invalid_request",

View File

@@ -57,6 +57,7 @@ async function created(
});
}
const query = new URLSearchParams(_query);
ctx.headers?.set("accept", "application/json");
ctx.query = deleteFromPrompt(query, "create");
const { url } = await authorizeEndpoint(ctx, opts);
return {

View File

@@ -30,6 +30,19 @@ import { oauthProviderResourceClient } from "./client-resource";
import { oauthProvider } from "./oauth";
import type { OAuthClient } from "./types/oauth";
function isRedirectResult(
value: unknown,
): value is { redirect: boolean; url: string } {
return (
typeof value === "object" &&
value !== null &&
"redirect" in value &&
typeof value.redirect === "boolean" &&
"url" in value &&
typeof value.url === "string"
);
}
describe("oauth - init", () => {
it("should fail without the jwt plugin", async () => {
await expect(
@@ -304,22 +317,20 @@ describe("oauth", async () => {
vi.unstubAllGlobals();
});
let signInEmailRedirectUri = "";
await authClient.signIn.email(
const signInResponse = await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
onResponse(ctx) {
signInEmailRedirectUri = ctx.response.headers.get("Location") || "";
},
throw: true,
},
);
expect(signInEmailRedirectUri).toContain(rpBaseUrl);
expect(signInResponse.redirect).toBe(true);
expect(signInResponse.url).toContain(rpBaseUrl);
let callbackUrl = "";
await client.$fetch(signInEmailRedirectUri!, {
await client.$fetch(signInResponse.url, {
method: "GET",
headers,
onError(context) {
@@ -401,6 +412,410 @@ describe("oauth", async () => {
expect(signInResponse.url).not.toContain(`${authServerBaseUrl}/login`);
});
/**
* @see https://github.com/better-auth/better-auth/issues/7041
*/
it("should return JSON redirect after sign-in without Sec-Fetch-Mode header", async ({
onTestFinished,
}) => {
if (!oauthClient?.client_id || !oauthClient?.client_secret) {
throw Error("beforeAll not run properly");
}
const { customFetchImpl: customFetchImplRP } = await createTestInstance();
const client = createAuthClient({
plugins: [genericOAuthClient()],
baseURL: rpBaseUrl,
fetchOptions: {
customFetchImpl: customFetchImplRP,
},
});
const headers = new Headers();
const data = await client.signIn.oauth2(
{
providerId,
callbackURL: "/success",
},
{
throw: true,
onSuccess: cookieSetter(headers),
},
);
let loginRedirectUri = "";
await authClient.$fetch(data.url, {
method: "GET",
onError(ctx) {
loginRedirectUri = ctx.response.headers.get("Location") || "";
},
});
expect(loginRedirectUri).toContain("/login");
vi.stubGlobal("window", {
location: {
href: "",
search: new URL(loginRedirectUri, authServerBaseUrl).search,
},
});
onTestFinished(() => {
vi.unstubAllGlobals();
});
// Sign in WITHOUT Sec-Fetch-Mode: cors header.
// The after-hook must still return JSON (not a 302 redirect)
// to avoid CORS errors when fetch follows cross-origin redirects.
let signInLocationHeader = "";
const signInResponse = await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
throw: true,
onResponse(ctx) {
signInLocationHeader = ctx.response.headers.get("Location") || "";
},
},
);
expect(signInLocationHeader).toBe("");
expect(signInResponse.redirect).toBe(true);
expect(signInResponse.url).toContain(
`${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`,
);
});
/**
* @see https://github.com/better-auth/better-auth/issues/7041
*/
it("should still return 302 redirect for navigate-mode requests (form submissions)", async ({
onTestFinished,
}) => {
if (!oauthClient?.client_id || !oauthClient?.client_secret) {
throw Error("beforeAll not run properly");
}
const { customFetchImpl: customFetchImplRP } = await createTestInstance();
const client = createAuthClient({
plugins: [genericOAuthClient()],
baseURL: rpBaseUrl,
fetchOptions: {
customFetchImpl: customFetchImplRP,
},
});
const headers = new Headers();
const data = await client.signIn.oauth2(
{
providerId,
callbackURL: "/success",
},
{
throw: true,
onSuccess: cookieSetter(headers),
},
);
let loginRedirectUri = "";
await authClient.$fetch(data.url, {
method: "GET",
onError(ctx) {
loginRedirectUri = ctx.response.headers.get("Location") || "";
},
});
expect(loginRedirectUri).toContain("/login");
vi.stubGlobal("window", {
location: {
href: "",
search: new URL(loginRedirectUri, authServerBaseUrl).search,
},
});
onTestFinished(() => {
vi.unstubAllGlobals();
});
// Sign in with Sec-Fetch-Mode: navigate (browser form submission).
// The after-hook must NOT override Accept, so handleRedirect
// throws a 302 that the browser follows naturally.
let signInLocationHeader = "";
await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
headers: {
"Sec-Fetch-Mode": "navigate",
},
onResponse(ctx) {
signInLocationHeader = ctx.response.headers.get("Location") || "";
},
},
);
expect(signInLocationHeader).toContain(rpBaseUrl);
});
/**
* @see https://github.com/better-auth/better-auth/issues/7041
*/
it("should still return 302 redirect for html accept requests without Sec-Fetch-Mode", async ({
onTestFinished,
}) => {
if (!oauthClient?.client_id || !oauthClient?.client_secret) {
throw Error("beforeAll not run properly");
}
const { customFetchImpl: customFetchImplRP } = await createTestInstance();
const client = createAuthClient({
plugins: [genericOAuthClient()],
baseURL: rpBaseUrl,
fetchOptions: {
customFetchImpl: customFetchImplRP,
},
});
const headers = new Headers();
const data = await client.signIn.oauth2(
{
providerId,
callbackURL: "/success",
},
{
throw: true,
onSuccess: cookieSetter(headers),
},
);
let loginRedirectUri = "";
await authClient.$fetch(data.url, {
method: "GET",
onError(ctx) {
loginRedirectUri = ctx.response.headers.get("Location") || "";
},
});
expect(loginRedirectUri).toContain("/login");
vi.stubGlobal("window", {
location: {
href: "",
search: new URL(loginRedirectUri, authServerBaseUrl).search,
},
});
onTestFinished(() => {
vi.unstubAllGlobals();
});
let signInLocationHeader = "";
await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
headers: {
accept: "text/html,application/xhtml+xml",
},
onResponse(ctx) {
signInLocationHeader = ctx.response.headers.get("Location") || "";
},
},
);
expect(signInLocationHeader).toContain(rpBaseUrl);
});
/**
* @see https://github.com/better-auth/better-auth/issues/7041
*/
it("should return JSON error redirect when client is deleted during OAuth flow", async ({
onTestFinished,
}) => {
// Create a separate client that we'll delete mid-flow
const { headers: adminHeaders } = await signInWithTestUser();
const tempClient = await authorizationServer.api.adminCreateOAuthClient({
headers: adminHeaders,
body: {
redirect_uris: [redirectUri],
skip_consent: true,
},
});
expect(tempClient?.client_id).toBeDefined();
const { customFetchImpl: customFetchImplRP } = await getTestInstance({
account: {
accountLinking: { trustedProviders: [providerId] },
},
plugins: [
genericOAuth({
config: [
{
scopes: ["openid", "profile", "email"],
providerId,
redirectURI: redirectUri,
authorizationUrl: `${authServerBaseUrl}/api/auth/oauth2/authorize`,
tokenUrl: `${authServerBaseUrl}/api/auth/oauth2/token`,
userInfoUrl: `${authServerBaseUrl}/api/auth/oauth2/userinfo`,
clientId: tempClient!.client_id,
clientSecret: tempClient!.client_secret!,
pkce: true,
},
],
}),
],
});
const client = createAuthClient({
plugins: [genericOAuthClient()],
baseURL: rpBaseUrl,
fetchOptions: { customFetchImpl: customFetchImplRP },
});
const rpHeaders = new Headers();
const data = await client.signIn.oauth2(
{ providerId, callbackURL: "/success" },
{ throw: true, onSuccess: cookieSetter(rpHeaders) },
);
let loginRedirectUri = "";
await authClient.$fetch(data.url, {
method: "GET",
onError(ctx) {
loginRedirectUri = ctx.response.headers.get("Location") || "";
},
});
expect(loginRedirectUri).toContain("/login");
vi.stubGlobal("window", {
location: {
href: "",
search: new URL(loginRedirectUri, authServerBaseUrl).search,
},
});
onTestFinished(() => {
vi.unstubAllGlobals();
});
// Delete the client AFTER the authorize redirect but BEFORE sign-in
await authorizationServer.api.deleteOAuthClient({
headers: adminHeaders,
body: { client_id: tempClient!.client_id },
});
// Sign in — the after hook calls authorizeEndpoint which finds the
// client is gone. This must return a JSON error redirect, not a 302.
const signInResponse = await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
throw: true,
},
);
expect(signInResponse.redirect).toBe(true);
expect(signInResponse.url).toContain("error=invalid_client");
});
/**
* @see https://github.com/better-auth/better-auth/issues/7041
*/
it("should return JSON error redirect when client is disabled during OAuth flow", async ({
onTestFinished,
}) => {
const { headers: adminHeaders } = await signInWithTestUser();
const tempClient = await authorizationServer.api.adminCreateOAuthClient({
headers: adminHeaders,
body: {
redirect_uris: [redirectUri],
skip_consent: true,
},
});
expect(tempClient?.client_id).toBeDefined();
const { customFetchImpl: customFetchImplRP } = await getTestInstance({
account: {
accountLinking: { trustedProviders: [providerId] },
},
plugins: [
genericOAuth({
config: [
{
scopes: ["openid", "profile", "email"],
providerId,
redirectURI: redirectUri,
authorizationUrl: `${authServerBaseUrl}/api/auth/oauth2/authorize`,
tokenUrl: `${authServerBaseUrl}/api/auth/oauth2/token`,
userInfoUrl: `${authServerBaseUrl}/api/auth/oauth2/userinfo`,
clientId: tempClient!.client_id,
clientSecret: tempClient!.client_secret!,
pkce: true,
},
],
}),
],
});
const client = createAuthClient({
plugins: [genericOAuthClient()],
baseURL: rpBaseUrl,
fetchOptions: { customFetchImpl: customFetchImplRP },
});
const rpHeaders = new Headers();
const data = await client.signIn.oauth2(
{ providerId, callbackURL: "/success" },
{ throw: true, onSuccess: cookieSetter(rpHeaders) },
);
let loginRedirectUri = "";
await authClient.$fetch(data.url, {
method: "GET",
onError(ctx) {
loginRedirectUri = ctx.response.headers.get("Location") || "";
},
});
expect(loginRedirectUri).toContain("/login");
vi.stubGlobal("window", {
location: {
href: "",
search: new URL(loginRedirectUri, authServerBaseUrl).search,
},
});
onTestFinished(() => {
vi.unstubAllGlobals();
});
const context = await authorizationServer.$context;
await context.adapter.update({
model: "oauthClient",
where: [
{
field: "clientId",
value: tempClient!.client_id,
},
],
update: {
disabled: true,
},
});
const signInResponse = await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
throw: true,
},
);
expect(signInResponse.redirect).toBe(true);
expect(signInResponse.url).toContain("error=client_disabled");
});
it("should sign in using generic oauth discovery", async ({
onTestFinished,
}) => {
@@ -458,22 +873,20 @@ describe("oauth", async () => {
vi.unstubAllGlobals();
});
let signInEmailRedirectUri = "";
await authClient.signIn.email(
const signInResponse = await authClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
onResponse(ctx) {
signInEmailRedirectUri = ctx.response.headers.get("Location") || "";
},
throw: true,
},
);
expect(signInEmailRedirectUri).toContain(rpBaseUrl);
expect(signInResponse.redirect).toBe(true);
expect(signInResponse.url).toContain(rpBaseUrl);
let callbackURL = "";
await client.$fetch(signInEmailRedirectUri!, {
await client.$fetch(signInResponse.url, {
method: "GET",
headers,
onError(ctx) {
@@ -823,6 +1236,106 @@ describe("oauth - prompt", async () => {
isUserRegistered = true;
});
it("create - should return JSON redirect when continuing from setup", async ({
onTestFinished,
}) => {
isUserRegistered = false;
onTestFinished(() => {
isUserRegistered = true;
vi.unstubAllGlobals();
});
const tempClient = await authorizationServer.api.adminCreateOAuthClient({
headers,
body: {
redirect_uris: [redirectUri],
},
});
if (!tempClient?.client_id || !tempClient.client_secret) {
expect.unreachable();
}
const { customFetchImpl: customFetchImplRP, cookieSetter } =
await getTestInstance({
account: {
accountLinking: { trustedProviders: [providerId] },
},
plugins: [
genericOAuth({
config: [
{
scopes: ["openid", "profile", "email"],
providerId,
redirectURI: redirectUri,
authorizationUrl: `${authServerBaseUrl}/api/auth/oauth2/authorize`,
tokenUrl: `${authServerBaseUrl}/api/auth/oauth2/token`,
userInfoUrl: `${authServerBaseUrl}/api/auth/oauth2/userinfo`,
clientId: tempClient.client_id,
clientSecret: tempClient.client_secret,
pkce: true,
},
],
}),
],
});
const client = createAuthClient({
plugins: [genericOAuthClient()],
baseURL: rpBaseUrl,
fetchOptions: {
customFetchImpl: customFetchImplRP,
},
});
const oauthHeaders = new Headers();
const data = await client.signIn.oauth2(
{
providerId,
callbackURL: "/success",
},
{
throw: true,
onSuccess: cookieSetter(oauthHeaders),
},
);
let setupRedirectUri = "";
await serverClient.$fetch(data.url, {
method: "GET",
headers,
onError(context) {
setupRedirectUri = context.response.headers.get("Location") || "";
},
});
expect(setupRedirectUri).toContain("/setup");
vi.stubGlobal("window", {
location: {
search: new URL(setupRedirectUri, authServerBaseUrl).search,
},
});
isUserRegistered = true;
let continueLocationHeader = "";
const continueRes = await serverClient.oauth2.continue(
{
created: true,
},
{
headers,
throw: true,
onResponse(context) {
continueLocationHeader =
context.response.headers.get("Location") || "";
},
},
);
expect(continueLocationHeader).toBe("");
expect(continueRes.redirect).toBe(true);
expect(continueRes.url).toContain("/consent");
expect(continueRes.url).toContain(`client_id=${tempClient.client_id}`);
});
it("consent - should sign in", async ({ onTestFinished }) => {
if (!oauthClient?.client_id || !oauthClient?.client_secret) {
throw Error("beforeAll not run properly");
@@ -1374,20 +1887,18 @@ describe("oauth - prompt", async () => {
});
// Check for redirection to /consent after login
let signInEmailRedirectUri = "";
await serverClient.signIn.email(
const signInResponse = await serverClient.signIn.email(
{
email: testUser.email,
password: testUser.password,
},
{
onResponse(ctx) {
signInEmailRedirectUri = ctx.response.headers.get("Location") || "";
},
throw: true,
},
);
expect(signInEmailRedirectUri).toContain("/consent");
expect(signInEmailRedirectUri).toContain("prompt=consent");
expect(signInResponse.redirect).toBe(true);
expect(signInResponse.url).toContain("/consent");
expect(signInResponse.url).toContain("prompt=consent");
});
it("select_account+consent - should always redirect to select_account and force consent (notice consent previously given)", async ({
@@ -1612,21 +2123,24 @@ describe("oauth - prompt", async () => {
vi.unstubAllGlobals();
});
let consentRedirectUri = "";
// Select Account and continue auth flow
await serverClient.organization.setActive(
const setActiveResponse = await serverClient.organization.setActive(
{
organizationId: org.id,
organizationSlug: org.slug,
},
{
headers,
onResponse(context) {
consentRedirectUri = context.response.headers.get("Location") || "";
cookieSetter(headers)(context);
},
throw: true,
onSuccess: cookieSetter(headers),
},
);
expect(isRedirectResult(setActiveResponse)).toBe(true);
if (!isRedirectResult(setActiveResponse)) {
expect.unreachable();
}
expect(setActiveResponse.redirect).toBe(true);
const consentRedirectUri = setActiveResponse.url;
expect(consentRedirectUri).toContain(`/consent`);
expect(consentRedirectUri).toContain(`client_id=${oauthClient.client_id}`);
expect(consentRedirectUri).toContain(`scope=`);

View File

@@ -283,6 +283,19 @@ export const oauthProvider = <O extends OAuthOptions<Scope[]>>(options: O) => {
if (!session) return;
ctx.context.session = session;
const secFetchMode = ctx.request?.headers
?.get("sec-fetch-mode")
?.toLowerCase();
const acceptHeader =
ctx.request?.headers?.get("accept")?.toLowerCase() ?? "";
const isNavigationRequest =
secFetchMode === "navigate" ||
(!secFetchMode &&
(acceptHeader.includes("text/html") ||
acceptHeader.includes("application/xhtml+xml")));
if (!isNavigationRequest) {
ctx.headers?.set("accept", "application/json");
}
ctx.query = deleteFromPrompt(query, "login");
return await authorizeEndpoint(ctx, opts);
}),