[WIP] Refactor scrypt hashing #5678
Conversation
|
This pull request introduces 1 alert when merging dbd5227 into 405d0cf - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 847e750 into 405d0cf - view on LGTM.com new alerts:
|
|
Hi @nabla-c0d3, Thanks for starting on this. I wanted to let you know that our availability to review is going to be limited through the end of the year, so if you get it ready and don't hear back, that's why. From a cursory glance it looks like one benefit of this will be partially addressing #5613, which is very welcome. We'll try to take a look as soon as we can. You might want to just check in with us before starting any changes this big after the first of the year. I absolutely do not want to discourage these refactorings, but the looming end of support for Ubuntu Xenial is going to be taking a lot of our time, and it may take us longer to schedule review for large PRs that aren't related to that. |
|
@rmol no problem and thanks for the heads-up. My plan is to finish this PR, and then see if it is feasible to split it into smaller ones. The initial goal was to refactor CryptoUtil.hash_codename() but getting to the "best" design I could think of required more changes and refactoring. But anyway, nothing in these changes is urgent. |
nabla-c0d3
force-pushed
the
refactor-scrypt-hashing
branch
from
5f4bfd1
to
87203a7
Compare
nabla-c0d3
force-pushed
the
refactor-scrypt-hashing
branch
from
e7f8773
to
7211159
Compare
Fix test Fix tests
Remove hash_codename() Save TODOs Get rid of flask.g Get rid of flask.g Move session expiration to SessionManager Remove scrypt as a requirement Remove noqa Delete source session on logout Remove TODO Remove usage of flask.g Use SimpleCache to store source user session Fix bug Clarify name Fix some tests Fix test Fix test Remove testing code Fix test Fix flake8 Fix test Fix test Fix test Fix test Fix test Fix test Fix tests Fix linting Fix test Fix tests Fix flake8 Less coupling with source_app Add tests for source_user.py Fix implementation of default SourceScryptManager Re-implement source_app fixture to make tests faster Move new fixtures to conftest Fix linting Fix type
nabla-c0d3
force-pushed
the
refactor-scrypt-hashing
branch
from
7211159
to
c2268af
Compare
|
This pull request introduces 2 alerts when merging c2268af into 7f16b5f - view on LGTM.com new alerts:
|
|
Replaced by #5692 |
WIP...
Status
Work in progress
Description of Changes
TBD
Fixes #.
Changes proposed in this pull request:
Testing
TBD
How should the reviewer test this PR?
Write out any special testing steps here.