From b1d7433eba21d66ac7b01b4f5b118c456ae93659 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Tue, 3 Jan 2023 09:16:36 +0000 Subject: [PATCH] fix: correct display name when adding an instance (#4930) * fix: handling of default values inside add instance * fix: remove release from 2.16.x branch * chore(lint): show all issues * refactor: instance converter Co-authored-by: Silvan --- .golangci.yaml | 11 +- .../api/grpc/system/instance_converter.go | 194 ++++++++++-------- 2 files changed, 114 insertions(+), 91 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index a09e0ee528..de00b5caf6 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,6 +1,9 @@ issues: new: true - fix: false + # Set to 0 to disable. + max-issues-per-linter: 0 + # Set to 0 to disable. + max-same-issues: 0 run: concurrency: 2 @@ -50,8 +53,6 @@ linters: - gosimple # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false] - govet - # In addition to fixing imports, goimports also formats your code in the same style as gofmt. [fast: true, auto-fix: true] - - goimports # Detects when assignments to existing variables are not used [fast: true, auto-fix: false] - ineffassign # Finds commonly misspelled English words in comments [fast: true, auto-fix: true] @@ -157,6 +158,9 @@ linters: # Checks is file header matches to pattern [fast: true, auto-fix: false] # ignored because we don't write licenses as headers - goheader + # In addition to fixing imports, goimports also formats your code in the same style as gofmt. [fast: true, auto-fix: true] + # ignored in favor of gci + - goimports #deprecated]: Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes [fast: false, auto-fix: false] # ignored in favor of goimports - golint @@ -301,3 +305,4 @@ linters-settings: - standard # Standard section: captures all standard packages. - default # Default section: contains all imports that could not be matched to another section type. - prefix(github.com/zitadel/zitadel) # Custom section: groups all imports with the specified Prefix. + custom-order: true diff --git a/internal/api/grpc/system/instance_converter.go b/internal/api/grpc/system/instance_converter.go index d5a663f08d..9ef1c343df 100644 --- a/internal/api/grpc/system/instance_converter.go +++ b/internal/api/grpc/system/instance_converter.go @@ -18,169 +18,187 @@ import ( ) func CreateInstancePbToSetupInstance(req *system_pb.CreateInstanceRequest, defaultInstance command.InstanceSetup, externalDomain string) *command.InstanceSetup { + instance := defaultInstance if req.InstanceName != "" { - defaultInstance.InstanceName = req.InstanceName - defaultInstance.Org.Name = req.InstanceName + instance.InstanceName = req.InstanceName + instance.Org.Name = req.InstanceName } if req.CustomDomain != "" { - defaultInstance.CustomDomain = req.CustomDomain + instance.CustomDomain = req.CustomDomain } if req.FirstOrgName != "" { - defaultInstance.Org.Name = req.FirstOrgName + instance.Org.Name = req.FirstOrgName } if user := req.GetMachine(); user != nil { - defaultMachine := command.AddMachine{} - if defaultInstance.Org.Machine != nil { - defaultMachine = *defaultInstance.Org.Machine + defaultMachine := instance.Org.Machine + if defaultMachine == nil { + defaultMachine = new(command.AddMachine) } - defaultInstance.Org.Machine = createInstancePbToAddMachine(user, defaultMachine) - defaultInstance.Org.Human = nil - } - if user := req.GetHuman(); user != nil { - defaultHuman := command.AddHuman{} - if defaultInstance.Org.Human != nil { - defaultHuman = *defaultInstance.Org.Human + instance.Org.Machine = createInstancePbToAddMachine(user, *defaultMachine) + instance.Org.Human = nil + } else if user := req.GetHuman(); user != nil { + defaultHuman := instance.Org.Human + if instance.Org.Human != nil { + defaultHuman = new(command.AddHuman) } - defaultInstance.Org.Human = createInstancePbToAddHuman(user, defaultHuman, defaultInstance.DomainPolicy.UserLoginMustBeDomain, defaultInstance.Org.Name, externalDomain) - defaultInstance.Org.Machine = nil + instance.Org.Human = createInstancePbToAddHuman(user, *defaultHuman, instance.DomainPolicy.UserLoginMustBeDomain, instance.Org.Name, externalDomain) + instance.Org.Machine = nil } - if lang := language.Make(req.DefaultLanguage); lang != language.Und { - defaultInstance.DefaultLanguage = lang + if lang := language.Make(req.DefaultLanguage); !lang.IsRoot() { + instance.DefaultLanguage = lang } - return &defaultInstance + return &instance } -func createInstancePbToAddHuman(user *system_pb.CreateInstanceRequest_Human, defaultHuman command.AddHuman, userLoginMustBeDomain bool, org, externalDomain string) *command.AddHuman { - if user.Email != nil { - defaultHuman.Email.Address = user.Email.Email - defaultHuman.Email.Verified = user.Email.IsEmailVerified +func createInstancePbToAddHuman(req *system_pb.CreateInstanceRequest_Human, defaultHuman command.AddHuman, userLoginMustBeDomain bool, org, externalDomain string) *command.AddHuman { + user := defaultHuman + if req.Email != nil { + user.Email.Address = req.Email.Email + user.Email.Verified = req.Email.IsEmailVerified } - if user.Profile != nil { - if user.Profile.FirstName != "" { - defaultHuman.FirstName = user.Profile.FirstName + if req.Profile != nil { + if req.Profile.FirstName != "" { + user.FirstName = req.Profile.FirstName } - if user.Profile.LastName != "" { - defaultHuman.LastName = user.Profile.LastName + if req.Profile.LastName != "" { + user.LastName = req.Profile.LastName } - if user.Profile.PreferredLanguage != "" { - lang, err := language.Parse(user.Profile.PreferredLanguage) + if req.Profile.PreferredLanguage != "" { + lang, err := language.Parse(req.Profile.PreferredLanguage) if err == nil { - defaultHuman.PreferredLanguage = lang + user.PreferredLanguage = lang } } } // check if default username is email style or else append @. // this way we have the same value as before changing `UserLoginMustBeDomain` to false - if !userLoginMustBeDomain && !strings.Contains(defaultHuman.Username, "@") { - defaultHuman.Username = defaultHuman.Username + "@" + domain.NewIAMDomainName(org, externalDomain) + if !userLoginMustBeDomain && !strings.Contains(user.Username, "@") { + user.Username = user.Username + "@" + domain.NewIAMDomainName(org, externalDomain) } - if user.UserName != "" { - defaultHuman.Username = user.UserName + if req.UserName != "" { + user.Username = req.UserName } - if user.Password != nil { - defaultHuman.Password = user.Password.Password - defaultHuman.PasswordChangeRequired = user.Password.PasswordChangeRequired + if req.Password != nil { + user.Password = req.Password.Password + user.PasswordChangeRequired = req.Password.PasswordChangeRequired } - return &defaultHuman + return &user } -func createInstancePbToAddMachine(user *system_pb.CreateInstanceRequest_Machine, defaultMachine command.AddMachine) *command.AddMachine { - machine := command.Machine{} +func createInstancePbToAddMachine(req *system_pb.CreateInstanceRequest_Machine, defaultMachine command.AddMachine) (machine *command.AddMachine) { + machine = new(command.AddMachine) if defaultMachine.Machine != nil { - machine = *defaultMachine.Machine - } - if user.UserName != "" { - machine.Username = user.UserName - } - if user.Name != "" { - machine.Name = user.Name - } - defaultMachine.Machine = &machine - - if defaultMachine.Pat != nil || user.PersonalAccessToken != nil { - pat := command.AddPat{} - if defaultMachine.Pat != nil { - pat = *defaultMachine.Pat - } - // scopes are currently static and can not be overwritten - pat.Scopes = []string{oidc.ScopeOpenID, z_oidc.ScopeUserMetaData, z_oidc.ScopeResourceOwner} - if user.PersonalAccessToken.ExpirationDate != nil { - pat.ExpirationDate = user.PersonalAccessToken.ExpirationDate.AsTime() - } - defaultMachine.Pat = &pat + machineCopy := *defaultMachine.Machine + machine.Machine = &machineCopy + } else { + machine.Machine = new(command.Machine) } - if defaultMachine.MachineKey != nil || user.MachineKey != nil { + if req.UserName != "" { + machine.Machine.Username = req.UserName + } + if req.Name != "" { + machine.Machine.Name = req.Name + } + + if defaultMachine.Pat != nil || req.PersonalAccessToken != nil { + pat := command.AddPat{ + // Scopes are currently static and can not be overwritten + Scopes: []string{oidc.ScopeOpenID, z_oidc.ScopeUserMetaData, z_oidc.ScopeResourceOwner}, + } + + if !defaultMachine.Pat.ExpirationDate.IsZero() { + pat.ExpirationDate = defaultMachine.Pat.ExpirationDate + } else if req.PersonalAccessToken.ExpirationDate.IsValid() { + pat.ExpirationDate = req.PersonalAccessToken.ExpirationDate.AsTime() + } + + machine.Pat = &pat + } + + if defaultMachine.MachineKey != nil || req.MachineKey != nil { machineKey := command.AddMachineKey{} if defaultMachine.MachineKey != nil { machineKey = *defaultMachine.MachineKey } - if user.MachineKey != nil { - if user.MachineKey.Type != 0 { - machineKey.Type = authn.KeyTypeToDomain(user.MachineKey.Type) + if req.MachineKey != nil { + if req.MachineKey.Type != 0 { + machineKey.Type = authn.KeyTypeToDomain(req.MachineKey.Type) } - if user.MachineKey.ExpirationDate != nil { - machineKey.ExpirationDate = user.MachineKey.ExpirationDate.AsTime() + if req.MachineKey.ExpirationDate.IsValid() { + machineKey.ExpirationDate = req.MachineKey.ExpirationDate.AsTime() } } - defaultMachine.MachineKey = &machineKey + machine.MachineKey = &machineKey } - return &defaultMachine + + return machine } func AddInstancePbToSetupInstance(req *system_pb.AddInstanceRequest, defaultInstance command.InstanceSetup, externalDomain string) *command.InstanceSetup { + instance := defaultInstance + if req.InstanceName != "" { - defaultInstance.InstanceName = req.InstanceName - defaultInstance.Org.Name = req.InstanceName + instance.InstanceName = req.InstanceName + instance.Org.Name = req.InstanceName } if req.CustomDomain != "" { - defaultInstance.CustomDomain = req.CustomDomain + instance.CustomDomain = req.CustomDomain } if req.FirstOrgName != "" { - defaultInstance.Org.Name = req.FirstOrgName + instance.Org.Name = req.FirstOrgName + } + + if defaultInstance.Org.Human != nil { + // used to not overwrite the default human later + humanCopy := *defaultInstance.Org.Human + instance.Org.Human = &humanCopy + } else { + instance.Org.Human = new(command.AddHuman) } if req.OwnerEmail.Email != "" { - defaultInstance.Org.Human.Email.Address = req.OwnerEmail.Email - defaultInstance.Org.Human.Email.Verified = req.OwnerEmail.IsEmailVerified + instance.Org.Human.Email.Address = req.OwnerEmail.Email + instance.Org.Human.Email.Verified = req.OwnerEmail.IsEmailVerified } if req.OwnerProfile != nil { if req.OwnerProfile.FirstName != "" { - defaultInstance.Org.Human.FirstName = req.OwnerProfile.FirstName + instance.Org.Human.FirstName = req.OwnerProfile.FirstName } if req.OwnerProfile.LastName != "" { - defaultInstance.Org.Human.LastName = req.OwnerProfile.LastName + instance.Org.Human.LastName = req.OwnerProfile.LastName } if req.OwnerProfile.PreferredLanguage != "" { lang, err := language.Parse(req.OwnerProfile.PreferredLanguage) if err == nil { - defaultInstance.Org.Human.PreferredLanguage = lang + instance.Org.Human.PreferredLanguage = lang } } } + if req.OwnerUserName != "" { + instance.Org.Human.Username = req.OwnerUserName + } // check if default username is email style or else append @. // this way we have the same value as before changing `UserLoginMustBeDomain` to false - if !defaultInstance.DomainPolicy.UserLoginMustBeDomain && !strings.Contains(defaultInstance.Org.Human.Username, "@") { - defaultInstance.Org.Human.Username = defaultInstance.Org.Human.Username + "@" + domain.NewIAMDomainName(defaultInstance.Org.Name, externalDomain) - } - if req.OwnerUserName != "" { - defaultInstance.Org.Human.Username = req.OwnerUserName + if !instance.DomainPolicy.UserLoginMustBeDomain && !strings.Contains(instance.Org.Human.Username, "@") { + instance.Org.Human.Username = instance.Org.Human.Username + "@" + domain.NewIAMDomainName(instance.Org.Name, externalDomain) } if req.OwnerPassword != nil { - defaultInstance.Org.Human.Password = req.OwnerPassword.Password - defaultInstance.Org.Human.PasswordChangeRequired = req.OwnerPassword.PasswordChangeRequired + instance.Org.Human.Password = req.OwnerPassword.Password + instance.Org.Human.PasswordChangeRequired = req.OwnerPassword.PasswordChangeRequired } if lang := language.Make(req.DefaultLanguage); lang != language.Und { - defaultInstance.DefaultLanguage = lang + instance.DefaultLanguage = lang } - return &defaultInstance + return &instance } + func ListInstancesRequestToModel(req *system_pb.ListInstancesRequest) (*query.InstanceSearchQueries, error) { offset, limit, asc := object.ListQueryToModel(req.Query) queries, err := instance_grpc.InstanceQueriesToModel(req.Queries)