-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat($rootScope): verifier for MD5 salted passwords #58
base: main
Are you sure you want to change the base?
Conversation
Allow salted passwords hashed with MD5 to be verified. Accept salt as prefix or as suffix of the password. Don't allow to use as hasher. new feature
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
===========================================
- Coverage 100.00% 98.45% -1.55%
===========================================
Files 10 15 +5
Lines 512 583 +71
===========================================
+ Hits 512 574 +62
- Misses 0 5 +5
- Partials 0 4 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the complete PR. Some suggestions on typos and gofmt.
Also added an extra testcase to keep 100% coverage.
), nil | ||
} | ||
|
||
// Verify parses encoded and verfies password against the checksum. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Verify parses encoded and verfies password against the checksum. | |
// Verify parses encoded and verifies password against the checksum. |
} | ||
c.setSaltPasswFunc(id) | ||
if c.saltpasswfunc == nil { | ||
return nil, fmt.Errorf("md5salted unknow identifier: %s", id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return nil, fmt.Errorf("md5salted unknow identifier: %s", id) | |
return nil, fmt.Errorf("md5salted unknown identifier: %s", id) |
Github is being weird now and not attaching pending comments to my review. Give me a minute |
}, | ||
{ | ||
name: "wrong identifier", | ||
args: args{"$md5salted-unknow$foo$foo"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
args: args{"$md5salted-unknow$foo$foo"}, | |
args: args{"$md5salted-unknown$foo$foo"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that should be all :)
args: args{Password}, | ||
want: verifier.OK, | ||
encoded: MD5SaltedEncodedP, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}, | |
}, | |
{ | |
name: "hash decode error", | |
args: args{Password}, | |
want: verifier.Skip, | |
encoded: "$md5salted-prefix$c2FsdA==$~~~~~~~", | |
}, |
"github.com/zitadel/passwap/internal/testvalues" | ||
"reflect" | ||
"testing" | ||
|
||
"github.com/zitadel/passwap/verifier" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"github.com/zitadel/passwap/internal/testvalues" | |
"reflect" | |
"testing" | |
"github.com/zitadel/passwap/verifier" | |
"reflect" | |
"testing" | |
"github.com/zitadel/passwap/internal/testvalues" | |
"github.com/zitadel/passwap/verifier" |
Allow salted passwords hashed with MD5 to be verified. Accept salt as prefix or as suffix of the password. Don't allow to use as hasher.
new feature