koajs / csrf

CSRF tokens for koa
MIT License
264 stars 32 forks source link

Add session check with test, update README #16

Closed cesarandreu closed 9 years ago

cesarandreu commented 9 years ago

This PR adds a session check. If the session is invalid, it returns null. Currently it'll throw an error if you have a null session.

This is a problem because I log out my users by setting this.session = null, but I also have the following middleware:

function* setCsrfCookieMiddleware (next) {
  yield next
  this.cookies.set('XSRF-TOKEN', this.csrf, {httpOnly: false})
}

This is needed so my client-side app can read the cookie and set the X-XSRF-TOKEN header on every request.

My current workaround is to add a conditional before setting the cookie:

function* setCsrfCookieMiddleware (next) {
  yield next
  if (this.session)
    this.cookies.set('XSRF-TOKEN', this.csrf, {httpOnly: false})
}
coveralls commented 9 years ago

Coverage Status

Coverage decreased (-2.31%) to 90.0% when pulling 3171e065d0f9b9d341e20c855c02d98240bdc4bc on cesarandreu:session-check into 9cfb0b6ca1b5e0079f90bcdff53c0ea6a80cf802 on koajs:master.

cesarandreu commented 9 years ago

It's passing tests locally... Is there any way to have travis retry?

~/D/csrf git:session-check ❯❯❯ npm run test test

> koa-csrf@2.1.3 test /Users/cesarandreu/Developer/csrf
> NODE_ENV=test mocha --harmony-generators --reporter spec --require should test

  CSRF Token
    should create
      ✓ a token
      ✓ a single token per request
      ✓ a new token per request
    should assert
      ✓ when no token is supplied
    should not assert when the token is supplied via
      ✓ json body
      ✓ querystring
      ✓ x-csrf-token
      ✓ x-xsrf-token
    .assertCSRF()
      ✓ should support a string value

  CSRF Token Middleware
    should create
      ✓ a token
      ✓ a single token per request
      ✓ a new token per request
      ✓ a null token when session is invalid
    should assert
      ✓ when no token is supplied
    should not assert when the token is supplied via
      ✓ json body
      ✓ querystring
      ✓ querystring with body
      ✓ x-csrf-token
      ✓ x-xsrf-token

  19 passing (111ms)
~/D/csrf git:session-check ❯❯❯ npm run test-cov

> koa-csrf@2.1.3 test-cov /Users/cesarandreu/Developer/csrf
> NODE_ENV=test node --harmony-generators node_modules/.bin/istanbul cover ./node_modules/.bin/_mocha -- --reporter dot --require should

  ․․․․․․․․․․․․․․․․․․․

  19 passing (104ms)

=============================================================================
Writing coverage object [/Users/cesarandreu/Developer/csrf/coverage/coverage.json]
Writing coverage reports at [/Users/cesarandreu/Developer/csrf/coverage]
=============================================================================

=============================== Coverage summary ===============================
Statements   : 90.91% ( 40/44 )
Branches     : 97.37% ( 37/38 )
Functions    : 77.78% ( 7/9 )
Lines        : 92.5% ( 37/40 )
================================================================================
jonathanong commented 9 years ago

i haven't looked at this PR yet, but can you rebase

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.19%) to 92.5% when pulling 531e8f147dafec03464ce86ab020d7a3afa93778 on cesarandreu:session-check into 9cfb0b6ca1b5e0079f90bcdff53c0ea6a80cf802 on koajs:master.

cesarandreu commented 9 years ago

I rebased and it passed, awesome :D. Thanks.

jonathanong commented 9 years ago

thanks 8f48e14a18cd8ae5594c0b23a699beefe7c6734f