Login API should only accept credentials in POST body
joemfb opened this issue · 9 comments
Currently, users are logged in via a GET request, and their credentials are passed as URL params. This is problematic, as any middleware access logging includes user credentials.
should be relatively easy to fix this, I think. having a quick look now..
Yep, working. Exchanged get with post in both server.js and auth-srv.js. Just needed to add an extra null before the params in $http.post, as it takes an extra data param on 2nd place..
Thanks for taking a look. Won't you need to swap req.query.username with req.body.username, or somesuch?
didn't bother. it is legal to pass in request params on a POST as well. You might argue that putting them in body is safer against sniffing though..
Yeah, I would argue that ;) While POST is preferable to GET regardless (for limiting crawlers, for instance), I'd prefer not to have domain credentials in access logs ... I don't see any advantage to passing that data in URL params.
checking now.. :)
Weird. Browser - NodeJs traffic seems ok, but something odd happens between NodeJS - MarkLogic. An extra pair of eyes might help. The changes I did so far:
- replace get for post in auth-srv.js and server.js
- added { username: username, password: password} as 2nd param in the login post call from auth-src, remove the params from 3rd param
- adjusted /user/login in server.js to use req.body.username instead of req.query.username, idem for password.
Username and password are received in good order, but call to MarkLogic simply fails. The on error shows 'socket hang up'. The ML access log seem to indicate the call was received and returned ok.
@joemfb wanna take a look? maybe share screen?
Found issue, http.get to profile extension was copying req.headers, that caused trouble.
Fixed in develop