fix: consistently treat returnTo parameter as an absolute path (#2185)

Author: guabu

src/server/auth-client.test.ts

@@ -1552,7 +1552,7 @@ ca/T0LLtgmbMmxSv/MmzIg==
           codeVerifier: expect.any(String),
           responseType: "code",
           state: authorizationUrl.searchParams.get("state"),
-          returnTo: "https://example.com/dashboard"
+          returnTo: "/dashboard"
         })
       );
     });
@@ -4851,8 +4851,27 @@ ca/T0LLtgmbMmxSv/MmzIg==
       // Mock the transactionStore.save method to verify the saved state
       const originalSave = authClient["transactionStore"].save;
       authClient["transactionStore"].save = vi.fn(async (cookies, state) => {
-        // The full URL is saved, not just the path
-        expect(state.returnTo).toBe("https://example.com/custom-return-path");
+        expect(state.returnTo).toBe("/custom-return-path");
+        return originalSave.call(
+          authClient["transactionStore"],
+          cookies,
+          state
+        );
+      });
+
+      await authClient.startInteractiveLogin({ returnTo });
+
+      expect(authClient["transactionStore"].save).toHaveBeenCalled();
+    });
+
+    it("should sanitize and use the provided returnTo parameter — absolute URL", async () => {
+      const authClient = await createAuthClient();
+      const returnTo =
+        DEFAULT.appBaseUrl + "/custom-return-path?query=param#hash";
+
+      const originalSave = authClient["transactionStore"].save;
+      authClient["transactionStore"].save = vi.fn(async (cookies, state) => {
+        expect(state.returnTo).toBe("/custom-return-path?query=param#hash");
         return originalSave.call(
           authClient["transactionStore"],
           cookies,
@@ -4957,7 +4976,7 @@ ca/T0LLtgmbMmxSv/MmzIg==
               codeVerifier: expect.any(String),
               responseType: "code",
               state: expect.any(String),
-              returnTo: "https://example.com/custom-path"
+              returnTo: "/custom-path"
             })
           );
           return originalSave.call(

src/server/auth-client.ts

@@ -140,10 +140,10 @@ export interface AuthClientOptions {
   noContentProfileResponseWhenUnauthenticated?: boolean;
 }
 
-function createRouteUrl(url: string, base: string) {
+function createRouteUrl(path: string, baseUrl: string) {
   return new URL(
-    ensureNoLeadingSlash(normalizeWithBasePath(url)),
-    ensureTrailingSlash(base)
+    ensureNoLeadingSlash(normalizeWithBasePath(path)),
+    ensureTrailingSlash(baseUrl)
   );
 }
 
@@ -328,7 +328,10 @@ export class AuthClient {
       const sanitizedReturnTo = toSafeRedirect(options.returnTo, safeBaseUrl);
 
       if (sanitizedReturnTo) {
-        returnTo = sanitizedReturnTo;
+        returnTo =
+          sanitizedReturnTo.pathname +
+          sanitizedReturnTo.search +
+          sanitizedReturnTo.hash;
       }
     }
 

src/utils/url-helpers.test.ts

@@ -14,27 +14,34 @@ describe("url-helpers", () => {
     });
 
     it("should allow relative urls", () => {
-      expect(toSafeRedirect("/foo", safeBaseUrl)).toEqual(
+      expect(toSafeRedirect("/foo", safeBaseUrl)?.toString()).toEqual(
         "http://www.example.com/foo"
       );
-      expect(toSafeRedirect("foo", safeBaseUrl)).toEqual(
+      expect(toSafeRedirect("foo", safeBaseUrl)?.toString()).toEqual(
         "http://www.example.com/foo"
       );
-      expect(toSafeRedirect("/foo?some=value", safeBaseUrl)).toEqual(
-        "http://www.example.com/foo?some=value"
-      );
       expect(
-        toSafeRedirect("/foo?someUrl=https://www.google.com", safeBaseUrl)
+        toSafeRedirect("/foo?some=value", safeBaseUrl)?.toString()
+      ).toEqual("http://www.example.com/foo?some=value");
+      expect(
+        toSafeRedirect(
+          "/foo?someUrl=https://www.google.com",
+          safeBaseUrl
+        )?.toString()
       ).toEqual("http://www.example.com/foo?someUrl=https://www.google.com");
       expect(
-        toSafeRedirect("/foo", new URL("http://www.example.com:8888"))
+        toSafeRedirect(
+          "/foo",
+          new URL("http://www.example.com:8888")
+        )?.toString()
       ).toEqual("http://www.example.com:8888/foo");
     });
 
     it("should prevent open redirects", () => {
       for (const payload of payloads) {
         expect(
-          toSafeRedirect(payload, safeBaseUrl) || safeBaseUrl.toString()
+          toSafeRedirect(payload, safeBaseUrl)?.toString() ||
+            safeBaseUrl.toString()
         ).toMatch(/^http:\/\/www\.example\.com\//);
       }
     });

src/utils/url-helpers.ts

@@ -1,7 +1,7 @@
 export function toSafeRedirect(
   dangerousRedirect: string,
   safeBaseUrl: URL
-): string | undefined {
+): URL | undefined {
   let url: URL;
 
   try {
@@ -11,7 +11,7 @@ export function toSafeRedirect(
   }
 
   if (url.origin === safeBaseUrl.origin) {
-    return url.toString();
+    return url;
   }
 
   return undefined;