TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-12 00:57:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: make user creation errors helpful (#5382)

Browse Source 
* fix: make user creation errors helpful

* fix linting and unit testing errors

* fix linting

* make zitadel config reusable

* fix human validations

* translate ssr errors

* make zitadel config reusable

* cover more translations for ssr

* handle email validation message centrally

* fix unit tests

* fix linting

* align signatures

* use more precise wording

* handle phone validation message centrally

* fix: return specific profile errors

* docs: edit comments

* fix unit tests

---------

Co-authored-by: Silvan <silvan.reusser@gmail.com>
This commit is contained in:
Elio Bischof
2023-03-14 20:20:38 +01:00
committed by GitHub
parent 9ff810eb92
commit e00cc187fa
79 changed files with 610 additions and 485 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
internal/command/email.go
View File

@@ -10,12 +10,12 @@ import (
)
type Email struct {
Address string
Address domain.EmailAddress
Verified bool
}
func (e *Email) Valid() bool {
return e.Address != "" && domain.EmailRegex.MatchString(e.Address)
func (e *Email) Validate() error {
return e.Address.Validate()
}
func newEmailCode(ctx context.Context, filter preparation.FilterToQueryReducer, alg crypto.EncryptionAlgorithm) (value *crypto.CryptoValue, expiry time.Duration, err error) {

16
internal/command/phone.go
View File

@@ -4,30 +4,16 @@ import (
"context"
"time"
"github.com/ttacon/libphonenumber"
"github.com/zitadel/zitadel/internal/command/preparation"
"github.com/zitadel/zitadel/internal/crypto"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/errors"
)
type Phone struct {
Number string
Number domain.PhoneNumber
Verified bool
}
func FormatPhoneNumber(number string) (string, error) {
if number == "" {
return "", nil
}
phoneNr, err := libphonenumber.Parse(number, libphonenumber.UNKNOWN_REGION)
if err != nil {
return "", errors.ThrowInvalidArgument(nil, "EVENT-so0wa", "Errors.User.Phone.Invalid")
}
number = libphonenumber.Format(phoneNr, libphonenumber.E164)
return number, nil
}
func newPhoneCode(ctx context.Context, filter preparation.FilterToQueryReducer, alg crypto.EncryptionAlgorithm) (value *crypto.CryptoValue, expiry time.Duration, err error) {
return newCryptoCodeWithExpiry(ctx, filter, domain.SecretGeneratorTypeVerifyPhoneCode, alg)
}

10
internal/command/phone_test.go
View File

@@ -3,12 +3,13 @@ package command
import (
"testing"
"github.com/zitadel/zitadel/internal/domain"
"github.com/zitadel/zitadel/internal/errors"
)
func TestFormatPhoneNumber(t *testing.T) {
type args struct {
number string
number domain.PhoneNumber
}
tests := []struct {
name string
@@ -44,10 +45,9 @@ func TestFormatPhoneNumber(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
formatted, err := FormatPhoneNumber(tt.args.number)
if tt.errFunc == nil && tt.result.Number != formatted {
t.Errorf("got wrong result: expected: %v, actual: %v ", tt.args.number, formatted)
normalized, err := tt.args.number.Normalize()
if tt.errFunc == nil && tt.result.Number != normalized {
t.Errorf("got wrong result: expected: %v, actual: %v ", tt.result.Number, normalized)
}
if tt.errFunc != nil && !tt.errFunc(err) {
t.Errorf("got wrong err: %v ", err)

47
internal/command/user_human.go
View File

@@ -104,29 +104,31 @@ func (c *Commands) AddHuman(ctx context.Context, resourceOwner string, human *Ad
type humanCreationCommand interface {
eventstore.Command
AddPhoneData(phoneNumber string)
AddPhoneData(phoneNumber domain.PhoneNumber)
AddPasswordData(secret *crypto.CryptoValue, changeRequired bool)
}
func AddHumanCommand(a *user.Aggregate, human *AddHuman, passwordAlg crypto.HashAlgorithm, codeAlg crypto.EncryptionAlgorithm) preparation.Validation {
return func() (_ preparation.CreateCommands, err error) {
if !human.Email.Valid() {
return nil, errors.ThrowInvalidArgument(nil, "USER-Ec7dM", "Errors.Invalid.Argument")
if err := human.Email.Validate(); err != nil {
return nil, err
}
if human.Username = strings.TrimSpace(human.Username); human.Username == "" {
return nil, errors.ThrowInvalidArgument(nil, "V2-zzad3", "Errors.Invalid.Argument")
}
if human.FirstName = strings.TrimSpace(human.FirstName); human.FirstName == "" {
return nil, errors.ThrowInvalidArgument(nil, "USER-UCej2", "Errors.Invalid.Argument")
return nil, errors.ThrowInvalidArgument(nil, "USER-UCej2", "Errors.User.Profile.FirstNameEmpty")
}
if human.LastName = strings.TrimSpace(human.LastName); human.LastName == "" {
return nil, errors.ThrowInvalidArgument(nil, "USER-DiAq8", "Errors.Invalid.Argument")
return nil, errors.ThrowInvalidArgument(nil, "USER-4hB7d", "Errors.User.Profile.LastNameEmpty")
}
human.ensureDisplayName()
if human.Phone.Number, err = FormatPhoneNumber(human.Phone.Number); err != nil {
return nil, errors.ThrowInvalidArgument(nil, "USER-tD6ax", "Errors.Invalid.Argument")
if human.Phone.Number != "" {
if human.Phone.Number, err = human.Phone.Number.Normalize(); err != nil {
return nil, err
}
}
return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) {
@@ -387,19 +389,12 @@ func (c *Commands) RegisterHuman(ctx context.Context, orgID string, human *domai
return writeModelToHuman(registeredHuman), nil
}
func (c *Commands) addHuman(ctx context.Context, orgID string, human *domain.Human, domainPolicy *domain.DomainPolicy, pwPolicy *domain.PasswordComplexityPolicy, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator crypto.Generator) ([]eventstore.Command, *HumanWriteModel, error) {
if orgID == "" || !human.IsValid() {
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-67Ms8", "Errors.User.Invalid")
}
if human.Password != nil && human.Password.SecretString != "" {
human.Password.ChangeRequired = true
}
return c.createHuman(ctx, orgID, human, nil, false, false, domainPolicy, pwPolicy, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator)
}
func (c *Commands) importHuman(ctx context.Context, orgID string, human *domain.Human, passwordless bool, links []*domain.UserIDPLink, domainPolicy *domain.DomainPolicy, pwPolicy *domain.PasswordComplexityPolicy, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator, passwordlessCodeGenerator crypto.Generator) (events []eventstore.Command, humanWriteModel *HumanWriteModel, passwordlessCodeWriteModel *HumanPasswordlessInitCodeWriteModel, code string, err error) {
if orgID == "" || !human.IsValid() {
return nil, nil, nil, "", errors.ThrowInvalidArgument(nil, "COMMAND-00p2b", "Errors.User.Invalid")
if orgID == "" {
return nil, nil, nil, "", errors.ThrowInvalidArgument(nil, "COMMAND-00p2b", "Errors.Org.Empty")
}
if err := human.Normalize(); err != nil {
return nil, nil, nil, "", err
}
events, humanWriteModel, err = c.createHuman(ctx, orgID, human, links, false, passwordless, domainPolicy, pwPolicy, initCodeGenerator, emailCodeGenerator, phoneCodeGenerator)
if err != nil {
@@ -421,10 +416,16 @@ func (c *Commands) registerHuman(ctx context.Context, orgID string, human *domai
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-JKefw", "Errors.User.Invalid")
}
if human.Username = strings.TrimSpace(human.Username); human.Username == "" {
human.Username = human.EmailAddress
human.Username = string(human.EmailAddress)
}
if orgID == "" || !human.IsValid() || link == nil && (human.Password == nil || human.Password.SecretString == "") {
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-9dk45", "Errors.User.Invalid")
if orgID == "" {
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-hYsVH", "Errors.Org.Empty")
}
if err := human.Normalize(); err != nil {
return nil, nil, err
}
if link == nil && (human.Password == nil || human.Password.SecretString == "") {
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-X23na", "Errors.User.Password.Empty")
}
if human.Password != nil && human.Password.SecretString != "" {
human.Password.ChangeRequired = false
@@ -441,7 +442,7 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain.
return nil, nil, err
}
human.Username = strings.TrimSpace(human.Username)
human.EmailAddress = strings.TrimSpace(human.EmailAddress)
human.EmailAddress = human.EmailAddress.Normalize()
if !domainPolicy.UserLoginMustBeDomain {
index := strings.LastIndex(human.Username, "@")
if index > 1 {

7
internal/command/user_human_email.go
View File

@@ -13,8 +13,11 @@ import (
)
func (c *Commands) ChangeHumanEmail(ctx context.Context, email *domain.Email, emailCodeGenerator crypto.Generator) (*domain.Email, error) {
if !email.IsValid() || email.AggregateID == "" {
return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-4M9sf", "Errors.Email.Invalid")
if email.AggregateID == "" {
return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-0Gzs3", "Errors.User.Email.IDMissing")
}
if err := email.Validate(); err != nil {
return nil, err
}
existingEmail, err := c.emailWriteModel(ctx, email.AggregateID, email.ResourceOwner)

4
internal/command/user_human_email_model.go
View File

@@ -14,7 +14,7 @@ import (
type HumanEmailWriteModel struct {
eventstore.WriteModel
Email string
Email domain.EmailAddress
IsEmailVerified bool
Code *crypto.CryptoValue
@@ -95,7 +95,7 @@ func (wm *HumanEmailWriteModel) Query() *eventstore.SearchQueryBuilder {
func (wm *HumanEmailWriteModel) NewChangedEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
email string,
email domain.EmailAddress,
) (*user.HumanEmailChangedEvent, bool) {
if wm.Email == email {
return nil, false

4
internal/command/user_human_init.go
View File

@@ -11,8 +11,8 @@ import (
"github.com/zitadel/zitadel/internal/repository/user"
)
//ResendInitialMail resend inital mail and changes email if provided
func (c *Commands) ResendInitialMail(ctx context.Context, userID, email, resourceOwner string, initCodeGenerator crypto.Generator) (objectDetails *domain.ObjectDetails, err error) {
// ResendInitialMail resend initial mail and changes email if provided
func (c *Commands) ResendInitialMail(ctx context.Context, userID string, email domain.EmailAddress, resourceOwner string, initCodeGenerator crypto.Generator) (objectDetails *domain.ObjectDetails, err error) {
if userID == "" {
return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-2n8vs", "Errors.User.UserIDMissing")
}

4
internal/command/user_human_init_model.go
View File

@@ -14,7 +14,7 @@ import (
type HumanInitCodeWriteModel struct {
eventstore.WriteModel
Email string
Email domain.EmailAddress
IsEmailVerified bool
Code *crypto.CryptoValue
@@ -92,7 +92,7 @@ func (wm *HumanInitCodeWriteModel) Query() *eventstore.SearchQueryBuilder {
func (wm *HumanInitCodeWriteModel) NewChangedEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
email string,
email domain.EmailAddress,
) (*user.HumanEmailChangedEvent, bool) {
changedEvent := user.NewHumanEmailChangedEvent(ctx, aggregate, email)
return changedEvent, wm.Email != email

2
internal/command/user_human_init_test.go
View File

@@ -289,7 +289,7 @@ func TestCommandSide_ResendInitialMail(t *testing.T) {
r := &Commands{
eventstore: tt.fields.eventstore,
}
got, err := r.ResendInitialMail(tt.args.ctx, tt.args.userID, tt.args.email, tt.args.resourceOwner, tt.args.secretGenerator)
got, err := r.ResendInitialMail(tt.args.ctx, tt.args.userID, domain.EmailAddress(tt.args.email), tt.args.resourceOwner, tt.args.secretGenerator)
if tt.res.err == nil {
assert.NoError(t, err)
}

4
internal/command/user_human_model.go
View File

@@ -22,10 +22,10 @@ type HumanWriteModel struct {
Gender domain.Gender
Avatar string
Email string
Email domain.EmailAddress
IsEmailVerified bool
Phone string
Phone domain.PhoneNumber
IsPhoneVerified bool
Country string

2
internal/command/user_human_otp.go
View File

@@ -69,7 +69,7 @@ func (c *Commands) AddHumanOTP(ctx context.Context, userID, resourceowner string
accountName := domain.GenerateLoginName(human.GetUsername(), org.PrimaryDomain, orgPolicy.UserLoginMustBeDomain)
if accountName == "" {
accountName = human.EmailAddress
accountName = string(human.EmailAddress)
}
key, secret, err := domain.NewOTPKey(c.multifactors.OTP.Issuer, accountName, c.multifactors.OTP.CryptoMFA)
if err != nil {

5
internal/command/user_human_phone.go
View File

@@ -14,10 +14,9 @@ import (
)
func (c *Commands) ChangeHumanPhone(ctx context.Context, phone *domain.Phone, resourceOwner string, phoneCodeGenerator crypto.Generator) (*domain.Phone, error) {
if !phone.IsValid() {
return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-6M0ds", "Errors.Phone.Invalid")
if err := phone.Normalize(); err != nil {
return nil, err
}
existingPhone, err := c.phoneWriteModelByID(ctx, phone.AggregateID, resourceOwner)
if err != nil {
return nil, err

4
internal/command/user_human_phone_model.go
View File

@@ -14,7 +14,7 @@ import (
type HumanPhoneWriteModel struct {
eventstore.WriteModel
Phone string
Phone domain.PhoneNumber
IsPhoneVerified bool
Code *crypto.CryptoValue
@@ -107,7 +107,7 @@ func (wm *HumanPhoneWriteModel) Query() *eventstore.SearchQueryBuilder {
func (wm *HumanPhoneWriteModel) NewChangedEvent(
ctx context.Context,
aggregate *eventstore.Aggregate,
phone string,
phone domain.PhoneNumber,
) (*user.HumanPhoneChangedEvent, bool) {
changedEvent := user.NewHumanPhoneChangedEvent(ctx, aggregate, phone)
return changedEvent, phone != wm.Phone

8
internal/command/user_human_profile.go
View File

@@ -9,10 +9,12 @@ import (
)
func (c *Commands) ChangeHumanProfile(ctx context.Context, profile *domain.Profile) (*domain.Profile, error) {
if !profile.IsValid() && profile.AggregateID != "" {
return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-8io0d", "Errors.User.Profile.Invalid")
if profile.AggregateID == "" {
return nil, caos_errs.ThrowPreconditionFailed(nil, "COMMAND-AwbEB", "Errors.User.Profile.IDMissing")
}
if err := profile.Validate(); err != nil {
return nil, err
}
existingProfile, err := c.profileWriteModelByID(ctx, profile.AggregateID, profile.ResourceOwner)
if err != nil {
return nil, err

10
internal/command/user_human_test.go
View File

@@ -3445,7 +3445,7 @@ func newAddHumanEvent(password string, changeRequired bool, phone string) *user.
changeRequired)
}
if phone != "" {
event.AddPhoneData(phone)
event.AddPhoneData(domain.PhoneNumber(phone))
}
return event
}
@@ -3473,7 +3473,7 @@ func newRegisterHumanEvent(username, password string, changeRequired bool, phone
changeRequired)
}
if phone != "" {
event.AddPhoneData(phone)
event.AddPhoneData(domain.PhoneNumber(phone))
}
return event
}
@@ -3503,7 +3503,7 @@ func TestAddHumanCommand(t *testing.T) {
},
},
want: Want{
ValidationErr: errors.ThrowInvalidArgument(nil, "USER-Ec7dM", "Errors.Invalid.Argument"),
ValidationErr: errors.ThrowInvalidArgument(nil, "EMAIL-599BI", "Errors.User.Email.Invalid"),
},
},
{
@@ -3519,7 +3519,7 @@ func TestAddHumanCommand(t *testing.T) {
},
},
want: Want{
ValidationErr: errors.ThrowInvalidArgument(nil, "USER-UCej2", "Errors.Invalid.Argument"),
ValidationErr: errors.ThrowInvalidArgument(nil, "USER-UCej2", "Errors.User.Profile.FirstNameEmpty"),
},
},
{
@@ -3534,7 +3534,7 @@ func TestAddHumanCommand(t *testing.T) {
},
},
want: Want{
ValidationErr: errors.ThrowInvalidArgument(nil, "USER-DiAq8", "Errors.Invalid.Argument"),
ValidationErr: errors.ThrowInvalidArgument(nil, "USER-4hB7d", "Errors.User.Profile.LastNameEmpty"),
},
},
{

2
internal/command/user_human_webauthn.go
View File

@@ -155,7 +155,7 @@ func (c *Commands) addHumanWebAuthN(ctx context.Context, userID, resourceowner s
}
accountName := domain.GenerateLoginName(user.GetUsername(), org.PrimaryDomain, orgPolicy.UserLoginMustBeDomain)
if accountName == "" {
accountName = user.EmailAddress
accountName = string(user.EmailAddress)
}
webAuthN, err := c.webauthnConfig.BeginRegistration(ctx, user, accountName, authenticatorPlatform, userVerification, isLoginUI, tokens...)
if err != nil {