adding configurable secret key option, readme, and test

krakenjs / lusca

Application security for express apps.

Other

1.79k stars 122 forks source link

adding configurable secret key option, readme, and test #34

Closed grawk closed 10 years ago

grawk commented 10 years ago

This should resolve https://github.com/krakenjs/lusca/issues/29

totherik commented 10 years ago

@grawk can you add explicit tests for both express Memory session store and cookie-session? Would like to try to prevent changes from slipping through again.

totherik commented 10 years ago

Also, after thinking about it some more, there doesn't seem to be any harm in changing the default to not have a leading underscore. It doesn't really buy us anything and if the intent is to shield it from accidentally being overwritten, etc. there are better ways to accomplish that.

grawk commented 10 years ago

@totherik regarding tests that would have caught this, we would have to upgrade the express devDependency to ^4.3.8. And after doing that, will have to determine what's going on with supertest as it doesn't seem to be maintaining session state during chained requests with express ^4.3.8. Using express 3, both memory session and cookie session behaved perfectly well. Do you want to finish this PR in order to provide this fix and then expend a bigger effort on the unit tests for a later pull request?

grawk commented 10 years ago

https://github.com/krakenjs/lusca/pull/38 should address all the concerns herein