fix(command): allow email as username (#6565)

Fixes #6460

Made the username checks consistent with create human user.
This commit is contained in:
Tim Möhlmann 2023-09-15 18:29:29 +03:00 committed by GitHub
parent 1a49b7d298
commit 9266f8f00b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 160 additions and 46 deletions

View File

@ -42,10 +42,18 @@ func (c *Commands) ChangeUsername(ctx context.Context, orgID, userID, userName s
if err != nil {
return nil, errors.ThrowPreconditionFailed(err, "COMMAND-38fnu", "Errors.Org.DomainPolicy.NotExisting")
}
if err := CheckDomainPolicyForUserName(userName, domainPolicy); err != nil {
if !domainPolicy.UserLoginMustBeDomain {
index := strings.LastIndex(userName, "@")
if index > 1 {
domainCheck := NewOrgDomainVerifiedWriteModel(userName[index+1:])
if err := c.eventstore.FilterToQueryReducer(ctx, domainCheck); err != nil {
return nil, err
}
if domainCheck.Verified && domainCheck.ResourceOwner != orgID {
return nil, errors.ThrowInvalidArgument(nil, "COMMAND-Di2ei", "Errors.User.DomainNotAllowedAsUsername")
}
}
}
userAgg := UserAggregateFromWriteModel(&existingUser.WriteModel)
pushedEvents, err := c.eventstore.Push(ctx,

View File

@ -1,12 +1,9 @@
package command
import (
"strings"
"github.com/zitadel/zitadel/internal/eventstore"
"github.com/zitadel/zitadel/internal/domain"
caos_errors "github.com/zitadel/zitadel/internal/errors"
"github.com/zitadel/zitadel/internal/repository/user"
)
@ -126,16 +123,6 @@ func UserAggregateFromWriteModel(wm *eventstore.WriteModel) *eventstore.Aggregat
return eventstore.AggregateFromWriteModel(wm, user.AggregateType, user.AggregateVersion)
}
func CheckDomainPolicyForUserName(userName string, policy *domain.DomainPolicy) error {
if policy == nil {
return caos_errors.ThrowPreconditionFailed(nil, "COMMAND-3Mb9s", "Errors.Users.DomainPolicyNil")
}
if policy.UserLoginMustBeDomain && strings.Contains(userName, "@") {
return caos_errors.ThrowPreconditionFailed(nil, "COMMAND-2k9fD", "Errors.User.EmailAsUsernameNotAllowed")
}
return nil
}
func isUserStateExists(state domain.UserState) bool {
return !hasUserState(state, domain.UserStateDeleted, domain.UserStateUnspecified)
}

View File

@ -23,7 +23,7 @@ import (
func TestCommandSide_UsernameChange(t *testing.T) {
type fields struct {
eventstore *eventstore.Eventstore
eventstore func(*testing.T) *eventstore.Eventstore
}
type (
args struct {
@ -46,9 +46,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "userid missing, invalid argument error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
eventstore: expectEventstore(),
},
args: args{
ctx: context.Background(),
@ -63,9 +61,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "orgid missing, invalid argument error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
eventstore: expectEventstore(),
},
args: args{
ctx: context.Background(),
@ -80,9 +76,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "username missing, invalid argument error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
eventstore: expectEventstore(),
},
args: args{
ctx: context.Background(),
@ -97,9 +91,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "username only spaces, invalid argument error",
fields: fields{
eventstore: eventstoreExpect(
t,
),
eventstore: expectEventstore(),
},
args: args{
ctx: context.Background(),
@ -114,8 +106,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "user removed, not found error",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
expectFilter(),
),
},
@ -132,8 +123,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "username not changed, precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
@ -165,8 +155,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "username not changed (spaces), precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
@ -198,8 +187,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "org iam policy not found, precondition error",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
@ -229,10 +217,60 @@ func TestCommandSide_UsernameChange(t *testing.T) {
},
},
{
name: "invalid username, precondition error",
name: "domain verified, wrong org",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
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,
false,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainVerifiedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
"test.ch",
),
),
),
),
},
args: args{
ctx: context.Background(),
orgID: "wrong",
userID: "user1",
username: "test@test.ch",
},
res: res{
err: errors.IsErrorInvalidArgument,
},
},
{
name: "email as username, ok",
fields: fields{
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
@ -260,6 +298,20 @@ func TestCommandSide_UsernameChange(t *testing.T) {
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
user.NewUsernameChangedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"test@test.ch",
true,
),
),
},
uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("username", "org1", true)),
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("test@test.ch", "org1", true)),
),
),
},
args: args{
@ -269,14 +321,82 @@ func TestCommandSide_UsernameChange(t *testing.T) {
username: "test@test.ch",
},
res: res{
err: errors.IsPreconditionFailed,
want: &domain.ObjectDetails{
ResourceOwner: "org1",
},
},
},
{
name: "email as username, verified domain, ok",
fields: fields{
eventstore: expectEventstore(
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,
false,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainVerifiedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
"test.ch",
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
user.NewUsernameChangedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username",
"test@test.ch",
true,
),
),
},
uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("username", "org1", false)),
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("test@test.ch", "org1", false)),
),
),
},
args: args{
ctx: context.Background(),
orgID: "org1",
userID: "user1",
username: "test@test.ch",
},
res: res{
want: &domain.ObjectDetails{
ResourceOwner: "org1",
},
},
},
{
name: "change username, ok",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
@ -335,8 +455,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
{
name: "change username (remove spaces), ok",
fields: fields{
eventstore: eventstoreExpect(
t,
eventstore: expectEventstore(
expectFilter(
eventFromEventPusher(
user.NewHumanAddedEvent(context.Background(),
@ -396,7 +515,7 @@ func TestCommandSide_UsernameChange(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := &Commands{
eventstore: tt.fields.eventstore,
eventstore: tt.fields.eventstore(t),
}
got, err := r.ChangeUsername(tt.args.ctx, tt.args.orgID, tt.args.userID, tt.args.username)
if tt.res.err == nil {