Skip to content

Commit

Permalink
feat: use TLD per default for cookie domain (#21)
Browse files Browse the repository at this point in the history
This PR changes the behavior of the cookie detection mechanism to automatically set the cookie domain to the TLD of the request. To get the old behavior, set `dontUseTldForCookieDomain` to `true`.

Closes #20
  • Loading branch information
aeneasr authored Feb 23, 2022
1 parent 31bd234 commit 808c421
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 15 deletions.
66 changes: 57 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@types/request": "^2.48.7",
"@types/set-cookie-parser": "^2.4.1",
"@types/supertest": "^2.0.11",
"@types/tldjs": "^2.3.1",
"babel-jest": "^27.2.0",
"esbuild": "^0.12.28",
"express": "^4.17.1",
Expand All @@ -35,14 +36,15 @@
"rollup-plugin-dts": "^4.0.0",
"rollup-plugin-esbuild": "^4.5.0",
"supertest": "^6.1.6",
"tldjs": "^2.3.1",
"typescript": "^4.4.3"
},
"peerDependencies": {
"@ory/client": ">=0.0.1-alpha.49",
"next": ">=12.0.10"
},
"dependencies": {
"@ory/client": ">=0.0.1-alpha.21",
"@ory/client": ">=0.0.1-alpha.49",
"cookie": "^0.4.1",
"istextorbinary": "^6.0.0",
"next": ">=12.0.10",
Expand Down
102 changes: 100 additions & 2 deletions src/next-edge/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { createApiHandler, CreateApiHandlerOptions } from './index'
import {
createApiHandler,
CreateApiHandlerOptions,
guessCookieDomain
} from './index'
import express from 'express'
import { NextApiRequest, NextApiResponse } from 'next'
import supertest from 'supertest'
Expand Down Expand Up @@ -60,6 +64,7 @@ describe('NextJS handler', () => {
.get(
'/?paths=api&paths=kratos&paths=public&paths=self-service&paths=login&paths=browser'
)
.set('Host', 'www.example.org')
.expect(303)
.then((res) => {
expect(res.headers['set-cookie']).toBeDefined()
Expand All @@ -70,7 +75,7 @@ describe('NextJS handler', () => {
).toBeDefined()

cookies.forEach(({ domain, secure }) => {
expect(domain).toBeUndefined()
expect(domain).toEqual('example.org')
expect(secure).toBeFalsy()
})

Expand All @@ -79,6 +84,29 @@ describe('NextJS handler', () => {
.catch(done)
})

test('sets the appropriate cookie domain based on headers', (done) => {
app = createApp({
apiBaseUrlOverride: 'https://playground.projects.oryapis.com'
})

supertest(app.app)
.get(
'/?paths=api&paths=kratos&paths=public&paths=self-service&paths=login&paths=browser'
)
.set('Host', 'www.example.org')
.set('X-Forwarded-Host', 'www.example.bar')
.expect(303)
.then((res) => {
const cookies = parse(res.headers['set-cookie'])
cookies.forEach(({ domain, secure }) => {
expect(domain).toEqual('example.bar')
})

done()
})
.catch(done)
})

test('sets secure true if a TLS connection', (done) => {
app = createApp({
apiBaseUrlOverride: 'https://playground.projects.oryapis.com'
Expand Down Expand Up @@ -253,3 +281,73 @@ describe('NextJS handler', () => {
)
})
})

describe('cookie guesser', () => {
test('uses force domain', async () => {
expect(
guessCookieDomain('https://localhost', {
forceCookieDomain: 'some-domain'
})
).toEqual('some-domain')
})

test('does not use any guessing domain', async () => {
expect(
guessCookieDomain('https://localhost', {
dontUseTldForCookieDomain: true
})
).toEqual(undefined)
})

test('is not confused by invalid data', async () => {
expect(
guessCookieDomain('5qw5tare4g', {
dontUseTldForCookieDomain: true
})
).toEqual(undefined)
expect(
guessCookieDomain('https://123.123.123.123.123', {
dontUseTldForCookieDomain: true
})
).toEqual(undefined)
})

test('is not confused by IP', async () => {
expect(
guessCookieDomain('https://123.123.123.123', {
dontUseTldForCookieDomain: true
})
).toEqual(undefined)
expect(
guessCookieDomain('https://2001:0db8:0000:0000:0000:ff00:0042:8329', {
dontUseTldForCookieDomain: true
})
).toEqual(undefined)
})

test('uses TLD', async () => {
expect(guessCookieDomain('https://foo.localhost', {})).toEqual(
'foo.localhost'
)

expect(guessCookieDomain('https://foo.localhost:1234', {})).toEqual(
'foo.localhost'
)

expect(
guessCookieDomain(
'https://spark-public.s3.amazonaws.com/dataanalysis/loansData.csv',
{}
)
).toEqual('spark-public.s3.amazonaws.com')

expect(guessCookieDomain('spark-public.s3.amazonaws.com', {})).toEqual(
'spark-public.s3.amazonaws.com'
)

expect(guessCookieDomain('https://localhost/123', {})).toEqual('localhost')
expect(guessCookieDomain('https://localhost:1234/123', {})).toEqual(
'localhost'
)
})
})
48 changes: 45 additions & 3 deletions src/next-edge/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import parse from 'set-cookie-parser'
import { IncomingHttpHeaders } from 'http'
import { Buffer } from 'buffer'
import { isText } from 'istextorbinary'
import { NextResponse } from 'next/server'
import type { NextMiddleware } from 'next/server'
import tldjs from 'tldjs'

const forwardedHeaders = [
'accept',
Expand Down Expand Up @@ -74,9 +73,19 @@ export interface CreateApiHandlerOptions {
*
* If you are running this app on a subdomain and you want the session and CSRF cookies
* to be valid for the whole TLD, you can use this setting to force a cookie domain.
*
* Please be aware that his method disables the `dontUseTldForCookieDomain` option.
*/
forceCookieDomain?: string

/**
* Per default the cookie will be set on the hosts top-level-domain. If the app
* runs on www.example.org, the cookie domain will be set automatically to example.org.
*
* Set this option to true to disable that behaviour.
*/
dontUseTldForCookieDomain?: boolean

/**
* If set to true will set the "Secure" flag for all cookies. This might come in handy when you deploy
* not on Vercel.
Expand Down Expand Up @@ -157,10 +166,18 @@ export function createApiHandler(options: CreateApiHandlerOptions) {
options.forceCookieSecure === undefined
? isTls
: options.forceCookieSecure

const forwarded = req.rawHeaders.findIndex(
(h) => h.toLowerCase() === 'x-forwarded-host'
)
const host =
forwarded > -1 ? req.rawHeaders[forwarded + 1] : req.headers.host
const domain = guessCookieDomain(host, options)

res.headers['set-cookie'] = parse(res)
.map((cookie) => ({
...cookie,
domain: options.forceCookieDomain,
domain,
secure,
encode
}))
Expand Down Expand Up @@ -202,3 +219,28 @@ export function createApiHandler(options: CreateApiHandlerOptions) {
})
}
}

export function guessCookieDomain(
url: string | undefined,
options: CreateApiHandlerOptions
) {
if (!url || options.forceCookieDomain) {
return options.forceCookieDomain
}

if (options.dontUseTldForCookieDomain) {
return undefined
}

const parsed = tldjs.parse(url || '')

if (!parsed.isValid || parsed.isIp) {
return undefined
}

if (!parsed.domain) {
return parsed.hostname
}

return parsed.domain
}

0 comments on commit 808c421

Please sign in to comment.