Discard cookies when redirected away

src/index.spec.ts

@@ -3,32 +3,95 @@ import { Request, Response } from "servie/dist/node";
 import { redirects } from "./index";
 
 describe("popsicle redirects", () => {
-  const req = new Request("http://example.com/");
-  const ok = new Response(null, { status: 200 });
-
-  const redirect = new Response(null, {
-    status: 302,
-    headers: {
-      Location: "/test",
-    },
-  });
-
   it("should follow 302 redirect", async () => {
-    let i = 0;
-
     const spy = jest.fn(async (req: Request) => {
-      if (i++ === 0) return redirect.clone();
+      if (spy.mock.calls.length === 1) {
+        return new Response(null, {
+          status: 302,
+          headers: {
+            Location: "/test",
+          },
+        });
+      }
+
       expect(req.url).toEqual("http://example.com/test");
-      return ok.clone();
+      return new Response(null, { status: 200 });
     });
 
     const transport = redirects(spy);
 
-    const res = await transport(req.clone(), async () => {
+    const res = await transport(new Request("http://example.com"), async () => {
       throw new TypeError("Unexpected response");
     });
 
     expect(res.status).toEqual(200);
     expect(spy).toHaveBeenCalledTimes(2);
   });
+
+  describe("secure headers", () => {
+    const headers = {
+      cookie: "example_cookie",
+      authorization: "example_authorization",
+    };
+
+    it("should maintain cookies when staying with original host", async () => {
+      const spy = jest.fn(async (req: Request) => {
+        if (spy.mock.calls.length === 1) {
+          return new Response(null, {
+            status: 302,
+            headers: {
+              Location: "/test",
+            },
+          });
+        }
+
+        expect(req.url).toEqual("http://example.com/test");
+        expect(req.headers.get("Cookie")).toBe(headers.cookie);
+        expect(req.headers.get("Authorization")).toBe(headers.authorization);
+        return new Response(null, { status: 200 });
+      });
+
+      const transport = redirects(spy);
+
+      const res = await transport(
+        new Request("http://example.com", { headers }),
+        async () => {
+          throw new TypeError("Unexpected response");
+        }
+      );
+
+      expect(res.status).toEqual(200);
+      expect(spy).toHaveBeenCalledTimes(2);
+    });
+
+    it("should discard cookies when leaving original host", async () => {
+      const spy = jest.fn(async (req: Request) => {
+        if (spy.mock.calls.length === 1) {
+          return new Response(null, {
+            status: 302,
+            headers: {
+              Location: "https://example.com",
+            },
+          });
+        }
+
+        expect(req.url).toEqual("https://example.com/");
+        expect(req.headers.get("Cookie")).toBe(null);
+        expect(req.headers.get("Authorization")).toBe(null);
+        return new Response(null, { status: 200 });
+      });
+
+      const transport = redirects(spy);
+
+      const res = await transport(
+        new Request("http://example.com", { headers }),
+        async () => {
+          throw new TypeError("Unexpected response");
+        }
+      );
+
+      expect(res.status).toEqual(200);
+      expect(spy).toHaveBeenCalledTimes(2);
+    });
+  });
 });

src/index.ts

@@ -1,12 +1,12 @@
-import { resolve } from "url";
+import { URL } from "url";
 import { CommonRequest, CommonResponse } from "servie/dist/common";
 
 /**
  * Add redirect support to servie events.
  */
 declare module "servie/dist/signal" {
   export interface SignalEvents {
-    redirect: [string];
+    redirect: [URL];
   }
 }
 
@@ -40,6 +40,32 @@ export class MaxRedirectsError extends Error {
   }
 }
 
+/**
+ * Create a new request object and tidy up any loose ends to avoid leaking info.
+ */
+function safeRedirect<T>(
+  request: CommonRequest<T>,
+  location: string,
+  method: string
+) {
+  const originalUrl = new URL(request.url);
+  const newUrl = new URL(location, originalUrl);
+
+  request.signal.emit("redirect", newUrl);
+
+  const newRequest = request.clone();
+  newRequest.url = newUrl.toString();
+  newRequest.method = method;
+
+  // Delete cookie header when leaving the original URL.
+  if (originalUrl.origin !== newUrl.origin) {
+    newRequest.headers.delete("cookie");
+    newRequest.headers.delete("authorization");
+  }
+
+  return newRequest;
+}
+
 /**
  * Redirect confirmation function.
  */
@@ -66,24 +92,17 @@ export function redirects<T extends CommonRequest, U extends CommonResponse>(
     while (redirectCount++ < maxRedirects) {
       const res = await fn(req as T, done);
       const redirect = REDIRECT_STATUS[res.status];
+      const location = res.headers.get("Location");
 
-      if (redirect === undefined || !res.headers.has("Location")) return res;
-
-      const newUrl = resolve(req.url, res.headers.get("Location")!); // tslint:disable-line
+      if (redirect === undefined || !location) return res;
 
       await res.destroy(); // Ignore the result of the response on redirect.
 
-      req.signal.emit("redirect", newUrl);
-
       if (redirect === REDIRECT_TYPE.FOLLOW_WITH_GET) {
         const method = initReq.method.toUpperCase() === "HEAD" ? "HEAD" : "GET";
 
-        req = initReq.clone();
-        req.url = newUrl;
-        req.method = method;
+        req = safeRedirect(initReq, location, method);
         req.$rawBody = null; // Override internal raw body.
-
-        // No body will be sent with this redirect.
         req.headers.set("Content-Length", "0");
 
         continue;
@@ -94,18 +113,14 @@ export function redirects<T extends CommonRequest, U extends CommonResponse>(
 
         // Following HTTP spec by automatically redirecting with GET/HEAD.
         if (method.toUpperCase() === "GET" || method.toUpperCase() === "HEAD") {
-          req = initReq.clone();
-          req.url = newUrl;
-          req.method = method;
+          req = safeRedirect(initReq, location, method);
 
           continue;
         }
 
         // Allow the user to confirm redirect according to HTTP spec.
         if (confirmRedirect(req, res)) {
-          req = initReq.clone();
-          req.url = newUrl;
-          req.method = method;
+          req = safeRedirect(initReq, location, method);
 
           continue;
         }