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

Revert crypt hash generation on login #3720

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

norbye
Copy link
Member

@norbye norbye commented Jan 24, 2025

We've been generating crypt hashes on login for users missing it due to it not being set on registration until 8 months ago. Now we're down from 487 users with no hash to 78, so I figured it could be time to remove the temporary update.

If any of the remaining 78 users against all odds require the hash they can reset their password.

Originally implemented in #3602 and #3603

Resolves ABA-942

Copy link

linear bot commented Jan 24, 2025

@norbye norbye added the review-needed Pull requests that need review label Jan 24, 2025
@norbye norbye requested a review from a team January 24, 2025 21:44
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to not just wait a little with merging?

@norbye
Copy link
Member Author

norbye commented Jan 24, 2025

Any reason to not just wait a little with merging?

Not really, just wanted to make sure it wasn't eventually forgotten. And I assumed that there wouldn't be that many users that haven't logged in during the last semester that will login this semester, and if I delay this until august I won't be here to remember it 🤷

Originally implemented in #3602 and #3603
@norbye norbye force-pushed the revert-crypt-generation branch from 93392c5 to 13cd44a Compare January 24, 2025 21:58
Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.66%. Comparing base (8c1ee36) to head (13cd44a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3720      +/-   ##
==========================================
- Coverage   89.67%   89.66%   -0.01%     
==========================================
  Files         703      703              
  Lines       22534    22524      -10     
==========================================
- Hits        20207    20197      -10     
  Misses       2327     2327              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Arashfa0301 Arashfa0301 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-needed Pull requests that need review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants