Merge pull request #2007 from microsoftgraph/fix/redirect-handler-strip-cookie-header

Fix: strip Cookie header on cross-origin redirects in RedirectHandler

Author: ramsessanchez

src/middleware/MiddlewareUtil.ts

@@ -130,6 +130,40 @@ export const appendRequestHeader = (request: RequestInfo, options: FetchOptions
 	}
 };
 
+/**
+ * @constant
+ * To remove a header from the given request
+ * @param {RequestInfo} request - The request object or the url string
+ * @param {FetchOptions|undefined} options - The request options object
+ * @param {string} key - The header key string
+ * @returns Nothing
+ */
+export const removeRequestHeader = (request: RequestInfo, options: FetchOptions | undefined, key: string): void => {
+	if (typeof Request !== "undefined" && request instanceof Request) {
+		(request as Request).headers.delete(key);
+	}
+	if (typeof options !== "undefined" && options.headers !== undefined) {
+		if (typeof Headers !== "undefined" && options.headers instanceof Headers) {
+			(options.headers as Headers).delete(key);
+		} else if (options.headers instanceof Array) {
+			const headers = options.headers as string[][];
+			const lowerKey = key.toLowerCase();
+			for (let i = headers.length - 1; i >= 0; i--) {
+				if (headers[i][0].toLowerCase() === lowerKey) {
+					headers.splice(i, 1);
+				}
+			}
+		} else {
+			const headers = options.headers as Record<string, string>;
+			for (const headerKey of Object.keys(headers)) {
+				if (headerKey.toLowerCase() === key.toLowerCase()) {
+					delete headers[headerKey];
+				}
+			}
+		}
+	}
+};
+
 /**
  * @constant
  * To clone the request with the new url

src/middleware/RedirectHandler.ts

@@ -13,7 +13,7 @@ import { Context } from "../IContext";
 import { RequestMethod } from "../RequestMethod";
 import { Middleware } from "./IMiddleware";
 import { MiddlewareControl } from "./MiddlewareControl";
-import { cloneRequestWithNewUrl } from "./MiddlewareUtil";
+import { cloneRequestWithNewUrl, removeRequestHeader } from "./MiddlewareUtil";
 import { RedirectHandlerOptions } from "./options/RedirectHandlerOptions";
 import { FeatureUsageFlag, TelemetryHandlerOptions } from "./options/TelemetryHandlerOptions";
 
@@ -58,6 +58,13 @@ export class RedirectHandler implements Middleware {
 	 */
 	private static AUTHORIZATION_HEADER = "Authorization";
 
+	/**
+	 * @private
+	 * @static
+	 * A member representing the cookie header name
+	 */
+	private static COOKIE_HEADER = "Cookie";
+
 	/**
 	 * @private
 	 * @static
@@ -131,10 +138,10 @@ export class RedirectHandler implements Middleware {
 
 	/**
 	 * @private
-	 * To check whether the authorization header in the request should be dropped for consequent redirected requests
+	 * To check whether the sensitive headers (Authorization, Cookie) in the request should be dropped for consequent redirected requests
 	 * @param {string} requestUrl - The request url value
 	 * @param {string} redirectUrl - The redirect url value
-	 * @returns A boolean representing whether the authorization header in the request should be dropped for consequent redirected requests
+	 * @returns A boolean representing whether the sensitive headers in the request should be dropped for consequent redirected requests
 	 */
 	private shouldDropAuthorizationHeader(requestUrl: string, redirectUrl: string): boolean {
 		const schemeHostRegex = /^[A-Za-z].+?:\/\/.+?(?=\/|$)/;
@@ -200,7 +207,8 @@ export class RedirectHandler implements Middleware {
 			} else {
 				const redirectUrl: string = this.getLocationHeader(response);
 				if (!this.isRelativeURL(redirectUrl) && this.shouldDropAuthorizationHeader(response.url, redirectUrl)) {
-					delete context.options.headers[RedirectHandler.AUTHORIZATION_HEADER];
+					removeRequestHeader(context.request, context.options, RedirectHandler.AUTHORIZATION_HEADER);
+					removeRequestHeader(context.request, context.options, RedirectHandler.COOKIE_HEADER);
 				}
 				await this.updateRequestUrl(redirectUrl, context);
 			}

test/common/middleware/RedirectHandler.ts

@@ -6,6 +6,7 @@
  */
 
 import { assert } from "chai";
+import * as sinon from "sinon";
 
 import { Context } from "../../../src/IContext";
 import { MiddlewareControl } from "../../../src/middleware/MiddlewareControl";
@@ -293,6 +294,67 @@ describe("RedirectHandler.ts", () => {
 			assert.equal(cxt.response.status, 200);
 		});
 
+		it("Should not drop Cookie header for same authority redirect url", async () => {
+			const options = new RedirectHandlerOptions();
+			const cxt: Context = {
+				request: "https://graph.microsoft.com/v1.0/me",
+				options: {
+					method: "POST",
+					body: "dummy body",
+					headers: {
+						[RedirectHandler["COOKIE_HEADER"]]: "session=abc",
+					},
+				},
+			};
+			dummyHTTPHandler.setResponses([
+				new Response("", {
+					status: 301,
+					headers: {
+						[RedirectHandler["LOCATION_HEADER"]]: "https://graph.microsoft.com/v2.0/me",
+					},
+				}),
+				new Response("ok", { status: 200 }),
+			]);
+			await handler["executeWithRedirect"](cxt, 0, options);
+			assert.isDefined(cxt.options.headers[RedirectHandler["COOKIE_HEADER"]]);
+			assert.equal(cxt.response.status, 200);
+		});
+
+		it("Should drop both Authorization and Cookie headers for cross origin redirects", async () => {
+			const options = new RedirectHandlerOptions();
+			const cxt: Context = {
+				request: "https://graph.microsoft.com/v1.0/me",
+				options: {
+					method: "POST",
+					body: "dummy body",
+					headers: {
+						[RedirectHandler["AUTHORIZATION_HEADER"]]: "******",
+						[RedirectHandler["COOKIE_HEADER"]]: "session=abc",
+						"X-Custom-Header": "custom-value",
+					},
+				},
+			};
+			dummyHTTPHandler.setResponses([
+				new Response("", {
+					status: 301,
+					headers: {
+						[RedirectHandler["LOCATION_HEADER"]]: "https://graphredirection.microsoft.com/v1.0/me",
+					},
+				}),
+				new Response("ok", { status: 200 }),
+			]);
+			const stub = sinon.stub(handler as any, "shouldDropAuthorizationHeader").returns(true);
+			try {
+				await handler["executeWithRedirect"](cxt, 0, options);
+			} finally {
+				stub.restore();
+			}
+			assert.isUndefined(cxt.options.headers[RedirectHandler["AUTHORIZATION_HEADER"]]);
+			assert.isUndefined(cxt.options.headers[RedirectHandler["COOKIE_HEADER"]]);
+			assert.equal(cxt.options.headers["X-Custom-Header"], "custom-value");
+			assert.equal(cxt.response.status, 200);
+		});
+
 		it("Should return success response after successful redirect", async () => {
 			const options = new RedirectHandlerOptions();
 			const cxt: Context = {