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 <silvan.reusser@gmail.com>

.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

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 @<orgname>.<custom-domain>
 	// 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 @<orgname>.<custom-domain>
 	// 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)