From 2d4cd331da6e4123b6e25cc79adec400f316810b Mon Sep 17 00:00:00 2001 From: Miguel Cabrerizo <30386061+doncicuto@users.noreply.github.com> Date: Wed, 11 Oct 2023 09:55:01 +0200 Subject: [PATCH] fix: allow unicode characters in org domains (#6675) Co-authored-by: Elio Bischof --- cmd/setup/03.go | 12 ++++++++++-- internal/api/grpc/admin/org.go | 6 +++++- internal/api/grpc/management/org.go | 6 +++++- internal/api/grpc/system/instance_converter.go | 6 ++++-- internal/api/ui/login/login.go | 6 +++++- internal/command/org.go | 5 ++++- internal/command/org_domain.go | 9 ++++++--- internal/domain/org.go | 3 ++- internal/domain/org_domain.go | 17 +++++++++++++---- internal/domain/org_domain_test.go | 14 +++++++++++--- internal/org/model/org.go | 3 ++- internal/static/i18n/bg.yaml | 1 + internal/static/i18n/de.yaml | 1 + internal/static/i18n/en.yaml | 1 + internal/static/i18n/es.yaml | 1 + internal/static/i18n/fr.yaml | 1 + internal/static/i18n/it.yaml | 2 ++ internal/static/i18n/ja.yaml | 1 + internal/static/i18n/mk.yaml | 1 + internal/static/i18n/pl.yaml | 1 + internal/static/i18n/pt.yaml | 1 + internal/static/i18n/zh.yaml | 1 + 22 files changed, 79 insertions(+), 20 deletions(-) diff --git a/cmd/setup/03.go b/cmd/setup/03.go index 4de513be40..4a93888f6b 100644 --- a/cmd/setup/03.go +++ b/cmd/setup/03.go @@ -105,13 +105,21 @@ func (mig *FirstInstance) Execute(ctx context.Context) error { // check if username is email style or else append @. //this way we have the same value as before changing `UserLoginMustBeDomain` to false if !mig.instanceSetup.DomainPolicy.UserLoginMustBeDomain && !strings.Contains(mig.instanceSetup.Org.Human.Username, "@") { - mig.instanceSetup.Org.Human.Username = mig.instanceSetup.Org.Human.Username + "@" + domain.NewIAMDomainName(mig.instanceSetup.Org.Name, mig.instanceSetup.CustomDomain) + orgDomain, err := domain.NewIAMDomainName(mig.instanceSetup.Org.Name, mig.instanceSetup.CustomDomain) + if err != nil { + return err + } + mig.instanceSetup.Org.Human.Username = mig.instanceSetup.Org.Human.Username + "@" + orgDomain } mig.instanceSetup.Org.Human.Email.Address = mig.instanceSetup.Org.Human.Email.Address.Normalize() if mig.instanceSetup.Org.Human.Email.Address == "" { mig.instanceSetup.Org.Human.Email.Address = domain.EmailAddress(mig.instanceSetup.Org.Human.Username) if !strings.Contains(string(mig.instanceSetup.Org.Human.Email.Address), "@") { - mig.instanceSetup.Org.Human.Email.Address = domain.EmailAddress(mig.instanceSetup.Org.Human.Username + "@" + domain.NewIAMDomainName(mig.instanceSetup.Org.Name, mig.instanceSetup.CustomDomain)) + orgDomain, err := domain.NewIAMDomainName(mig.instanceSetup.Org.Name, mig.instanceSetup.CustomDomain) + if err != nil { + return err + } + mig.instanceSetup.Org.Human.Email.Address = domain.EmailAddress(mig.instanceSetup.Org.Human.Username + "@" + orgDomain) } } diff --git a/internal/api/grpc/admin/org.go b/internal/api/grpc/admin/org.go index 9edd6f123c..80cb76571d 100644 --- a/internal/api/grpc/admin/org.go +++ b/internal/api/grpc/admin/org.go @@ -66,7 +66,11 @@ func (s *Server) ListOrgs(ctx context.Context, req *admin_pb.ListOrgsRequest) (* } func (s *Server) SetUpOrg(ctx context.Context, req *admin_pb.SetUpOrgRequest) (*admin_pb.SetUpOrgResponse, error) { - userIDs, err := s.getClaimedUserIDsOfOrgDomain(ctx, domain.NewIAMDomainName(req.Org.Name, authz.GetInstance(ctx).RequestedDomain())) + orgDomain, err := domain.NewIAMDomainName(req.Org.Name, authz.GetInstance(ctx).RequestedDomain()) + if err != nil { + return nil, err + } + userIDs, err := s.getClaimedUserIDsOfOrgDomain(ctx, orgDomain) if err != nil { return nil, err } diff --git a/internal/api/grpc/management/org.go b/internal/api/grpc/management/org.go index 1627530e6c..df8e76c0e2 100644 --- a/internal/api/grpc/management/org.go +++ b/internal/api/grpc/management/org.go @@ -72,7 +72,11 @@ func (s *Server) ListOrgChanges(ctx context.Context, req *mgmt_pb.ListOrgChanges } func (s *Server) AddOrg(ctx context.Context, req *mgmt_pb.AddOrgRequest) (*mgmt_pb.AddOrgResponse, error) { - userIDs, err := s.getClaimedUserIDsOfOrgDomain(ctx, domain.NewIAMDomainName(req.Name, authz.GetInstance(ctx).RequestedDomain()), "") + orgDomain, err := domain.NewIAMDomainName(req.Name, authz.GetInstance(ctx).RequestedDomain()) + if err != nil { + return nil, err + } + userIDs, err := s.getClaimedUserIDsOfOrgDomain(ctx, orgDomain, "") if err != nil { return nil, err } diff --git a/internal/api/grpc/system/instance_converter.go b/internal/api/grpc/system/instance_converter.go index 271dc1520d..4eea0a18f7 100644 --- a/internal/api/grpc/system/instance_converter.go +++ b/internal/api/grpc/system/instance_converter.go @@ -79,7 +79,8 @@ func createInstancePbToAddHuman(req *system_pb.CreateInstanceRequest_Human, defa // 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(user.Username, "@") { - user.Username = user.Username + "@" + domain.NewIAMDomainName(org, externalDomain) + orgDomain, _ := domain.NewIAMDomainName(org, externalDomain) + user.Username = user.Username + "@" + orgDomain } if req.UserName != "" { user.Username = req.UserName @@ -185,7 +186,8 @@ func AddInstancePbToSetupInstance(req *system_pb.AddInstanceRequest, defaultInst // check if default username is email style or else append @. // this way we have the same value as before changing `UserLoginMustBeDomain` to false if !instance.DomainPolicy.UserLoginMustBeDomain && !strings.Contains(instance.Org.Human.Username, "@") { - instance.Org.Human.Username = instance.Org.Human.Username + "@" + domain.NewIAMDomainName(instance.Org.Name, externalDomain) + orgDomain, _ := domain.NewIAMDomainName(instance.Org.Name, externalDomain) + instance.Org.Human.Username = instance.Org.Human.Username + "@" + orgDomain } if req.OwnerPassword != nil { instance.Org.Human.Password = req.OwnerPassword.Password diff --git a/internal/api/ui/login/login.go b/internal/api/ui/login/login.go index 06a7787b59..fcda0252d6 100644 --- a/internal/api/ui/login/login.go +++ b/internal/api/ui/login/login.go @@ -161,7 +161,11 @@ func (l *Login) Handler() http.Handler { } func (l *Login) getClaimedUserIDsOfOrgDomain(ctx context.Context, orgName string) ([]string, error) { - loginName, err := query.NewUserPreferredLoginNameSearchQuery("@"+domain.NewIAMDomainName(orgName, authz.GetInstance(ctx).RequestedDomain()), query.TextEndsWithIgnoreCase) + orgDomain, err := domain.NewIAMDomainName(orgName, authz.GetInstance(ctx).RequestedDomain()) + if err != nil { + return nil, err + } + loginName, err := query.NewUserPreferredLoginNameSearchQuery("@"+orgDomain, query.TextEndsWithIgnoreCase) if err != nil { return nil, err } diff --git a/internal/command/org.go b/internal/command/org.go index 1ba999e97c..72a97b6f01 100644 --- a/internal/command/org.go +++ b/internal/command/org.go @@ -238,7 +238,10 @@ func AddOrgCommand(ctx context.Context, a *org.Aggregate, name string, userIDs . if name = strings.TrimSpace(name); name == "" { return nil, errors.ThrowInvalidArgument(nil, "ORG-mruNY", "Errors.Invalid.Argument") } - defaultDomain := domain.NewIAMDomainName(name, authz.GetInstance(ctx).RequestedDomain()) + defaultDomain, err := domain.NewIAMDomainName(name, authz.GetInstance(ctx).RequestedDomain()) + if err != nil { + return nil, err + } return func(ctx context.Context, filter preparation.FilterToQueryReducer) ([]eventstore.Command, error) { return []eventstore.Command{ org.NewOrgAddedEvent(ctx, &a.Aggregate, name), diff --git a/internal/command/org_domain.go b/internal/command/org_domain.go index aae1b14b68..6ae30baeb2 100644 --- a/internal/command/org_domain.go +++ b/internal/command/org_domain.go @@ -309,13 +309,16 @@ func (c *Commands) changeDefaultDomain(ctx context.Context, orgID, newName strin return nil, err } iamDomain := authz.GetInstance(ctx).RequestedDomain() - defaultDomain := domain.NewIAMDomainName(orgDomains.OrgName, iamDomain) + defaultDomain, _ := domain.NewIAMDomainName(orgDomains.OrgName, iamDomain) isPrimary := defaultDomain == orgDomains.PrimaryDomain orgAgg := OrgAggregateFromWriteModel(&orgDomains.WriteModel) for _, orgDomain := range orgDomains.Domains { if orgDomain.State == domain.OrgDomainStateActive { if orgDomain.Domain == defaultDomain { - newDefaultDomain := domain.NewIAMDomainName(newName, iamDomain) + newDefaultDomain, err := domain.NewIAMDomainName(newName, iamDomain) + if err != nil { + return nil, err + } events := []eventstore.Command{ org.NewDomainAddedEvent(ctx, orgAgg, newDefaultDomain), org.NewDomainVerifiedEvent(ctx, orgAgg, newDefaultDomain), @@ -338,7 +341,7 @@ func (c *Commands) removeCustomDomains(ctx context.Context, orgID string) ([]eve return nil, err } hasDefault := false - defaultDomain := domain.NewIAMDomainName(orgDomains.OrgName, authz.GetInstance(ctx).RequestedDomain()) + defaultDomain, _ := domain.NewIAMDomainName(orgDomains.OrgName, authz.GetInstance(ctx).RequestedDomain()) isPrimary := defaultDomain == orgDomains.PrimaryDomain orgAgg := OrgAggregateFromWriteModel(&orgDomains.WriteModel) events := make([]eventstore.Command, 0, len(orgDomains.Domains)) diff --git a/internal/domain/org.go b/internal/domain/org.go index c5375e83cd..0e87d20a43 100644 --- a/internal/domain/org.go +++ b/internal/domain/org.go @@ -25,7 +25,8 @@ func (o *Org) IsValid() bool { } func (o *Org) AddIAMDomain(iamDomain string) { - o.Domains = append(o.Domains, &OrgDomain{Domain: NewIAMDomainName(o.Name, iamDomain), Verified: true, Primary: true}) + orgDomain, _ := NewIAMDomainName(o.Name, iamDomain) + o.Domains = append(o.Domains, &OrgDomain{Domain: orgDomain, Verified: true, Primary: true}) } type OrgState int32 diff --git a/internal/domain/org_domain.go b/internal/domain/org_domain.go index ad39e36795..bbd2e664e5 100644 --- a/internal/domain/org_domain.go +++ b/internal/domain/org_domain.go @@ -6,6 +6,7 @@ import ( http_util "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/eventstore/v1/models" ) @@ -32,15 +33,18 @@ func (domain *OrgDomain) GenerateVerificationCode(codeGenerator crypto.Generator return validationCode, nil } -func NewIAMDomainName(orgName, iamDomain string) string { +func NewIAMDomainName(orgName, iamDomain string) (string, error) { // Reference: label domain requirements https://www.nic.ad.jp/timeline/en/20th/appendix1.html // Replaces spaces in org name with hyphens label := strings.ReplaceAll(orgName, " ", "-") // The label must only contains alphanumeric characters and hyphens - // Invalid characters are replaced with and empty space - label = string(regexp.MustCompile(`[^a-zA-Z0-9-]`).ReplaceAll([]byte(label), []byte(""))) + // Invalid characters are replaced with and empty space but as #6471, + // as these domains are not used to host ZITADEL, but only for user names, + // the characters shouldn't matter that much so we'll accept unicode + // characters, accented characters (\p{L}\p{M}), numbers and hyphens. + label = string(regexp.MustCompile(`[^\p{L}\p{M}0-9-]`).ReplaceAll([]byte(label), []byte(""))) // The label cannot exceed 63 characters if len(label) > 63 { @@ -64,7 +68,12 @@ func NewIAMDomainName(orgName, iamDomain string) string { label = label[:len(label)-1] } - return strings.ToLower(label + "." + iamDomain) + // Empty string should be invalid + if len(label) > 0 { + return strings.ToLower(label + "." + iamDomain), nil + } + + return "", errors.ThrowInvalidArgument(nil, "ORG-RrfXY", "Errors.Org.Domain.EmptyString") } type OrgDomainValidationType int32 diff --git a/internal/domain/org_domain_test.go b/internal/domain/org_domain_test.go index 8dd77ca328..e19138e10a 100644 --- a/internal/domain/org_domain_test.go +++ b/internal/domain/org_domain_test.go @@ -41,10 +41,10 @@ func TestNewIAMDomainName(t *testing.T) { { name: "replace invalid characters [^a-zA-Z0-9-] with empty spaces", args: args{ - orgName: "mí Örg name?", + orgName: "mí >**name?", iamDomain: "localhost", }, - result: "m-rg-name.localhost", + result: "mí-name.localhost", }, { name: "label created from org name size is not greater than 63 chars", @@ -78,10 +78,18 @@ func TestNewIAMDomainName(t *testing.T) { }, result: "my-super-long-organization-name-with-many-many-many-characters.localhost", }, + { + name: "string full with invalid characters returns empty", + args: args{ + orgName: "*¿=@^[])", + iamDomain: "localhost", + }, + result: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - domain := NewIAMDomainName(tt.args.orgName, tt.args.iamDomain) + domain, _ := NewIAMDomainName(tt.args.orgName, tt.args.iamDomain) if tt.result != domain { t.Errorf("got wrong result: expected: %v, actual: %v ", tt.result, domain) } diff --git a/internal/org/model/org.go b/internal/org/model/org.go index 4267506b58..50ceda8c85 100644 --- a/internal/org/model/org.go +++ b/internal/org/model/org.go @@ -46,5 +46,6 @@ func (o *Org) GetPrimaryDomain() *OrgDomain { } func (o *Org) AddIAMDomain(iamDomain string) { - o.Domains = append(o.Domains, &OrgDomain{Domain: domain.NewIAMDomainName(o.Name, iamDomain), Verified: true, Primary: true}) + orgDomain, _ := domain.NewIAMDomainName(o.Name, iamDomain) + o.Domains = append(o.Domains, &OrgDomain{Domain: orgDomain, Verified: true, Primary: true}) } diff --git a/internal/static/i18n/bg.yaml b/internal/static/i18n/bg.yaml index ea8f06409f..b516fcadb6 100644 --- a/internal/static/i18n/bg.yaml +++ b/internal/static/i18n/bg.yaml @@ -204,6 +204,7 @@ Errors: Domain: AlreadyExists: Домейнът вече съществува InvalidCharacter: "Само буквено-цифрови знаци, . " + EmptyString: Невалидни нецифрови и азбучни знаци бяха заменени с празни интервали и полученият домейн е празен низ IDP: InvalidSearchQuery: Невалидна заявка за търсене ClientIDMissing: Липсва ClientID diff --git a/internal/static/i18n/de.yaml b/internal/static/i18n/de.yaml index 6bf7115477..7610d69401 100644 --- a/internal/static/i18n/de.yaml +++ b/internal/static/i18n/de.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: Domäne existiert bereits InvalidCharacter: Nur alphanumerische Zeichen, . und - sind für eine Domäne erlaubt + EmptyString: Ungültige nicht numerische und alphabetische Zeichen wurden durch Leerzeichen ersetzt und die resultierende Domäne ist eine leere Zeichenfolge IDP: InvalidSearchQuery: Ungültiger Suchparameter ClientIDMissing: ClientID fehlt diff --git a/internal/static/i18n/en.yaml b/internal/static/i18n/en.yaml index 03a7e16b7c..0c694f463e 100644 --- a/internal/static/i18n/en.yaml +++ b/internal/static/i18n/en.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: Domain already exists InvalidCharacter: Only alphanumeric characters, . and - are allowed for a domain + EmptyString: Invalid non numeric and alphabetical characters were replaced with empty spaces and resulting domain is an empty string IDP: InvalidSearchQuery: Invalid search query ClientIDMissing: ClientID missing diff --git a/internal/static/i18n/es.yaml b/internal/static/i18n/es.yaml index a6783a1706..9776a61f23 100644 --- a/internal/static/i18n/es.yaml +++ b/internal/static/i18n/es.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: El dominio ya existe InvalidCharacter: Solo caracteres alfanuméricos, . y - se permiten para un dominio + EmptyString: Los caracteres alfabéticos y no numéricos no válidos se reemplazaron con espacios vacíos y el dominio resultante es una cadena vacía IDP: InvalidSearchQuery: Consulta de búsqueda no válida ClientIDMissing: Falta ClientID diff --git a/internal/static/i18n/fr.yaml b/internal/static/i18n/fr.yaml index 2e9592fe31..b72a5ba1f3 100644 --- a/internal/static/i18n/fr.yaml +++ b/internal/static/i18n/fr.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: Le domaine existe déjà InvalidCharacter: Seuls les caractères alphanumériques, . et - sont autorisés pour un domaine + EmptyString: Les caractères non numériques et alphabétiques non valides ont été remplacés par des espaces vides et le domaine résultant est une chaîne vide IDP: InvalidSearchQuery: Paramètre de recherche non valide ClientIDMissing: ID client manquant diff --git a/internal/static/i18n/it.yaml b/internal/static/i18n/it.yaml index 9a3e48af74..43891039ea 100644 --- a/internal/static/i18n/it.yaml +++ b/internal/static/i18n/it.yaml @@ -201,6 +201,8 @@ Errors: IdpIsNotOIDC: La configurazione IDP non è di tipo oidc Domain: AlreadyExists: Il dominio già esistente + InvalidCharacter: Solo caratteri alfanumerici, . e - sono consentiti per un dominio + EmptyString: I caratteri non numerici e alfabetici non validi sono stati sostituiti con spazi vuoti e il dominio risultante è una stringa vuota IDP: InvalidSearchQuery: Parametro di ricerca non valido InvalidCharacter: Per un dominio sono ammessi solo caratteri alfanumerici, . e - diff --git a/internal/static/i18n/ja.yaml b/internal/static/i18n/ja.yaml index 3e2960dc27..72a7aa9d32 100644 --- a/internal/static/i18n/ja.yaml +++ b/internal/static/i18n/ja.yaml @@ -194,6 +194,7 @@ Errors: Domain: AlreadyExists: ドメインはすでに存在します InvalidCharacter: ドメインは英数字、'.'、'-'のみ使用可能です。 + EmptyString: 無効な数字およびアルファベット以外の文字は空のスペースに置き換えられ、結果のドメインは空の文字列になります IDP: InvalidSearchQuery: 無効な検索クエリです ClientIDMissing: クライアントIDがありません diff --git a/internal/static/i18n/mk.yaml b/internal/static/i18n/mk.yaml index f613eebb2f..ae27d818b2 100644 --- a/internal/static/i18n/mk.yaml +++ b/internal/static/i18n/mk.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: Доменот веќе постои InvalidCharacter: Дозволени се само алфанумерички знаци, . и - се дозволени за домен + EmptyString: Неважечките ненумерички и азбучни знаци се заменети со празни места и добиениот домен е празна низа IDP: InvalidSearchQuery: Невалидно пребарување ClientID Missing: ClientID недостасува diff --git a/internal/static/i18n/pl.yaml b/internal/static/i18n/pl.yaml index 2e0c3a8244..9e7a55ef0f 100644 --- a/internal/static/i18n/pl.yaml +++ b/internal/static/i18n/pl.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: Domena już istnieje InvalidCharacter: Tylko znaki alfanumeryczne, . i - są dozwolone dla domeny + EmptyString: Nieprawidłowe znaki inne niż numeryczne i alfabetyczne zostały zastąpione pustymi spacjami, a wynikowa domena jest pustym ciągiem znaków IDP: InvalidSearchQuery: Nieprawidłowe zapytanie wyszukiwania ClientIDMissing: Brak ClientID diff --git a/internal/static/i18n/pt.yaml b/internal/static/i18n/pt.yaml index 7017e12ccf..71448e1a89 100644 --- a/internal/static/i18n/pt.yaml +++ b/internal/static/i18n/pt.yaml @@ -200,6 +200,7 @@ Errors: Domain: AlreadyExists: Domínio já existe InvalidCharacter: Apenas caracteres alfanuméricos, . e - são permitidos para um domínio + EmptyString: Caracteres não numéricos e alfabéticos inválidos foram substituídos por espaços vazios e o domínio resultante é uma string vazia IDP: InvalidSearchQuery: Consulta de pesquisa inválida ClientIDMissing: ClientID ausente diff --git a/internal/static/i18n/zh.yaml b/internal/static/i18n/zh.yaml index 27d719a828..526e67f055 100644 --- a/internal/static/i18n/zh.yaml +++ b/internal/static/i18n/zh.yaml @@ -202,6 +202,7 @@ Errors: Domain: AlreadyExists: 域名已存在 InvalidCharacter: 只有字母数字字符，.和 - 允许用于域名中 + EmptyString: 无效的非数字和字母字符被替换为空格，结果域是空字符串 IDP: InvalidSearchQuery: 无效的搜索查询 ClientIDMissing: 客户端 ID 丢失