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 lib/utils more independent #1414

Merged
merged 2 commits into from Feb 1, 2019

Conversation

Projects
None yet
2 participants
@plettich
Copy link
Contributor

plettich commented Jan 31, 2019

  • moving the functions generate_otpkey() and generate_password() to
    lib/crypto allows to remove this dependency from lib/utils.
  • Also the function hexlify_and_unicode() is moved to lib/utils.
  • some cleanup of import statements

Fixes #1413

Merging #1414 into master will increase coverage by 0.07%.
The diff coverage is 100%.
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1414      +/-   ##
==========================================
+ Coverage   96.75%   96.83%   +0.07%
==========================================
Files         144      144
Lines       17214    17325     +111
==========================================
+ Hits        16656    16776     +120
+ Misses        558      549       -9
Impacted Files Coverage Δ
privacyidea/lib/tokens/hotptoken.py 100% <ø> (ø) ⬆️
privacyidea/lib/passwordreset.py 94.91% <100%> (-0.09%) ⬇️
privacyidea/lib/tokens/registrationtoken.py 97.82% <100%> (ø) ⬆️
privacyidea/lib/tokens/tiqrtoken.py 99.34% <100%> (-0.01%) ⬇️
privacyidea/models.py 99.04% <100%> (ø) ⬆️
privacyidea/lib/tokens/HMAC.py 100% <100%> (ø) ⬆️
privacyidea/lib/token.py 95.87% <100%> (-0.01%) ⬇️
privacyidea/lib/tokenclass.py 98.96% <100%> (ø) ⬆️
privacyidea/api/auth.py 99.1% <100%> (-0.03%) ⬇️
privacyidea/lib/tokens/motptoken.py 96.1% <100%> (ø) ⬆️
... and 8 more

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 f98714e...444e5e9. Read the comment docs.

  

Make lib/utils more independent
- moving the functions `generate_otpkey()` and `generate_password()` to
  `lib/crypto` allows to remove this dependency from `lib/utils`.
- Also the function `hexlify_and_unicode()` is moved to `lib/utils`.
- some cleanup of import statements

@plettich plettich requested a review from privacyidea/core Jan 31, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 31, 2019

Codecov Report

Merging #1414 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1414      +/-   ##
==========================================
+ Coverage   96.75%   96.83%   +0.07%     
==========================================
  Files         144      144              
  Lines       17214    17321     +107     
==========================================
+ Hits        16656    16772     +116     
+ Misses        558      549       -9
Impacted Files Coverage Δ
privacyidea/lib/tokens/hotptoken.py 100% <ø> (ø) ⬆️
privacyidea/lib/passwordreset.py 94.91% <100%> (-0.09%) ⬇️
privacyidea/lib/tokens/registrationtoken.py 97.82% <100%> (ø) ⬆️
privacyidea/lib/tokens/tiqrtoken.py 99.34% <100%> (-0.01%) ⬇️
privacyidea/models.py 99.04% <100%> (ø) ⬆️
privacyidea/lib/tokens/HMAC.py 100% <100%> (ø) ⬆️
privacyidea/lib/token.py 95.87% <100%> (-0.01%) ⬇️
privacyidea/lib/tokenclass.py 98.96% <100%> (ø) ⬆️
privacyidea/api/auth.py 99.1% <100%> (-0.03%) ⬇️
privacyidea/lib/tokens/motptoken.py 96.1% <100%> (ø) ⬆️
... and 8 more

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 f98714e...22a489a. Read the comment docs.

@fredreichbier
Copy link
Member

fredreichbier left a comment

Just one tangential comment, everything else looks fine to me!


# On App Engine, this function is not available.
if hasattr(os, 'getpid'):
_pid = os.getpid()
else: # pragma: no cover
# Fake PID
_pid = urandom.randint(0, 100000)
_pid = SystemRandom().randint(0, 100000)

This comment has been minimized.

@fredreichbier

fredreichbier Feb 1, 2019

Member

Hm, do we even use the _pid variable? I don't see any usage :-/ If we don't use it, I guess we could just delete the snippet above.

This comment has been minimized.

@plettich

plettich Feb 1, 2019

Author Contributor

You are right, this was used to initialize a pseudo rng which i removed in #1374
I'll fix this.

This comment has been minimized.

@plettich

plettich Feb 1, 2019

Author Contributor

Fixed in 22a489a

This comment has been minimized.

@fredreichbier

fredreichbier Feb 1, 2019

Member

👍 thanks!

Remove remnants of passlib migration
- in #1374 the calculation of password hashes was removed in favor of the
  passlib module. It looks like not everything related to this code was
  removed...
- and the utils tests seem to work with Python 3 as well.

@fredreichbier fredreichbier merged commit 333d704 into master Feb 1, 2019

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.75%)
Details
codecov/project 96.83% (+0.07%) compared to f98714e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@fredreichbier fredreichbier deleted the 1413/make_utils_more_independent branch Feb 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment