Skip to content

Check passwords against HaveIBeenPwned#12716

Merged
jolheiser merged 11 commits into
go-gitea:masterfrom
jolheiser:pwn
Sep 8, 2020
Merged

Check passwords against HaveIBeenPwned#12716
jolheiser merged 11 commits into
go-gitea:masterfrom
jolheiser:pwn

Conversation

@jolheiser
Copy link
Copy Markdown
Member

This PR implements a way to check a password against HaveIBeenPwned.

Something to note, I made a decision to add padding to all requests for extra security. Since we are using the service solely to check passwords for a user (vs some automated security script), the added overhead should be fairly negligible.

You can read more about HaveIBeenPwned's padding here: https://haveibeenpwned.com/API/v3#PwnedPasswordsPadding

Signed-off-by: jolheiser <john.olheiser@gmail.com>
@jolheiser jolheiser added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Sep 4, 2020
@jolheiser jolheiser added this to the 1.14.0 milestone Sep 4, 2020
Comment thread vendor/go.jolheiser.com/pwn/pwn.go Outdated
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 4, 2020
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Comment thread cmd/admin.go Outdated
Comment thread options/locale/locale_en-US.ini Outdated
jolheiser and others added 4 commits September 4, 2020 15:24
Co-authored-by: mrsdizzie <info@mrsdizzie.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@jolheiser
Copy link
Copy Markdown
Member Author

@mrsdizzie Done. I also added the link to the admin command.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 4, 2020

Codecov Report

Merging #12716 into master will decrease coverage by 0.00%.
The diff coverage is 10.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12716      +/-   ##
==========================================
- Coverage   43.25%   43.24%   -0.01%     
==========================================
  Files         648      649       +1     
  Lines       71928    72000      +72     
==========================================
+ Hits        31111    31137      +26     
- Misses      35775    35819      +44     
- Partials     5042     5044       +2     
Impacted Files Coverage Δ
cmd/admin.go 0.00% <0.00%> (ø)
routers/api/v1/admin/user.go 16.02% <0.00%> (-1.46%) ⬇️
routers/user/setting/account.go 26.04% <0.00%> (-0.85%) ⬇️
routers/admin/users.go 25.71% <5.55%> (-1.89%) ⬇️
routers/user/auth.go 11.59% <5.55%> (-0.12%) ⬇️
modules/password/pwn.go 37.50% <37.50%> (ø)
modules/password/password.go 70.83% <50.00%> (-2.50%) ⬇️
modules/setting/setting.go 47.61% <100.00%> (+0.11%) ⬆️
modules/indexer/stats/db.go 43.47% <0.00%> (-17.40%) ⬇️
modules/indexer/stats/queue.go 64.70% <0.00%> (-11.77%) ⬇️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bea343c...ab26504. Read the comment docs.

Comment thread modules/password/pwn.go Outdated
Comment thread modules/password/pwn.go
Comment thread routers/api/v1/admin/user.go Outdated
Comment thread routers/api/v1/admin/user.go Outdated
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Signed-off-by: jolheiser <john.olheiser@gmail.com>
Comment thread modules/password/password.go
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 5, 2020
Comment thread cmd/admin.go
Comment thread docs/content/doc/advanced/config-cheat-sheet.en-us.md
Comment thread options/locale/locale_en-US.ini
@jolheiser
Copy link
Copy Markdown
Member Author

@lafriks Let me know if you still think they should be changed, but for the moment I've resolved the issues as HIBP capitalizes the URL part.

@GiteaBot GiteaBot removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Sep 8, 2020
@GiteaBot GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Sep 8, 2020
@mrsdizzie
Copy link
Copy Markdown
Member

lgtm -- also wouldn't mind this being in 1.13 as a better alternative to the password complexity feature : )

@jolheiser jolheiser modified the milestones: 1.14.0, 1.13.0 Sep 8, 2020
@jolheiser jolheiser merged commit c6e4bc5 into go-gitea:master Sep 8, 2020
@jolheiser jolheiser deleted the pwn branch September 8, 2020 22:06
@mrsdizzie mrsdizzie added the release/highlight Marks a PR as a highlight-worthy change for the release notes. label Sep 8, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the release/highlight Marks a PR as a highlight-worthy change for the release notes. label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants