mirror of
https://github.com/andwati/zitadel.git
synced 2026-11-03 21:02:15 +00:00
chore: update passwap v0.12.1 and align hash validation defaults/errors (#12179)
# Which Problems Are Solved - Upgrading to `zitadel/passwap` v0.12.1 introduced new encoded-hash validation paths that still had review feedback open. - Secret hasher defaults were internally inconsistent (`Hasher.Cost: 4` vs `Limits.Bcrypt.MinCost: 10`), which could reject hashes created by the configured hasher. - New validation error IDs/messages and test coverage needed to be aligned with project conventions and expected behavior branches. # How the Problems Are Solved - Kept the dependency upgrade to `zitadel/passwap` v0.12.1 and completed the validation integration. - Updated `ValidateEncodedHash` error handling in `internal/crypto/passwap.go` to: - use unique random-style error IDs, - return `Errors.Hash.NotSupported` for no-verifier cases, - keep invalid-hash branches mapped to invalid argument errors. - Expanded `TestHasher_ValidateEncodedHash` in `internal/crypto/passwap_test.go` to cover and assert: - bounds error branch, - no-verifier branch, - generic invalid-hash branch, - expected ZITADEL error IDs/messages. - Restored lost inline verifier-context comments for argon2 and md5plain verifier entries. # Additional Changes - Added the missing explanatory `Limits` comment for `SecretHasher` in `cmd/defaults.yaml`. - Corrected `SecretHasher.Limits.Bcrypt.MinCost` from `10` to `4` to match the configured default bcrypt cost and avoid configuration footguns. # Additional Context - Follow-up for PR review feedback in https://github.com/zitadel/zitadel/pull/12179#pullrequestreview-4313121965 --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Livio Spring <[email protected]>
This commit is contained in:
@@ -262,7 +262,7 @@ func Test_newHashedSecretWithDefault(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
hasher := &crypto.Hasher{
|
||||
Swapper: passwap.NewSwapper(bcrypt.New(bcrypt.MinCost)),
|
||||
Swapper: passwap.NewSwapper(bcrypt.New(bcrypt.DefaultMinCost, nil)),
|
||||
}
|
||||
defaultConfig := &crypto.GeneratorConfig{
|
||||
Length: 32,
|
||||
|
||||
@@ -293,6 +293,14 @@ func (h plainHasher) Hash(password string) (string, error) {
|
||||
return strings.Join([]string{"", "plain", h.x, password}, "$"), nil
|
||||
}
|
||||
|
||||
func (h plainHasher) Validate(encoded string) (verifier.Result, error) {
|
||||
nodes := strings.Split(encoded, "$")
|
||||
if len(nodes) != 4 || nodes[1] != "plain" {
|
||||
return verifier.Skip, nil
|
||||
}
|
||||
return verifier.OK, nil
|
||||
}
|
||||
|
||||
func (h plainHasher) Verify(encoded, password string) (verifier.Result, error) {
|
||||
nodes := strings.Split(encoded, "$")
|
||||
if len(nodes) != 4 || nodes[1] != "plain" {
|
||||
|
||||
@@ -119,8 +119,8 @@ func (h *AddHuman) Validate(hasher *crypto.Hasher) (err error) {
|
||||
}
|
||||
}
|
||||
if h.EncodedPasswordHash != "" {
|
||||
if !hasher.EncodingSupported(h.EncodedPasswordHash) {
|
||||
return zerrors.ThrowInvalidArgument(nil, "USER-JDk4t", "Errors.User.Password.NotSupported")
|
||||
if err := hasher.ValidateEncodedHash(h.EncodedPasswordHash); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
@@ -374,6 +374,9 @@ func addHumanCommandPassword(ctx context.Context, filter preparation.FilterToQue
|
||||
}
|
||||
|
||||
if human.EncodedPasswordHash != "" {
|
||||
if err := hasher.ValidateEncodedHash(human.EncodedPasswordHash); err != nil {
|
||||
return err
|
||||
}
|
||||
createCmd.AddPasswordData(human.EncodedPasswordHash, human.PasswordChangeRequired)
|
||||
}
|
||||
return nil
|
||||
@@ -569,6 +572,11 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain.
|
||||
return nil, nil, nil, err
|
||||
}
|
||||
}
|
||||
if human.HashedPassword != "" {
|
||||
if err := c.userPasswordHasher.ValidateEncodedHash(human.HashedPassword); err != nil {
|
||||
return nil, nil, nil, err
|
||||
}
|
||||
}
|
||||
|
||||
addedHuman = NewHumanWriteModel(human.AggregateID, orgID)
|
||||
|
||||
|
||||
@@ -228,6 +228,11 @@ func (c *Commands) setPasswordCommand(ctx context.Context, agg *eventstore.Aggre
|
||||
encodedPassword = newEncodedPassword
|
||||
}
|
||||
}
|
||||
if encodedPassword != "" {
|
||||
if err := c.userPasswordHasher.ValidateEncodedHash(encodedPassword); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
// If password is provided, let's check if is compliant with the policy.
|
||||
// If only a encodedPassword is passed, we can skip this.
|
||||
if password != "" {
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"github.com/muhlemmer/gu"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"github.com/zitadel/passwap"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/text/language"
|
||||
|
||||
@@ -4007,7 +4008,7 @@ func TestAddHumanCommand(t *testing.T) {
|
||||
hasher: mockPasswordHasher("x"),
|
||||
},
|
||||
want: Want{
|
||||
ValidationErr: zerrors.ThrowInvalidArgument(nil, "USER-JDk4t", "Errors.User.Password.NotSupported"),
|
||||
ValidationErr: zerrors.ThrowInvalidArgument(passwap.ErrNoVerifier, "CRYPT-2xK7d", "Errors.Hash.NotSupported"),
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -80,8 +80,8 @@ func (h *ChangeHuman) Validate(hasher *crypto.Hasher) (err error) {
|
||||
|
||||
func (p *Password) Validate(hasher *crypto.Hasher) error {
|
||||
if p.EncodedPasswordHash != "" {
|
||||
if !hasher.EncodingSupported(p.EncodedPasswordHash) {
|
||||
return zerrors.ThrowInvalidArgument(nil, "USER-oz74onzvqr", "Errors.User.Password.NotSupported")
|
||||
if err := hasher.ValidateEncodedHash(p.EncodedPasswordHash); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if p.Password == "" && p.EncodedPasswordHash == "" {
|
||||
|
||||
Reference in New Issue
Block a user