search

feathersjs-ecosystem / authentication-local

[MOVED] Local authentication plugin for feathers-authentication
https://github.com/feathersjs/feathers
MIT License
26 stars 15 forks source link

Bad error message is returned for invalid credentials #10

Closed ekryski closed 7 years ago

ekryski commented 7 years ago

Steps to reproduce

Attempt to authenticate with a username and password that don't exist.

Expected behavior

It should return a proper UnAuthorized error with a nice error message as to what the problem is.

Actual behavior

It returns an error object without an error message.

System configuration

Module versions (especially the part that's not working): feathers-authentication-local@0.3.3

NodeJS version: All

Operating System: All

Browser Version: All

React Native Version: All

Module Loader: All

marshallswain commented 7 years ago

I don't think it should leak information about whether or not the user exists. Just a plain not-authenticated error.

marshallswain commented 7 years ago

Actually, I just tested on a newly-generated project, and I'm not experiencing this same error. It's returning an actual feathers-errors 401.

I'm using "feathers-authentication-local": "^0.3.4",

daffl commented 7 years ago

Yes but it just says "Error" as the message. It should at least say something like "Invalid login"

marshallswain commented 7 years ago

Looks like it starts here: https://github.com/feathersjs/feathers-authentication-local/blob/master/src/verifier.js#L58

After _normalizeResult, it lands in the catch block, here: https://github.com/feathersjs/feathers-authentication-local/blob/master/src/verifier.js#L81. Since error was false it runs the else in the ternary. I think all we have to do is return a third argument. I'll make a PR.

marshallswain commented 7 years ago

I fixed the problem in #25, but we will need to put a conditional in the sinon.stub here: https://github.com/feathersjs/feathers-authentication-local/blob/master/test/verifier.test.js#L28 in order to fix the test.

ekryski commented 7 years ago

Thanks for digging in @marshallswain. I left a comment on #25.