Adding fall back method for sha1 in case default algo is sha256
#33345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
awais786
changed the title
sha1 has been deprecated in django32 and removed in django42.sha1 has been deprecated in django32 and removed in django42.
iamsobanjaved
previously approved these changes
Sep 26, 2023
iamsobanjaved
dismissed
their stale review
Found multiple instances of sha1
|
It looks like most/all of the |
awais786
force-pushed
the
removing-sha1
branch
from
1b1c74c to
2c5c164
Compare
iamsobanjaved
force-pushed
the
removing-sha1
branch
from
ec4df89 to
8cb5472
Compare
awais786
changed the title
sha1 has been deprecated in django32 and removed in django42.sha1 in case default algo is sha256
robrap
reviewed
Oct 3, 2023
|
@robrap Just fixing tests, will then implement your suggestions regarding monitoring |
robrap
reviewed
Oct 4, 2023
robrap
reviewed
Oct 4, 2023
iamsobanjaved
force-pushed
the
removing-sha1
branch
from
8e9213c to
53be61a
Compare
robrap
reviewed
Oct 5, 2023
robrap
reviewed
Oct 5, 2023
iamsobanjaved
force-pushed
the
removing-sha1
branch
from
a9603ba to
62b57ad
Compare
robrap
reviewed
Oct 6, 2023
robrap
reviewed
Oct 6, 2023
I was wondering about all the cases, so I updated the test to reflect this. I also made some other minor adjustments.
robrap
approved these changes
Oct 11, 2023
There was a problem hiding this comment.
Thanks so much for this.
A possible deployment plan:
- Merge
- Create (and share) a small New Relic dashboard (ask if you need help). If you FACET by appName (or add separate charts for Stage), we can watch in Stage for a bit before going to Prod.
- Enable in Stage and ensure all is well. (Maybe leaves this for a day?)
- Enable in Prod.
Thoughts?
|
Yeah, this is what we had in our minds for deployment. I will try to create a New Relic dashboard and surely ask for help if I will get stuck. Also merging this PR now. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
iamsobanjaved
added a commit
that referenced
this pull request
Oct 13, 2023
…a256` (#33345) --------- Co-authored-by: Muhammad Soban Javed <[email protected]> Co-authored-by: Robert Raposa <[email protected]> Co-authored-by: Muhammad Soban Javed <[email protected]>
shURenZHOUluxun
pushed a commit
to EduTrigger/edx-platform
that referenced
this pull request
Jan 3, 2024
…a256` (openedx#33345) --------- Co-authored-by: Muhammad Soban Javed <[email protected]> Co-authored-by: Robert Raposa <[email protected]> Co-authored-by: Muhammad Soban Javed <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://docs.djangoproject.com/en/3.2/ref/settings/#default-hashing-algorithm
https://docs.djangoproject.com/en/4.2/releases/4.0/#features-removed-in-4-0
The
DEFAULT_HASHING_ALGORITHMtransitional setting is removed.its defined in django-global settings as 256https://github.com/django/django/blob/stable/3.2.x/django/conf/global_settings.py#L450
Initially we were assuming that platform is going to usesha256as default algorithm for every thing. But now we got clarity thatsafesessionmiddlewarewill use thesha1so created new PR with new variable.Testing steps1. create sandbox with master branch and login on sandbox.2. install
django42and restartlmsonly.3. keep continue using the site, some
middlewarecall that method. It will mismatch due to algorithm and logouts the user.4. Also moving to
django42will invalidates the forgot passwords links generated viadjango32.Picking
SHA256value from internal config for stage. https://github.com/edx/edx-internal/pull/9332For production it will remain
sha1.