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

Make dependency on argon2 optional #1701

Merged
merged 1 commit into from May 17, 2019

Conversation

Projects
None yet
4 participants
@jeromelebleu
Copy link
Contributor

commented Mar 19, 2019

Description of the issue this PR addresses

Thanks to #1690, Argon2 is now supported as password hasher. However, it is not optional and Argon2 requires a third-party library - that's why Django do not use it as default (see the related documentation). This PR try to make this password hasher optional.

Current behavior before PR

  • argon2 is in the requirements
  • argon2 is imported in modoboa/core/password_hashers/advanced.py
  • ARGON2IDHasher is defined and inherits from PasswordHasher
  • tests depend on argon2

Desired behavior after PR is merged

  • argon2 is not in requirements but in extras
  • argon2 is imported in modoboa/core/password_hashers/advanced.py with a try...except catch
  • ARGON2IDHasher is defined but only inherits from PasswordHasher if argon2 is installed
  • tests related to argon2 are skipped if it is not installed
@codecov

This comment has been minimized.

Copy link

commented Mar 19, 2019

Codecov Report

Merging #1701 into master will decrease coverage by 0.03%.
The diff coverage is 80.64%.

@@            Coverage Diff             @@
##           master    #1701      +/-   ##
==========================================
- Coverage    86.8%   86.76%   -0.04%     
==========================================
  Files         154      154              
  Lines        8032     8038       +6     
==========================================
+ Hits         6972     6974       +2     
- Misses       1060     1064       +4

@jeromelebleu jeromelebleu changed the title Make dependency to argon2 optional Make dependency on argon2 optional Mar 19, 2019

Show resolved Hide resolved test-requirements.txt Outdated

@jeromelebleu jeromelebleu force-pushed the jeromelebleu:argon2-optional branch from 390c421 to aa5d45d May 15, 2019

@tonioo tonioo merged commit 793dba9 into modoboa:master May 17, 2019

2 of 3 checks passed

codecov/project 86.76% (-0.04%) compared to 1af015c
Details
codecov/patch 80.64% of diff hit (target 70%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@camdenorrb

This comment has been minimized.

Copy link

commented Jun 15, 2019

How do you enable argon2 after installation?

@Arvedui

This comment has been minimized.

Copy link
Contributor

commented Jun 16, 2019

The argon2 support has not yet been released, you would need to install current master and make sure the dependencies are installed. Then you can choose it in the settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.