mirror of
https://github.com/zitadel/zitadel.git
synced 2024-12-14 03:54:21 +00:00
fix: Idp bugs (#2259)
* fix: remove external idp unique constraints on user remove * fix: auto register user login mapping * fix: remove external idps on user remove * fix: tests * fix: login policy removed, reset idp provider
This commit is contained in:
parent
74688394d8
commit
f85fd4a1fc
@ -23,6 +23,8 @@ func (wm *IdentityProviderWriteModel) Reduce() error {
|
|||||||
wm.State = domain.IdentityProviderStateActive
|
wm.State = domain.IdentityProviderStateActive
|
||||||
case *policy.IdentityProviderRemovedEvent:
|
case *policy.IdentityProviderRemovedEvent:
|
||||||
wm.State = domain.IdentityProviderStateRemoved
|
wm.State = domain.IdentityProviderStateRemoved
|
||||||
|
case *policy.LoginPolicyRemovedEvent:
|
||||||
|
wm.State = domain.IdentityProviderStateRemoved
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return wm.WriteModel.Reduce()
|
return wm.WriteModel.Reduce()
|
||||||
|
@ -34,6 +34,8 @@ func (wm *OrgIdentityProviderWriteModel) AppendEvents(events ...eventstore.Event
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
wm.IdentityProviderWriteModel.AppendEvents(&e.IdentityProviderRemovedEvent)
|
wm.IdentityProviderWriteModel.AppendEvents(&e.IdentityProviderRemovedEvent)
|
||||||
|
case *org.LoginPolicyRemovedEvent:
|
||||||
|
wm.IdentityProviderWriteModel.AppendEvents(&e.LoginPolicyRemovedEvent)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -50,6 +52,7 @@ func (wm *OrgIdentityProviderWriteModel) Query() *eventstore.SearchQueryBuilder
|
|||||||
AggregateIDs(wm.AggregateID).
|
AggregateIDs(wm.AggregateID).
|
||||||
EventTypes(
|
EventTypes(
|
||||||
org.LoginPolicyIDPProviderAddedEventType,
|
org.LoginPolicyIDPProviderAddedEventType,
|
||||||
org.LoginPolicyIDPProviderRemovedEventType).
|
org.LoginPolicyIDPProviderRemovedEventType,
|
||||||
|
org.LoginPolicyRemovedEventType).
|
||||||
Builder()
|
Builder()
|
||||||
}
|
}
|
||||||
|
@ -86,6 +86,7 @@ func TestCommandSide_AddOrg(t *testing.T) {
|
|||||||
context.Background(),
|
context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username1",
|
"username1",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
|
@ -179,7 +179,7 @@ func (c *Commands) RemoveUser(ctx context.Context, userID, resourceOwner string,
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
if !isUserStateExists(existingUser.UserState) {
|
if !isUserStateExists(existingUser.UserState) {
|
||||||
return nil, caos_errs.ThrowNotFound(nil, "COMMAND-5M0od", "Errors.User.NotFound")
|
return nil, caos_errs.ThrowNotFound(nil, "COMMAND-m9od", "Errors.User.NotFound")
|
||||||
}
|
}
|
||||||
|
|
||||||
orgIAMPolicy, err := c.getOrgIAMPolicy(ctx, existingUser.ResourceOwner)
|
orgIAMPolicy, err := c.getOrgIAMPolicy(ctx, existingUser.ResourceOwner)
|
||||||
@ -188,7 +188,7 @@ func (c *Commands) RemoveUser(ctx context.Context, userID, resourceOwner string,
|
|||||||
}
|
}
|
||||||
var events []eventstore.EventPusher
|
var events []eventstore.EventPusher
|
||||||
userAgg := UserAggregateFromWriteModel(&existingUser.WriteModel)
|
userAgg := UserAggregateFromWriteModel(&existingUser.WriteModel)
|
||||||
events = append(events, user.NewUserRemovedEvent(ctx, userAgg, existingUser.UserName, orgIAMPolicy.UserLoginMustBeDomain))
|
events = append(events, user.NewUserRemovedEvent(ctx, userAgg, existingUser.UserName, existingUser.ExternalIDPs, orgIAMPolicy.UserLoginMustBeDomain))
|
||||||
|
|
||||||
for _, grantID := range cascadingGrantIDs {
|
for _, grantID := range cascadingGrantIDs {
|
||||||
removeEvent, _, err := c.removeUserGrant(ctx, grantID, "", true)
|
removeEvent, _, err := c.removeUserGrant(ctx, grantID, "", true)
|
||||||
|
@ -99,6 +99,7 @@ func TestCommandSide_AddUserGrant(t *testing.T) {
|
|||||||
context.Background(),
|
context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username1",
|
"username1",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
@ -670,6 +671,7 @@ func TestCommandSide_ChangeUserGrant(t *testing.T) {
|
|||||||
context.Background(),
|
context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username1",
|
"username1",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
|
@ -340,6 +340,7 @@ func TestCommandSide_RemoveExternalIDP(t *testing.T) {
|
|||||||
user.NewUserRemovedEvent(context.Background(),
|
user.NewUserRemovedEvent(context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username",
|
"username",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
@ -500,6 +501,7 @@ func TestCommandSide_ExternalLoginCheck(t *testing.T) {
|
|||||||
user.NewUserRemovedEvent(context.Background(),
|
user.NewUserRemovedEvent(context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username",
|
"username",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
|
@ -14,6 +14,7 @@ type UserWriteModel struct {
|
|||||||
eventstore.WriteModel
|
eventstore.WriteModel
|
||||||
|
|
||||||
UserName string
|
UserName string
|
||||||
|
ExternalIDPs []*domain.ExternalIDP
|
||||||
UserState domain.UserState
|
UserState domain.UserState
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -23,6 +24,7 @@ func NewUserWriteModel(userID, resourceOwner string) *UserWriteModel {
|
|||||||
AggregateID: userID,
|
AggregateID: userID,
|
||||||
ResourceOwner: resourceOwner,
|
ResourceOwner: resourceOwner,
|
||||||
},
|
},
|
||||||
|
ExternalIDPs: make([]*domain.ExternalIDP, 0),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -39,6 +41,24 @@ func (wm *UserWriteModel) Reduce() error {
|
|||||||
wm.UserState = domain.UserStateInitial
|
wm.UserState = domain.UserStateInitial
|
||||||
case *user.HumanInitializedCheckSucceededEvent:
|
case *user.HumanInitializedCheckSucceededEvent:
|
||||||
wm.UserState = domain.UserStateActive
|
wm.UserState = domain.UserStateActive
|
||||||
|
case *user.HumanExternalIDPAddedEvent:
|
||||||
|
wm.ExternalIDPs = append(wm.ExternalIDPs, &domain.ExternalIDP{IDPConfigID: e.IDPConfigID, ExternalUserID: e.ExternalUserID})
|
||||||
|
case *user.HumanExternalIDPRemovedEvent:
|
||||||
|
idx, _ := wm.ExternalIDPByID(e.IDPConfigID, e.ExternalUserID)
|
||||||
|
if idx < 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
copy(wm.ExternalIDPs[idx:], wm.ExternalIDPs[idx+1:])
|
||||||
|
wm.ExternalIDPs[len(wm.ExternalIDPs)-1] = nil
|
||||||
|
wm.ExternalIDPs = wm.ExternalIDPs[:len(wm.ExternalIDPs)-1]
|
||||||
|
case *user.HumanExternalIDPCascadeRemovedEvent:
|
||||||
|
idx, _ := wm.ExternalIDPByID(e.IDPConfigID, e.ExternalUserID)
|
||||||
|
if idx < 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
copy(wm.ExternalIDPs[idx:], wm.ExternalIDPs[idx+1:])
|
||||||
|
wm.ExternalIDPs[len(wm.ExternalIDPs)-1] = nil
|
||||||
|
wm.ExternalIDPs = wm.ExternalIDPs[:len(wm.ExternalIDPs)-1]
|
||||||
case *user.MachineAddedEvent:
|
case *user.MachineAddedEvent:
|
||||||
wm.UserName = e.UserName
|
wm.UserName = e.UserName
|
||||||
wm.UserState = domain.UserStateActive
|
wm.UserState = domain.UserStateActive
|
||||||
@ -76,6 +96,9 @@ func (wm *UserWriteModel) Query() *eventstore.SearchQueryBuilder {
|
|||||||
user.HumanAddedType,
|
user.HumanAddedType,
|
||||||
user.HumanRegisteredType,
|
user.HumanRegisteredType,
|
||||||
user.HumanInitializedCheckSucceededType,
|
user.HumanInitializedCheckSucceededType,
|
||||||
|
user.HumanExternalIDPAddedType,
|
||||||
|
user.HumanExternalIDPRemovedType,
|
||||||
|
user.HumanExternalIDPCascadeRemovedType,
|
||||||
user.MachineAddedEventType,
|
user.MachineAddedEventType,
|
||||||
user.UserUserNameChangedType,
|
user.UserUserNameChangedType,
|
||||||
user.MachineChangedEventType,
|
user.MachineChangedEventType,
|
||||||
@ -125,3 +148,12 @@ func hasUserState(check domain.UserState, states ...domain.UserState) bool {
|
|||||||
}
|
}
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (wm *UserWriteModel) ExternalIDPByID(idpID, externalUserID string) (idx int, idp *domain.ExternalIDP) {
|
||||||
|
for idx, idp = range wm.ExternalIDPs {
|
||||||
|
if idp.IDPConfigID == idpID && idp.ExternalUserID == externalUserID {
|
||||||
|
return idx, idp
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return -1, nil
|
||||||
|
}
|
||||||
|
@ -1037,6 +1037,7 @@ func TestCommandSide_RemoveUser(t *testing.T) {
|
|||||||
user.NewUserRemovedEvent(context.Background(),
|
user.NewUserRemovedEvent(context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username",
|
"username",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
@ -1056,6 +1057,71 @@ func TestCommandSide_RemoveUser(t *testing.T) {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "remove user with erxternal idp, 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,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
eventFromEventPusher(
|
||||||
|
user.NewHumanExternalIDPAddedEvent(context.Background(),
|
||||||
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
|
"idpConfigID",
|
||||||
|
"displayName",
|
||||||
|
"externalUserID",
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
expectFilter(),
|
||||||
|
expectFilter(
|
||||||
|
eventFromEventPusher(
|
||||||
|
iam.NewOrgIAMPolicyAddedEvent(context.Background(),
|
||||||
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
|
true,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
expectPush(
|
||||||
|
[]*repository.Event{
|
||||||
|
eventFromEventPusher(
|
||||||
|
user.NewUserRemovedEvent(context.Background(),
|
||||||
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
|
"username",
|
||||||
|
nil,
|
||||||
|
true,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
},
|
||||||
|
uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("username", "org1", true)),
|
||||||
|
uniqueConstraintsFromEventConstraint(user.NewRemoveExternalIDPUniqueConstraint("idpConfigID", "externalUserID")),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
},
|
||||||
|
args: args{
|
||||||
|
ctx: context.Background(),
|
||||||
|
orgID: "org1",
|
||||||
|
userID: "user1",
|
||||||
|
},
|
||||||
|
res: res{
|
||||||
|
want: &domain.ObjectDetails{
|
||||||
|
ResourceOwner: "org1",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "remove user with user memberships, ok",
|
name: "remove user with user memberships, ok",
|
||||||
fields: fields{
|
fields: fields{
|
||||||
@ -1092,6 +1158,7 @@ func TestCommandSide_RemoveUser(t *testing.T) {
|
|||||||
user.NewUserRemovedEvent(context.Background(),
|
user.NewUserRemovedEvent(context.Background(),
|
||||||
&user.NewAggregate("user1", "org1").Aggregate,
|
&user.NewAggregate("user1", "org1").Aggregate,
|
||||||
"username",
|
"username",
|
||||||
|
nil,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
|
@ -5,6 +5,7 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/caos/zitadel/internal/domain"
|
||||||
"github.com/caos/zitadel/internal/eventstore"
|
"github.com/caos/zitadel/internal/eventstore"
|
||||||
|
|
||||||
"github.com/caos/zitadel/internal/errors"
|
"github.com/caos/zitadel/internal/errors"
|
||||||
@ -162,6 +163,7 @@ type UserRemovedEvent struct {
|
|||||||
eventstore.BaseEvent `json:"-"`
|
eventstore.BaseEvent `json:"-"`
|
||||||
|
|
||||||
userName string
|
userName string
|
||||||
|
externalIDPs []*domain.ExternalIDP
|
||||||
loginMustBeDomain bool
|
loginMustBeDomain bool
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -170,13 +172,21 @@ func (e *UserRemovedEvent) Data() interface{} {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (e *UserRemovedEvent) UniqueConstraints() []*eventstore.EventUniqueConstraint {
|
func (e *UserRemovedEvent) UniqueConstraints() []*eventstore.EventUniqueConstraint {
|
||||||
return []*eventstore.EventUniqueConstraint{NewRemoveUsernameUniqueConstraint(e.userName, e.Aggregate().ResourceOwner, e.loginMustBeDomain)}
|
events := make([]*eventstore.EventUniqueConstraint, 0)
|
||||||
|
if e.userName != "" {
|
||||||
|
events = append(events, NewRemoveUsernameUniqueConstraint(e.userName, e.Aggregate().ResourceOwner, e.loginMustBeDomain))
|
||||||
|
}
|
||||||
|
for _, idp := range e.externalIDPs {
|
||||||
|
events = append(events, NewRemoveExternalIDPUniqueConstraint(idp.IDPConfigID, idp.ExternalUserID))
|
||||||
|
}
|
||||||
|
return events
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewUserRemovedEvent(
|
func NewUserRemovedEvent(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
aggregate *eventstore.Aggregate,
|
aggregate *eventstore.Aggregate,
|
||||||
userName string,
|
userName string,
|
||||||
|
externalIDPs []*domain.ExternalIDP,
|
||||||
userLoginMustBeDomain bool,
|
userLoginMustBeDomain bool,
|
||||||
) *UserRemovedEvent {
|
) *UserRemovedEvent {
|
||||||
return &UserRemovedEvent{
|
return &UserRemovedEvent{
|
||||||
@ -186,6 +196,7 @@ func NewUserRemovedEvent(
|
|||||||
UserRemovedType,
|
UserRemovedType,
|
||||||
),
|
),
|
||||||
userName: userName,
|
userName: userName,
|
||||||
|
externalIDPs: externalIDPs,
|
||||||
loginMustBeDomain: userLoginMustBeDomain,
|
loginMustBeDomain: userLoginMustBeDomain,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -275,6 +275,9 @@ func (l *Login) mapExternalUserToLoginUser(orgIamPolicy *iam_model.OrgIAMPolicyV
|
|||||||
username = linkingUser.Email
|
username = linkingUser.Email
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if username == "" {
|
||||||
|
username = linkingUser.Email
|
||||||
|
}
|
||||||
|
|
||||||
if orgIamPolicy.UserLoginMustBeDomain {
|
if orgIamPolicy.UserLoginMustBeDomain {
|
||||||
splittedUsername := strings.Split(username, "@")
|
splittedUsername := strings.Split(username, "@")
|
||||||
@ -310,6 +313,9 @@ func (l *Login) mapExternalUserToLoginUser(orgIamPolicy *iam_model.OrgIAMPolicyV
|
|||||||
displayName = linkingUser.Email
|
displayName = linkingUser.Email
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if displayName == "" {
|
||||||
|
displayName = linkingUser.Email
|
||||||
|
}
|
||||||
|
|
||||||
externalIDP := &domain.ExternalIDP{
|
externalIDP := &domain.ExternalIDP{
|
||||||
IDPConfigID: idpConfig.IDPConfigID,
|
IDPConfigID: idpConfig.IDPConfigID,
|
||||||
|
Loading…
Reference in New Issue
Block a user