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

Expand authtkt checksums #24

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@lod
Contributor

lod commented Apr 26, 2016

This fixes #22, authtkt only supporting MD5 checksums

It is an alternative to pull-request #23 by @frostyfrog
The main differences are:

  • I tried to remain as faithful as possible to the upstream paste/authtkt implementation
  • I did not change the default from MD5, to maintain compatibility
  • I got carried away and took things a bit further

The pull-request is a touch large but broken in to several commits to allow cherrypicking or elements to be carved off as desired.

Work done was:

  1. Pulled in changes from paste/authtkt which expand the supported checksums. I based these off paste HEAD rather than try and apply their patches. All changes to _auth_tkt.py where based on upstream however it is not a synchronised file, changes made to the repoze branch were retained.
  2. Expanded test coverage in test__auth_tkt.py to cover alternate digests.
  3. Tweaked travis and tox files to fix an issue with the latest virtualenv no longer supporting python 3.2. Testing was also expanded to cover python 3.5.
  4. Added alternate hash support to plugins/auth_tkt.py and expanded test coverage.
  5. Modified documentation to cover alternative digests. The boilerplate examples have all been updated to use sha512.

lod added some commits Apr 18, 2016

Modified _auth_tkt in line with paste upstream
Refreshed _auth_tkt to bring it in line with upstream paste auth_tkt.py
This brings with it support for non-MD5 digests.

All changes to _auth_tkt.py came from upstream.
It is not however a synchronised file, repoze variations have been
retained.

Changes were made to test__auth_tkt.py to allow tests to pass.
The number of arguments for calculate_digest() has changed requiring the
tests to be updated. This is an internal function and should not impact
users.
Fix travis test execution
Virtualenv 14.0.0 no longer supports Python 3.2
Work around this by pegging the virtualenv version

Added Python 3.5 (current stable) to the testing matrix
This has issues with calling tox from travis travis-ci/travis-ci#4794
Work around this by making the travis environment use python 3.5 so it
is present when tox is invoked.
@frostyfrog

This comment has been minimized.

Show comment
Hide comment
@frostyfrog

frostyfrog Apr 29, 2016

I know that it's a good idea to maintain the old behavior, but... Is it a good idea to leave the default at something insecure like MD5? Honestly, I like how you've structured the commit. I could learn from it 😄 Thank you! 😃

frostyfrog commented Apr 29, 2016

I know that it's a good idea to maintain the old behavior, but... Is it a good idea to leave the default at something insecure like MD5? Honestly, I like how you've structured the commit. I could learn from it 😄 Thank you! 😃

@tseaver

This comment has been minimized.

Show comment
Hide comment
@tseaver

tseaver May 31, 2016

Member

I've merged this branch, dropping the conflicting commit which tweaked .travis.yml/tox.ini.

Member

tseaver commented May 31, 2016

I've merged this branch, dropping the conflicting commit which tweaked .travis.yml/tox.ini.

@tseaver

This comment has been minimized.

Show comment
Hide comment
@tseaver

tseaver May 31, 2016

Member

@lod, @frostyfrog Thank you for your efforts!

Member

tseaver commented May 31, 2016

@lod, @frostyfrog Thank you for your efforts!

tseaver added a commit that referenced this pull request May 31, 2016

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