Skip to content

Commit

Permalink
Fix: Cookie with secure attribute are not attached to the request on …
Browse files Browse the repository at this point in the history
…localhost(closes #2715) (#2761)

* Fix: Cookie with secure attribute are not attached to the request on localhost(closes #2715)

* fix code style

* fix code style 2

* fix code style 3
  • Loading branch information
Artem-Babich committed Apr 19, 2022
1 parent 7f80940 commit a50cb5e
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/request-pipeline/context.ts
Expand Up @@ -30,7 +30,7 @@ import getMockResponse from '../request-pipeline/request-hooks/response-mock/get
import { Http2Response } from './destination-request/http2';
import RequestEvent from '../session/events/request-event';

interface DestInfo {
export interface DestInfo {
url: string;
protocol: string;
host: string;
Expand Down
4 changes: 2 additions & 2 deletions src/request-pipeline/header-transforms/transforms.ts
Expand Up @@ -108,7 +108,7 @@ export const requestTransforms = {

export const forcedRequestTransforms = {
[BUILTIN_HEADERS.cookie]: (_src: string, ctx: RequestPipelineContext) =>
shouldOmitCredentials(ctx) ? void 0 : ctx.session.cookies.getHeader(ctx.dest.url) || void 0,
shouldOmitCredentials(ctx) ? void 0 : ctx.session.cookies.getHeader(ctx.dest) || void 0,
};

// Response headers
Expand Down Expand Up @@ -160,7 +160,7 @@ export const responseTransforms = {
src = src.replace('ALLOW-FROM', '').trim();

const isCrossDomain = ctx.isIframe && !urlUtils.sameOriginCheck(ctx.dest.url, src);
const proxiedUrl = ctx.toProxyUrl(src, isCrossDomain, ctx.contentInfo.contentTypeUrlToken);
const proxiedUrl = ctx.toProxyUrl(src, isCrossDomain, ctx.contentInfo.contentTypeUrlToken);

return 'ALLOW-FROM ' + proxiedUrl;
},
Expand Down
26 changes: 18 additions & 8 deletions src/session/cookies.ts
Expand Up @@ -4,6 +4,7 @@ import { castArray, flattenDeep } from 'lodash';
import { parseUrl } from '../utils/url';
import { parse as parseJSON, stringify as stringifyJSON } from '../utils/json';
import { URL } from 'url';
import { DestInfo } from '../request-pipeline/context';

const LOCALHOST_DOMAIN = 'localhost';
const LOCALHOST_IP = '127.0.0.1';
Expand Down Expand Up @@ -46,11 +47,11 @@ export default class Cookies {
this._pendingSyncCookies = [];
}

_syncWrap(method: string) {
_syncWrap (method: string) {
return (...args) => {
let syncErr, syncResult;
this._cookieJar.store[method](...args, (err, result) => {
syncErr = err;
syncErr = err;
syncResult = result;
});

Expand All @@ -61,9 +62,11 @@ export default class Cookies {
};
}

static _isLocalHostDomain = (domain): boolean => domain === LOCALHOST_DOMAIN || domain === LOCALHOST_IP;

static _hasLocalhostDomain (cookie): boolean {
if (cookie)
return cookie.domain === LOCALHOST_DOMAIN || cookie.domain === LOCALHOST_IP;
return this._isLocalHostDomain(cookie.domain);

return false;
}
Expand Down Expand Up @@ -144,8 +147,8 @@ export default class Cookies {
private _findCookiesByApi (urls: Url[], key?: string): (Cookie | Cookie[])[] {
return urls.map(({ domain, path }) => {
const cookies = key
? this._findCookieSync(domain, path, key)
: this._findCookiesSync(domain, path);
? this._findCookieSync(domain, path, key)
: this._findCookiesSync(domain, path);

return cookies || [];
});
Expand Down Expand Up @@ -244,7 +247,7 @@ export default class Cookies {

for (const deletedCookie of deletedCookies) {
if (deletedCookie.domain && deletedCookie.path && deletedCookie.key) {
this._removeCookieSync(deletedCookie.domain, deletedCookie.path, deletedCookie.key);
this._removeCookieSync(deletedCookie.domain, deletedCookie.path, deletedCookie.key);

deletedCookie.expires = new Date(0);
this._pendingSyncCookies.push(deletedCookie);
Expand Down Expand Up @@ -278,7 +281,14 @@ export default class Cookies {
return this._cookieJar.getCookieStringSync(url, { http: false });
}

getHeader (url: string): string | null {
return this._cookieJar.getCookieStringSync(url, { http: true }) || null;
getHeader ({ url, hostname }: DestInfo): string | null {
// NOTE: https://github.com/DevExpress/testcafe-hammerhead/issues/2715
// Cookies with the secure attribute should be passed to the localhost server(without ssl) or any other server(with ssl).
// CookieJar only checks the protocol, but not a hostname. That is why we should to add secure attribute in case of localhost.
const cookieJarOpts = Cookies._isLocalHostDomain(hostname) ?
{ http: true, secure: true } :
{ http: true };

return this._cookieJar.getCookieStringSync(url, cookieJarOpts) || null;
}
}
43 changes: 43 additions & 0 deletions test/server/cookies-test.js
Expand Up @@ -380,6 +380,49 @@ describe('Cookies', () => {
expect(expectedCookies).eql(cookies);
});
});
describe('Attach secure cookies to request', () => {
beforeEach(() => {
cookieJar.setCookies([
{ name: 'apiCookie1', value: 'value1', domain: 'domain1.com', path: '/', secure: true },
{ name: 'apiCookie2', value: 'value2', domain: 'localhost', path: '/', secure: true },
{ name: 'apiCookie3', value: 'value3', domain: '127.0.0.1', path: '/', secure: true },
]);
});

afterEach(() => {
cookieJar.deleteCookies();
cookieJar._pendingSyncCookies = [];
});

it('Should get secure cookies string from ssl domain', () => {
const expectedCookieString = 'apiCookie1=value1';
const destInfo = { url: 'https://domain1.com/', hostname: 'domain1.com' };
const cookieStr = cookieJar.getHeader(destInfo);

expect(cookieStr).eql(expectedCookieString);
});

it('Should get secure cookies string from localhost', () => {
const expectedCookieString1 = 'apiCookie2=value2';
const expectedCookieString2 = 'apiCookie3=value3';

const destInfo1 = { url: 'http://localhost:3006', hostname: 'localhost' };
const destInfo2 = { url: 'http://127.0.0.1:3001', hostname: '127.0.0.1' };
const cookieStr1 = cookieJar.getHeader(destInfo1);
const cookieStr2 = cookieJar.getHeader(destInfo2);

expect(cookieStr1).eql(expectedCookieString1);
expect(cookieStr2).eql(expectedCookieString2);
});

it('Should return null if domain is not HTTPS', () => {
const expectedCookieString = null;
const destInfo1 = { url: 'http://domain1.com', hostname: 'domain1.com' };
const cookieStr1 = cookieJar.getHeader(destInfo1);

expect(cookieStr1).eql(expectedCookieString);
});
});

describe('Set cookies', () => {
afterEach(() => {
Expand Down

0 comments on commit a50cb5e

Please sign in to comment.