fix: trim spaces for usernames and organization names (#4217)

This commit is contained in:
Livio Spring 2022-08-19 15:00:14 +02:00 committed by GitHub
parent d656b3f3c9
commit cc612fed07
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 334 additions and 7 deletions

View File

@ -2,6 +2,7 @@ package eventstore
import (
"context"
"strings"
"time"
"github.com/zitadel/logging"
@ -628,6 +629,7 @@ func (repo *AuthRequestRepo) tryUsingOnlyUserSession(request *domain.AuthRequest
func (repo *AuthRequestRepo) checkLoginName(ctx context.Context, request *domain.AuthRequest, loginName string) (err error) {
var user *user_view_model.UserView
loginName = strings.TrimSpace(loginName)
preferredLoginName := loginName
if request.RequestedOrgID != "" {
if request.RequestedOrgID != "" {
@ -662,7 +664,7 @@ func (repo *AuthRequestRepo) checkLoginName(ctx context.Context, request *domain
return nil
}
func (repo AuthRequestRepo) checkLoginPolicyWithResourceOwner(ctx context.Context, request *domain.AuthRequest, user *user_view_model.UserView) error {
func (repo *AuthRequestRepo) checkLoginPolicyWithResourceOwner(ctx context.Context, request *domain.AuthRequest, user *user_view_model.UserView) error {
loginPolicy, idpProviders, err := repo.getLoginPolicyAndIDPProviders(ctx, user.ResourceOwner)
if err != nil {
return err

View File

@ -135,7 +135,7 @@ func (c *Commands) AddOrgWithID(ctx context.Context, name, userID, resourceOwner
}
func (c *Commands) AddOrg(ctx context.Context, name, userID, resourceOwner string, claimedUserIDs []string) (*domain.Org, error) {
if name == "" {
if name = strings.TrimSpace(name); name == "" {
return nil, caos_errs.ThrowInvalidArgument(nil, "EVENT-Mf9sd", "Errors.Org.Invalid")
}
@ -174,6 +174,7 @@ func (c *Commands) addOrgWithIDAndMember(ctx context.Context, name, userID, reso
}
func (c *Commands) ChangeOrg(ctx context.Context, orgID, name string) (*domain.ObjectDetails, error) {
name = strings.TrimSpace(name)
if orgID == "" || name == "" {
return nil, caos_errs.ThrowInvalidArgument(nil, "EVENT-Mf9sd", "Errors.Org.Invalid")
}
@ -186,7 +187,7 @@ func (c *Commands) ChangeOrg(ctx context.Context, orgID, name string) (*domain.O
return nil, caos_errs.ThrowNotFound(nil, "ORG-1MRds", "Errors.Org.NotFound")
}
if orgWriteModel.Name == name {
return nil, caos_errs.ThrowNotFound(nil, "ORG-4VSdf", "Errors.Org.NotChanged")
return nil, caos_errs.ThrowPreconditionFailed(nil, "ORG-4VSdf", "Errors.Org.NotChanged")
}
orgAgg := OrgAggregateFromWriteModel(&orgWriteModel.WriteModel)
events := make([]eventstore.Command, 0)

View File

@ -106,6 +106,23 @@ func TestCommandSide_AddOrg(t *testing.T) {
err: errors.IsErrorInvalidArgument,
},
},
{
name: "invalid org (spaces), error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
},
args: args{
ctx: context.Background(),
userID: "user1",
resourceOwner: "org1",
name: " ",
},
res: res{
err: errors.IsErrorInvalidArgument,
},
},
{
name: "user removed, error",
fields: fields{
@ -362,6 +379,82 @@ func TestCommandSide_AddOrg(t *testing.T) {
},
},
},
{
name: "add org (remove spaces), no error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilterOrgDomainNotFound(),
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username1",
"firstname1",
"lastname1",
"nickname1",
"displayname1",
language.German,
domain.GenderMale,
"email1",
true,
),
),
),
expectFilterOrgMemberNotFound(),
expectPush(
[]*repository.Event{
eventFromEventPusher(org.NewOrgAddedEvent(context.Background(),
&org.NewAggregate("org2").Aggregate,
"Org",
)),
eventFromEventPusher(org.NewDomainAddedEvent(context.Background(),
&org.NewAggregate("org2").Aggregate, "org.iam-domain",
)),
eventFromEventPusher(org.NewDomainVerifiedEvent(context.Background(),
&org.NewAggregate("org2").Aggregate,
"org.iam-domain",
)),
eventFromEventPusher(org.NewDomainPrimarySetEvent(context.Background(),
&org.NewAggregate("org2").Aggregate,
"org.iam-domain",
)),
eventFromEventPusher(org.NewMemberAddedEvent(context.Background(),
&org.NewAggregate("org2").Aggregate,
"user1",
domain.RoleOrgOwner,
)),
},
uniqueConstraintsFromEventConstraint(org.NewAddOrgNameUniqueConstraint("Org")),
uniqueConstraintsFromEventConstraint(org.NewAddOrgDomainUniqueConstraint("org.iam-domain")),
uniqueConstraintsFromEventConstraint(member.NewAddMemberUniqueConstraint("org2", "user1")),
),
),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "org2"),
zitadelRoles: []authz.RoleMapping{
{
Role: "ORG_OWNER",
},
},
},
args: args{
ctx: authz.WithRequestedDomain(context.Background(), "iam-domain"),
name: " Org ",
userID: "user1",
resourceOwner: "org1",
},
res: res{
want: &domain.Org{
ObjectRoot: models.ObjectRoot{
AggregateID: "org2",
ResourceOwner: "org2",
},
Name: "Org",
State: domain.OrgStateActive,
PrimaryDomain: "org.iam-domain",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -394,8 +487,7 @@ func TestCommandSide_ChangeOrg(t *testing.T) {
name string
}
type res struct {
want *domain.Org
err func(error) bool
err func(error) bool
}
tests := []struct {
name string
@ -418,6 +510,22 @@ func TestCommandSide_ChangeOrg(t *testing.T) {
err: errors.IsErrorInvalidArgument,
},
},
{
name: "empty name (spaces), invalid argument error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
},
args: args{
ctx: context.Background(),
orgID: "org1",
name: " ",
},
res: res{
err: errors.IsErrorInvalidArgument,
},
},
{
name: "org not found, error",
fields: fields{
@ -435,6 +543,29 @@ func TestCommandSide_ChangeOrg(t *testing.T) {
err: errors.IsNotFound,
},
},
{
name: "no change (spaces), error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
org.NewOrgAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
"org"),
),
),
),
},
args: args{
ctx: authz.WithRequestedDomain(context.Background(), "zitadel.ch"),
orgID: "org1",
name: " org ",
},
res: res{
err: errors.IsPreconditionFailed,
},
},
{
name: "push failed, error",
fields: fields{

View File

@ -3,6 +3,7 @@ package command
import (
"context"
"fmt"
"strings"
"time"
"github.com/zitadel/logging"
@ -19,6 +20,7 @@ import (
)
func (c *Commands) ChangeUsername(ctx context.Context, orgID, userID, userName string) (*domain.ObjectDetails, error) {
userName = strings.TrimSpace(userName)
if orgID == "" || userID == "" || userName == "" {
return nil, caos_errs.ThrowInvalidArgument(nil, "COMMAND-2N9fs", "Errors.IDMissing")
}

View File

@ -416,7 +416,10 @@ func (c *Commands) importHuman(ctx context.Context, orgID string, human *domain.
}
func (c *Commands) registerHuman(ctx context.Context, orgID string, human *domain.Human, link *domain.UserIDPLink, domainPolicy *domain.DomainPolicy, pwPolicy *domain.PasswordComplexityPolicy, initCodeGenerator crypto.Generator, phoneCodeGenerator crypto.Generator) ([]eventstore.Command, *HumanWriteModel, error) {
if human != nil && human.Username == "" {
if human == nil {
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-JKefw", "Errors.User.Invalid")
}
if human.Username = strings.TrimSpace(human.Username); human.Username == "" {
human.Username = human.EmailAddress
}
if orgID == "" || !human.IsValid() || link == nil && (human.Password == nil || human.Password.SecretString == "") {
@ -432,6 +435,8 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain.
if err := human.CheckDomainPolicy(domainPolicy); err != nil {
return nil, nil, err
}
human.Username = strings.TrimSpace(human.Username)
human.EmailAddress = strings.TrimSpace(human.EmailAddress)
if !domainPolicy.UserLoginMustBeDomain {
usernameSplit := strings.Split(human.Username, "@")
if len(usernameSplit) != 2 {

View File

@ -410,6 +410,76 @@ func TestCommandSide_AddHuman(t *testing.T) {
},
},
},
{
name: "add human email verified, trim spaces, ok",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
true,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewPasswordComplexityPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
1,
false,
false,
false,
false,
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
newAddHumanEvent("password", true, ""),
),
eventFromEventPusher(
user.NewHumanEmailVerifiedEvent(context.Background(),
&userAgg.Aggregate),
),
},
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("username", "org1", true)),
),
),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
userPasswordAlg: crypto.CreateMockHashAlg(gomock.NewController(t)),
codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
ctx: context.Background(),
orgID: "org1",
human: &AddHuman{
Username: " username ",
Password: "password",
FirstName: "firstname",
LastName: "lastname",
Email: Email{
Address: "email@test.ch",
Verified: true,
},
PreferredLanguage: language.English,
PasswordChangeRequired: true,
},
secretGenerator: GetMockSecretGenerator(t),
},
res: res{
want: &domain.HumanDetails{
ID: "user1",
ObjectDetails: domain.ObjectDetails{
ResourceOwner: "org1",
},
},
},
},
{
name: "add human (with phone), ok",
fields: fields{

View File

@ -96,6 +96,23 @@ func TestCommandSide_UsernameChange(t *testing.T) {
err: caos_errs.IsErrorInvalidArgument,
},
},
{
name: "username only spaces, invalid argument error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
},
args: args{
ctx: context.Background(),
orgID: "org1",
userID: "user1",
username: " ",
},
res: res{
err: caos_errs.IsErrorInvalidArgument,
},
},
{
name: "user removed, not found error",
fields: fields{
@ -147,6 +164,39 @@ func TestCommandSide_UsernameChange(t *testing.T) {
err: caos_errs.IsPreconditionFailed,
},
},
{
name: "username not changed (spaces), precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
),
},
args: args{
ctx: context.Background(),
orgID: "org1",
userID: "user1",
username: "username ",
},
res: res{
err: caos_errs.IsPreconditionFailed,
},
},
{
name: "org iam policy not found, precondition error",
fields: fields{
@ -284,6 +334,66 @@ func TestCommandSide_UsernameChange(t *testing.T) {
},
},
},
{
name: "change username (remove spaces), ok",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"firstname",
"lastname",
"nickname",
"displayname",
language.German,
domain.GenderUnspecified,
"email@test.ch",
true,
),
),
),
expectFilter(),
expectFilter(
eventFromEventPusher(
instance.NewDomainPolicyAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
true,
true,
true,
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
user.NewUsernameChangedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"username1",
true,
),
),
},
uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("username", "org1", true)),
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("username1", "org1", true)),
),
),
},
args: args{
ctx: context.Background(),
orgID: "org1",
userID: "user1",
username: "username1 ",
},
res: res{
want: &domain.ObjectDetails{
ResourceOwner: "org1",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

View File

@ -1,6 +1,8 @@
package domain
import (
"strings"
"github.com/zitadel/zitadel/internal/eventstore/v1/models"
)
@ -15,7 +17,11 @@ type Org struct {
}
func (o *Org) IsValid() bool {
return o != nil && o.Name != ""
if o == nil {
return false
}
o.Name = strings.TrimSpace(o.Name)
return o.Name != ""
}
func (o *Org) AddIAMDomain(iamDomain string) {