Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cookie security #587

Merged
merged 6 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brown-rivers-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ima/core": patch
---

Fixed cookie parsing from setCookie header when multiple cookies were sent to server. Previously only the first cookie was parsed while multiple set-cookies could alter the cookie settings
6 changes: 6 additions & 0 deletions .changeset/flat-beds-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"create-ima-app": minor
"@ima/core": minor
---

Added new settings `validateCookies` to enable/disable cookie validation. It validates cookie options and request url before saving cookie or sending it to the server. This means that path, subdomain and secure options must match between the request url and the cookie, otherwise the cookie is not saved or sent.
1 change: 1 addition & 0 deletions packages/core/src/http/HttpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface HttpAgentRequestOptions {
) => HttpAgentResponse<B>)[];
abortController?: AbortController;
keepSensitiveHeaders?: boolean;
validateCookies?: boolean;
}

/**
Expand Down
22 changes: 15 additions & 7 deletions packages/core/src/http/HttpAgentImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ export class HttpAgentImpl extends HttpAgent {
data?: UnknownParameters,
options?: Partial<HttpAgentRequestOptions>
): Promise<HttpAgentResponse<B>> {
const optionsWithDefault = this._prepareOptions(options);
const optionsWithDefault = this._prepareOptions(options, url);

if (optionsWithDefault.cache) {
const cachedData = this._getCachedData<B>(method, url, data);
Expand Down Expand Up @@ -433,7 +433,8 @@ export class HttpAgentImpl extends HttpAgent {
* internally.
*/
_prepareOptions(
options: Partial<HttpAgentRequestOptions> = {}
options: Partial<HttpAgentRequestOptions> = {},
url: string
): HttpAgentRequestOptions {
const composedOptions = {
...this._defaultRequestOptions,
Expand All @@ -455,7 +456,9 @@ export class HttpAgentImpl extends HttpAgent {
if (composedOptions.fetchOptions?.credentials === 'include') {
// mock default browser behavior for server-side (sending cookie with a fetch request)
composedOptions.fetchOptions.headers.Cookie =
this._cookie.getCookiesStringForCookieHeader();
this._cookie.getCookiesStringForCookieHeader(
options.validateCookies ? url : undefined
);
}

return composedOptions;
Expand Down Expand Up @@ -497,10 +500,15 @@ export class HttpAgentImpl extends HttpAgent {
*/
_setCookiesFromResponse<B>(agentResponse: HttpAgentResponse<B>): void {
if (agentResponse.headersRaw) {
const receivedCookies = agentResponse.headersRaw.get('set-cookie');

if (receivedCookies) {
this._cookie.parseFromSetCookieHeader(receivedCookies);
const receivedCookies = agentResponse.headersRaw.getSetCookie();

if (receivedCookies.length > 0) {
this._cookie.parseFromSetCookieHeader(
receivedCookies,
this._defaultRequestOptions.validateCookies
? agentResponse.params.url
: undefined
);
}
}
}
Expand Down
8 changes: 3 additions & 5 deletions packages/core/src/http/__tests__/HttpAgentImplSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,9 @@ describe('ima.core.http.HttpAgentImpl', () => {
'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'],
},
// @ts-ignore
headersRaw: new Map(
Object.entries({
'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'],
})
),
headersRaw: new Headers({
'set-cookie': ['cookie1=cookie1', 'cookie2=cookie2'],
}),
};
});

Expand Down
123 changes: 116 additions & 7 deletions packages/core/src/storage/CookieStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,77 @@ export class CookieStorage extends Storage<Cookie['value']> {
return [Window, Request, Response];
}

/**
* Filters invalid cookies based on the provided url.
* We try to check validity of the domain based on secure, path and
* domain definitions.
*/
static validateCookieSecurity(cookie: Cookie, url: string): boolean {
const { pathname, hostname, protocol } = new URL(url);
const secure = protocol === 'https:';

/**
* Don't allow setting secure cookies without the secure
* defined in the validate options.
*/
if (
typeof cookie.options.secure === 'boolean' &&
secure !== cookie.options.secure
) {
return false;
}

/**
* Don't allow setting cookies with a path that doesn't start with
* the path defined in the validate options. Root path is always valid.
*/
if (
typeof cookie.options.path === 'string' &&
cookie.options.path !== '/'
) {
const pathChunks = pathname.split('/');
const cookiePathChunks = cookie.options.path.split('/');

/**
* Compare the path chunks of the request path and the cookie path.
*/
for (let i = 0; i < cookiePathChunks.length; i++) {
/**
* There are no more path chunks to compare, so the cookie path is a
* prefix of the request path.
*/
if (cookiePathChunks[i] === undefined) {
break;
}

/**
* The path chunks don't match, the cookie path is not a prefix of
* the request path.
*/
if (pathChunks[i] !== cookiePathChunks[i]) {
return false;
}
}
}

/**
* Domain security check, we also check for subdomain match.
*/
if (cookie.options.domain) {
const cookieDomain = cookie.options.domain.toLowerCase();
const normalizedCookieDomain = cookieDomain.startsWith('.')
? cookieDomain.slice(1)
: cookieDomain;

return normalizedCookieDomain === hostname ||
hostname.endsWith(normalizedCookieDomain)
? true
: false;
}

return true;
}

/**
* Initializes the cookie storage.
*
Expand Down Expand Up @@ -238,15 +309,29 @@ export class CookieStorage extends Storage<Cookie['value']> {
* Returns all cookies in this storage serialized to a string compatible
* with the `Cookie` HTTP header.
*
* When `url` is provided, the method validates the cookie security based on
* the `url` and the cookie's domain, path, and secure attributes.
*
* @return All cookies in this storage serialized to a string
* compatible with the `Cookie` HTTP header.
*/
getCookiesStringForCookieHeader(): string {
getCookiesStringForCookieHeader(url?: string): string {
const cookieStrings = [];

for (const cookieName of this._storage.keys()) {
const cookieItem = this._storage.get(cookieName);

/**
* Skip cookies that are not secure for the provided url.
*/
if (
url &&
cookieItem &&
!CookieStorage.validateCookieSecurity(cookieItem, url)
) {
continue;
}

cookieStrings.push(
this.#generateCookieString(cookieName, cookieItem!.value, {})
);
Expand All @@ -258,18 +343,42 @@ export class CookieStorage extends Storage<Cookie['value']> {
/**
* Parses cookies from the provided `Set-Cookie` HTTP header value.
*
* When `url` is provided, the method validates the cookie security based on
* the `url` and the cookie's domain, path, and secure attributes.
*
* The parsed cookies will be set to the internal storage, and the current
* HTTP response (via the `Set-Cookie` HTTP header) if at the server
* side, or the browser (via the `document.cookie` property).
*
* @param setCookieHeader The value of the `Set-Cookie` HTTP
* header.
* @param cookiesString The value of the `Set-Cookie` HTTP
* header. When there are multiple cookies, the value can be
* provided as an array of strings.
*/
parseFromSetCookieHeader(setCookieHeader: string): void {
const cookie = this.#extractCookie(setCookieHeader);
parseFromSetCookieHeader(
cookiesString: string | string[],
url?: string
): void {
const cookiesArray = Array.isArray(cookiesString)
? cookiesString
: [cookiesString];

for (const cookie of cookiesArray) {
const cookieItem = this.#extractCookie(cookie);

/**
* Skip cookies that are not secure for the provided url.
*/
if (
url &&
cookieItem &&
!CookieStorage.validateCookieSecurity(cookieItem, url)
) {
continue;
}

if (typeof cookie.name === 'string') {
this.set(cookie.name, cookie.value, cookie.options);
if (typeof cookieItem.name === 'string') {
this.set(cookieItem.name, cookieItem.value, cookieItem.options);
}
}
}

Expand Down
Loading
Loading