Use localstorage instead of cookies for password hash #296
Conversation
Used to request password hash during initial websocket connection
duvholt
force-pushed
the
feat/localstorage-passwordhash
branch
from
895e65f
to
998ce7b
Compare
Codecov Report
@@ Coverage Diff @@
## master #296 +/- ##
==========================================
+ Coverage 86.96% 87.98% +1.01%
==========================================
Files 20 24 +4
Lines 744 832 +88
==========================================
+ Hits 647 732 +85
- Misses 97 100 +3
Continue to review full report at Codecov.
|
server/utils/socketAction.js
Outdated
| @@ -0,0 +1,16 @@ | |||
| const { emit } = require('../utils'); | |||
|
|
|||
| const waitForAction = (socket, event, requestAction, receiveAction) => ( | |||
There was a problem hiding this comment.
The websocket emits events on a channel, so I'd say the function should be named waitForEvent and have channel as one of its parameters (instead of event as it has now).
Considering that we use "action" as the name for everything that goes through redux, I'm okay with leaving it as is.
This would also do something with the weird naming like socket.createAction.action, which probably should be socket.createEvent.action (or auth or admin), if I'm not mistaken.
TL;DR: naming things is hard
There was a problem hiding this comment.
In this case it is literally waiting for an action though. Yes, it uses events to do that, but it only cares about a single action from that event listener.
And regarding channel: both Nodejs and socketio use eventName to refer to what I have called event. I will consider changing it to eventName though.
I agree about socket.createAction. That started as an ugly hack that grew uglier. I think I'll rename it to emitEvent. Preferably it would just be named emit, but we have to mock that function for testing.
server/utils/socketAction.js
Outdated
|
|
||
| const waitForAction = (socket, event, requestAction, receiveAction) => ( | ||
| new Promise((resolve) => { | ||
| socket.once(event, async (payload) => { |
There was a problem hiding this comment.
I'm a bit uncertain on how Node handles these promises if e.g. 1000000 users open and quickly closes a WS connection (as a DoS attempt). NodeJS runtime will GC this if it's not referenced and/or in scope, so it most likely won't be an issue. It could be something to consider researching further, though.
There was a problem hiding this comment.
I think it will be garbage collected like everything else in connection when socket is disconnected.
Should be a bit easier to read than createAction.action()
Fixes #265
Implemented using a separate websocket event and only sends passwordhash to backend when needed.
Client reply is implemented using a saga that simply waits for the 'request action' and responds with a 'send action'.