WebThingsIO / gateway

WebThings Gateway - a self-hosted web application for monitoring and controlling a building over the web
http://webthings.io/gateway
Mozilla Public License 2.0
2.61k stars 339 forks source link

Validate MFA authentication requests #2874

Open jcherrabi opened 3 years ago

jcherrabi commented 3 years ago

Hello, while testing the MFA through an API i ran into a but in the following file: login_controller.ts gateway version 1.1.0

function name code line 40: controller.post('/', limiter, async (request, response) follwing lines: if (body.mfa.totp.length === 12) { and backupMatch = await Passwords.compare(body.mfa.totp, backup);

Should be updated to : line code: 69 if (body.mfa.length === 12) { and line 72 backupMatch = await Passwords.compare(body.mfa, backup);

basically totp isn't a property in body.mfa

once that change was done the MFA login via the RestAPI returned the token correctly..

Cheers, Jay

benfrancis commented 3 years ago

Hi @jcherrabi, thanks very much for this report. Would you be willing to create a pull request with your fix?

jcherrabi commented 3 years ago

Hello @benfrancis, no need !!! actually after inspecting the code i realized that i was sending the wrong format through the API since the frontend behaves a bit differently... although this needs to be dealt with properly through a validator of some sort...

i was sending this: { "email":"jay@email.com", "password":"myPassword", "mfa":"986312948612" } instead of this: basically the totp must be nested inside the mfa { "email":"jay@email.com", "password":"password", "mfa":{ "totp":"986312948612" } }

so no changes should be made and maybe we should include an input validator as a middleware something like this package: https://express-validator.github.io/docs/

Cheers, Jay

benfrancis commented 3 years ago

I don't know about adding yet another dependency, but this could definitely do with more validation. Thanks for reporting.