Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

True2thepen patch 3 #67

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

true2thepen
Copy link
Contributor

@true2thepen true2thepen commented Nov 30, 2022

Issue Link

#19

Types of changes

  • Bug fix
  • New feature

Description

Added check for username/email in username field on /auth endpoint. This allows user to login in with either username or email in username field as they normally would in WordPress.

How has this been tested?

Tested on dev server running WP 6.1.1 and PHP 8.1

Screenshots (optional)

Checklist:

  • My code is tested.
  • I wrote tests for the impacted area
  • I ran composer check-plugin locally

@true2thepen
Copy link
Contributor Author

This is failing 2 tests. I have yet to discover why. It works on the dev server. I did find during all these failed test that I inadvertently allowed the email field to contain either the username or email. I have since fixed this. Any ideas on why these two tests are failing let me know.

@true2thepen
Copy link
Contributor Author

true2thepen commented Nov 30, 2022

@nicumicle
Help please. Are the following two test possibly incorrect. Both mock a password of 1234 and send a password of 123. It's the only reason I can see that my changes are failing the tests. If not, any suggestions - cause I'm befuddled at the moment. Code works with testing on my dev server.

Use Postman to test. Pass username field with username or email and correct password returns true. Pass incorrect credentials returns false. Pass email field with email and correct password returns true. Pass email field with username and correct password returns false. Pass incorrect credentials returns false.

Error output:
There were 2 errors:

  1. SimpleJWTLoginTests\Services\AuthenticateServiceTest::testSuccessFlowWithFullPayload
    Exception: Wrong user credentials.

/app/simple-jwt-login/src/Services/AuthenticateService.php:119
/app/simple-jwt-login/src/Services/AuthenticateService.php:79
/app/tests/Services/AuthenticateServiceTest.php:293

This test mocks the password to 1234 yet sends a password of 123. Test is supposed to be a successful test.

  1. SimpleJWTLoginTests\Services\AuthenticateServiceTest::testSuccessFlowWithFullPayloadAndPasshash
    Exception: Wrong user credentials.

/app/simple-jwt-login/src/Services/AuthenticateService.php:119
/app/simple-jwt-login/src/Services/AuthenticateService.php:79
/app/tests/Services/AuthenticateServiceTest.php:346

This test mocks a password of 1234, yet sends a password of 123. Test is supposed to be a successful test.

ERRORS!
Tests: 270, Assertions: 602, Errors: 2.

@true2thepen
Copy link
Contributor Author

Found the problem in the tests. When you change the authentication to allow username to contain either the username or the email, you need to account for this in the tests. Which they weren't. Tests are now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant