Skip to content

Conversation

@julio-lopez
Copy link
Collaborator

@julio-lopez julio-lopez commented Jun 7, 2024

Rationale: this code path is primarily executed from the server. A potential error, say from a corrupt, unsupported or otherwise invalid user profile should not cause the server to panic (and crash).

It is possible for computePasswordHash to return an error, not just an impossibility.
The issue was introduced in #3725

@codecov
Copy link

codecov bot commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.12%. Comparing base (cb455c6) to head (60c2dff).
Report is 154 commits behind head on master.

Files Patch % Lines
internal/auth/authn_repo.go 40.00% 2 Missing and 1 partial ⚠️
internal/user/user_profile_pw_hash.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3907      +/-   ##
==========================================
+ Coverage   75.86%   77.12%   +1.26%     
==========================================
  Files         470      479       +9     
  Lines       37301    28757    -8544     
==========================================
- Hits        28299    22180    -6119     
+ Misses       7071     4673    -2398     
+ Partials     1931     1904      -27     

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

@julio-lopez julio-lopez force-pushed the fix/is-valid-password-panic branch from 640b130 to 60c2dff Compare June 7, 2024 05:02
@julio-lopez julio-lopez requested a review from jkowalski June 7, 2024 05:49
@julio-lopez julio-lopez changed the title fix(general): avoid panic on error from computing password hash fix(general): avoid panic on error when computing password hash Jun 7, 2024
@julio-lopez julio-lopez marked this pull request as ready for review June 7, 2024 05:49
@julio-lopez julio-lopez enabled auto-merge (squash) June 7, 2024 05:50
@julio-lopez julio-lopez requested review from plar and redgoat650 June 7, 2024 18:45
@julio-lopez julio-lopez changed the title fix(general): avoid panic on error when computing password hash fix(general): avoid panic on computing password hash error Jun 7, 2024
@julio-lopez julio-lopez disabled auto-merge June 7, 2024 19:57
@julio-lopez julio-lopez merged commit adedd1e into kopia:master Jun 7, 2024
@julio-lopez julio-lopez deleted the fix/is-valid-password-panic branch June 7, 2024 20:00
@julio-lopez
Copy link
Collaborator Author

@bathina2 FYI

@julio-lopez julio-lopez changed the title fix(general): avoid panic on computing password hash error fix(general): avoid panic on hashing password error Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant