Skip to content

use MD5 password hasher under test #130

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

Merged
merged 1 commit into from
Apr 10, 2020
Merged

Conversation

chris48s
Copy link
Contributor

@chris48s chris48s commented Apr 8, 2020

This restores the optimisation I removed in #109 to get the tests working.
Now that we're using Factories rather than fixtures, we can use what we want under test.

A good production password hashing algorithm is slow by design and hard to optimise. This makes it difficult to brute-force. The reason cookiecutter suggests this is because MD5 is not very secure but it is fast to calculate. We don't care about security of data we generate while running tests, but we do want it to be fast to build, so using a known vulnerable hash algorithm makes sense here. Its a bit of a micro-optimisation, but its definitely not going to make the tests run any slower, so we might as well re-enable it now.

@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #130 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #130   +/-   ##
=======================================
  Coverage   76.33%   76.33%           
=======================================
  Files          28       28           
  Lines         393      393           
=======================================
  Hits          300      300           
  Misses         93       93           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 412d7dd...008a04e. Read the comment docs.

Copy link
Member

@lpatmo lpatmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Thanks for the context, and feel free to merge. :)

@chris48s chris48s merged commit c5eb5b6 into codebuddies:master Apr 10, 2020
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.

2 participants