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

Add method that does not make use of the real password #18

Merged

Conversation

tacovandenbroek
Copy link
Contributor

However unlikely, passing a real password to a library is a risk in itself. Since there is no need to do that in this case we should provide a method to check a password by it's SHA-1 hash.

@DivineOmega
Copy link
Owner

This looks good to me. Thanks. 👍

Would you mind adding some tests specifically for the passwordExposedByHash method, and then I'll get this merged? Feel free to just copy the code from PasswordExposedTest.php and supply the method with pre-hashed values instead of the plaintext password.

@tacovandenbroek tacovandenbroek force-pushed the use_sha1_for_password_check branch from c1e9b27 to fee4a51 Compare December 6, 2018 08:56
@tacovandenbroek
Copy link
Contributor Author

After some trouble with the styleci check, I've managed to add those tests ;)

@DivineOmega
Copy link
Owner

Thanks for adding the tests! 🙂

Just added some documentation for the new method. This will go into a new release shortly.

@DivineOmega DivineOmega merged commit cc48b59 into DivineOmega:master Dec 6, 2018
@coveralls
Copy link

coveralls commented Dec 8, 2018

Pull Request Test Coverage Report for Build 115

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 89.13%

Totals Coverage Status
Change from base Build 109: -0.2%
Covered Lines: 41
Relevant Lines: 46

💛 - Coveralls

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.

3 participants