suggestion: check emailVerified after valid password #931
Comments
|
Thank you for reporting the issue. Would you mind contributing the change yourself? |
|
@bajtos this update is simple enough. I've never submitted a pull request to a public repo so probably a good one to start with! A question though, the current tests seem to all supply valid credentials supporting the suggestion. Do you have any other recommended tests that should be created for this? Supplying invalid credentials should never encounter this error but seems silly to test for that. |
It would be best to write a test that fails before the fix and passes once your change is applied. Just implement what you have described above:
|
|
Sounds good. Either tonight or over the weekend I will add the test and adjust the pull request. |
|
@bajtos for the local work I'm doing, is there anyway I can only run the user mocha tests? When I attempt to run |
|
|
… supplied for verified error message to be returned - tests added as suggested and fail under previous version of User model - strongloop#931
… supplied for verified error message to be returned - tests added as suggested and fail under previous version of User model - strongloop#931
|
PR merged. |
…globalize-error-messages * 'master' of github.com:strongloop/loopback: (25 commits) Update strong-remoting dep Allow accessType per remote method v2.9.0 Update juggler dep v2.8.8 Fix context middleware to preserve domains Fix Geo test cases Allow User.hashPassword/validatePassword to be overridden Additional password reset unit tests for API and REST - strongloop#944 Small formatting update to have consistency with identical logic in other areas. - strongloop#944 Simplify the API test for invalidCredentials (removed create), move above REST calls for better grouping of tests - strongloop#944 Force request to send body as string, this ensures headers aren't automatically set to application/json - strongloop#944 Ensure error checking logic is in place for all REST calls, expand formatting for consistency with existing instances. - strongloop#944 Correct invalidCredentials so that it differs from validCredentialsEmailVerified, unit test now passes as desired. - strongloop#944 Update to demonstrate unit test is actually failing due to incorrect values of invalidCredentials - strongloop#944 v2.8.7 API and REST tests added to ensure complete and valid credentials are supplied for verified error message to be returned - tests added as suggested and fail under previous version of User model - strongloop#931 Require valid login credentials before verified email check. - strongloop#931. Change urlNotFound.js to url-not-found.js Add lib/server-app.js ... Conflicts: common/models/user.js
2.10.0 * Revert the peer dep change to avoid npm complaints (Raymond Feng) * Update strong-remoting dep (Raymond Feng) * Allow accessType per remote method (Raymond Feng) * API and REST tests added to ensure complete and valid credentials are supplied for verified error message to be returned - tests added as suggested and fail under previous version of User model - #931 (Ron Edgecomb) * Require valid login credentials before verified email check. - #931. (Ron Edgecomb)
I would suggest moving the User model
emailVerificationRequiredcheck after successful matching of the password (supplied to remote/loginrequest). This would require that the user supply a valid username/password combination before the email verification check is made.For the scenario where an email is not verified, leaving it as is makes it possible to check if a username or email address exists. A password doesn't have to be supplied to the remote
/logincall, just either theusernameoremail.This may seem like trivial suggestion but it's an easy fix to prevent this type of behavior.
The text was updated successfully, but these errors were encountered: