Skip to content

Commit

Permalink
fix: prevent open redirect with returnTo (#1897)
Browse files Browse the repository at this point in the history
  • Loading branch information
nandan-bhat authored Feb 12, 2025
2 parents a56eafe + 98e4723 commit 8c0ee57
Show file tree
Hide file tree
Showing 6 changed files with 626 additions and 35 deletions.
47 changes: 15 additions & 32 deletions e2e/test-app/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 45 additions & 1 deletion src/server/auth-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,51 @@ ca/T0LLtgmbMmxSv/MmzIg==
codeVerifier: expect.any(String),
responseType: "code",
state: authorizationUrl.searchParams.get("state"),
returnTo: "/dashboard"
returnTo: "https://example.com/dashboard"
});
});

it("should prevent open redirects originating from the returnTo parameter", async () => {
const secret = await generateSecret(32);
const transactionStore = new TransactionStore({
secret
});
const sessionStore = new StatelessSessionStore({
secret
});
const authClient = new AuthClient({
transactionStore,
sessionStore,

domain: DEFAULT.domain,
clientId: DEFAULT.clientId,
clientSecret: DEFAULT.clientSecret,

secret,
appBaseUrl: DEFAULT.appBaseUrl,

fetch: getMockAuthorizationServer()
});
const loginUrl = new URL("/auth/login", DEFAULT.appBaseUrl);
loginUrl.searchParams.set("returnTo", "https://google.com");
const request = new NextRequest(loginUrl, {
method: "GET"
});

const response = await authClient.handleLogin(request);
const authorizationUrl = new URL(response.headers.get("Location")!);

// transaction state
const transactionCookie = response.cookies.get(
`__txn_${authorizationUrl.searchParams.get("state")}`
);
expect(transactionCookie).toBeDefined();
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
nonce: authorizationUrl.searchParams.get("nonce"),
codeVerifier: expect.any(String),
responseType: "code",
state: authorizationUrl.searchParams.get("state"),
returnTo: "/"
});
});

Expand Down
17 changes: 15 additions & 2 deletions src/server/auth-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
SdkError
} from "../errors";
import { LogoutToken, SessionData, TokenSet } from "../types";
import { toSafeRedirect } from "../utils/url-helpers";
import { AbstractSessionStore } from "./session/abstract-session-store";
import { TransactionState, TransactionStore } from "./transaction-store";
import { filterClaims } from "./user";
Expand Down Expand Up @@ -261,8 +262,20 @@ export class AuthClient {

async handleLogin(req: NextRequest): Promise<NextResponse> {
const redirectUri = new URL(this.routes.callback, this.appBaseUrl); // must be registed with the authorization server
const returnTo =
req.nextUrl.searchParams.get("returnTo") || this.signInReturnToPath;
const dangerousReturnTo = req.nextUrl.searchParams.get("returnTo");
let returnTo = this.signInReturnToPath;

if (dangerousReturnTo) {
const safeBaseUrl = new URL(
(this.authorizationParameters.redirect_uri as string | undefined) ||
this.appBaseUrl
);
const sanitizedReturnTo = toSafeRedirect(dangerousReturnTo, safeBaseUrl);

if (sanitizedReturnTo) {
returnTo = sanitizedReturnTo;
}
}

const codeChallengeMethod = "S256";
const codeVerifier = oauth.generateRandomCodeVerifier();
Expand Down
Loading

0 comments on commit 8c0ee57

Please sign in to comment.