Skip to content

Commit

Permalink
Fix bug when "keys" were set but setting cookies did not default to "…
Browse files Browse the repository at this point in the history
…signed"

fixes #82
  • Loading branch information
dougwilson committed Jan 29, 2017
1 parent 5b67f0d commit c3c156f
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 9 deletions.
5 changes: 5 additions & 0 deletions History.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
unreleased
==========

* Fix bug when `keys` were set but setting cookies did not default to `signed`

0.6.2 / 2016-11-12
==================

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ This adds cookie support as a Connect middleware layer for use in Express apps,

This extracts the cookie with the given name from the `Cookie` header in the request. If such a cookie exists, its value is returned. Otherwise, nothing is returned.

`{ signed: true }` can optionally be passed as the second parameter _options_. In this case, a signature cookie (a cookie of same name ending with the `.sig` suffix appended) is fetched. If no such cookie exists, nothing is returned.
`{ signed: true }` can optionally be passed as the second parameter _options_. In this case, a signature cookie (a cookie of same name ending with the `.sig` suffix appended) is fetched. If no such cookie exists, nothing is returned. The `signed` option will default to `true` if `keys` are present, otherwise `false`.

If the signature cookie _does_ exist, the provided [Keygrip](https://www.npmjs.com/package/keygrip) object is used to check whether the hash of _cookie-name_=_cookie-value_ matches that of any registered key:

Expand All @@ -71,7 +71,7 @@ If the _options_ object is provided, it will be used to generate the outbound co
* `domain`: a string indicating the domain of the cookie (no default).
* `secure`: a boolean indicating whether the cookie is only to be sent over HTTPS (`false` by default for HTTP, `true` by default for HTTPS). [Read more about this option below](#secure-cookies).
* `httpOnly`: a boolean indicating whether the cookie is only to be sent over HTTP(S), and not made available to client JavaScript (`true` by default).
* `signed`: a boolean indicating whether the cookie is to be signed (`false` by default). If this is true, another cookie of the same name with the `.sig` suffix appended will also be sent, with a 27-byte url-safe base64 SHA1 value representing the hash of _cookie-name_=_cookie-value_ against the first [Keygrip](https://www.npmjs.com/package/keygrip) key. This signature key is used to detect tampering the next time a cookie is received.
* `signed`: a boolean indicating whether the cookie is to be signed (the option will default to `true` if `keys` are present, otherwise `false`). If this is true, another cookie of the same name with the `.sig` suffix appended will also be sent, with a 27-byte url-safe base64 SHA1 value representing the hash of _cookie-name_=_cookie-value_ against the first [Keygrip](https://www.npmjs.com/package/keygrip) key. This signature key is used to detect tampering the next time a cookie is received.
* `overwrite`: a boolean indicating whether to overwrite previously set cookies of the same name (`false` by default). If this is true, all cookies set during the same request with the same name (regardless of path or domain) are filtered out of the Set-Cookie header when setting this cookie.

### Secure cookies
Expand Down
2 changes: 1 addition & 1 deletion lib/cookies.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Cookies.prototype.set = function(name, value, opts) {

headers = pushCookie(headers, cookie)

if (opts && signed) {
if (signed) {
if (!this.keys) throw new Error('.keys required for signed cookies');
cookie.value = this.keys.sign(cookie.toString())
cookie.name += ".sig"
Expand Down
4 changes: 2 additions & 2 deletions test/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ describe('Express', function () {
.set( "signed", "bar", { signed: true } )

// mimic a signed cookie, but with a bogus signature
.set( "tampered", "baz" )
.set( "tampered.sig", "bogus" )
.set( "tampered", "baz", { signed: false } )
.set( "tampered.sig", "bogus", { signed: false } )

// set a cookie that will be overwritten
.set( "overwrite", "old-value", { signed: true } )
Expand Down
4 changes: 2 additions & 2 deletions test/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ describe('HTTP', function () {
.set( "signed", "bar", { signed: true } )

// mimic a signed cookie, but with a bogus signature
.set( "tampered", "baz" )
.set( "tampered.sig", "bogus" )
.set( "tampered", "baz", { signed: false } )
.set( "tampered.sig", "bogus", { signed: false } )

// set a cookie that will be overwritten
.set( "overwrite", "old-value", { signed: true } )
Expand Down
4 changes: 2 additions & 2 deletions test/restify.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ function setCookies(req, res) {
cookies
.set('unsigned', 'foo', { signed:false, httpOnly: false })
.set('signed', 'bar', { signed: true })
.set('tampered', 'baz')
.set('tampered.sig', 'bogus')
.set('tampered', 'baz', { signed: false })
.set('tampered.sig', 'bogus', { signed: false })
.set('overwrite', 'old-value', { signed: true })
.set('overwrite', 'new-value', { overwrite: true, signed: true })
}
Expand Down

0 comments on commit c3c156f

Please sign in to comment.