From aa3c352ae7a53553f0e8db2614fef21d74c453e4 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Fri, 8 Dec 2023 19:22:07 +0200 Subject: [PATCH] fix: update external username on idp if auto update is enabled (#7048) * fix: update external username on idp if auto update is enabled * update errors package --- .../api/ui/login/external_provider_handler.go | 33 +++++ internal/command/user_idp_link.go | 21 +++ internal/command/user_idp_link_test.go | 138 ++++++++++++++++++ internal/query/projection/idp_user_link.go | 25 ++++ .../query/projection/idp_user_link_test.go | 35 +++++ internal/repository/user/eventstore.go | 1 + .../repository/user/human_external_idp.go | 47 +++++- 7 files changed, 296 insertions(+), 4 deletions(-) diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 2a01a74f45..fd009cf691 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -693,6 +693,9 @@ func (l *Login) updateExternalUser(ctx context.Context, authReq *domain.AuthRequ err = l.updateExternalUserProfile(ctx, user, externalUser) logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update profile") + err = l.updateExternalUsername(ctx, user, externalUser) + logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to update external username") + return nil } @@ -757,6 +760,36 @@ func (l *Login) updateExternalUserProfile(ctx context.Context, user *query.User, return err } +func (l *Login) updateExternalUsername(ctx context.Context, user *query.User, externalUser *domain.ExternalUser) error { + externalIDQuery, err := query.NewIDPUserLinksExternalIDSearchQuery(externalUser.ExternalUserID) + if err != nil { + return err + } + idpIDQuery, err := query.NewIDPUserLinkIDPIDSearchQuery(externalUser.IDPConfigID) + if err != nil { + return err + } + userIDQuery, err := query.NewIDPUserLinksUserIDSearchQuery(user.ID) + if err != nil { + return err + } + links, err := l.query.IDPUserLinks(ctx, &query.IDPUserLinksSearchQuery{Queries: []query.SearchQuery{externalIDQuery, idpIDQuery, userIDQuery}}, false) + if err != nil || len(links.Links) == 0 { + return err + } + if links.Links[0].ProvidedUsername == externalUser.PreferredUsername { + return nil + } + return l.command.UpdateUserIDPLinkUsername( + setContext(ctx, user.ResourceOwner), + user.ID, + user.ResourceOwner, + externalUser.IDPConfigID, + externalUser.ExternalUserID, + externalUser.PreferredUsername, + ) +} + func hasEmailChanged(user *query.User, externalUser *domain.ExternalUser) bool { externalUser.Email = externalUser.Email.Normalize() if externalUser.Email == "" { diff --git a/internal/command/user_idp_link.go b/internal/command/user_idp_link.go index 29bebd3bc1..afbdb47cdc 100644 --- a/internal/command/user_idp_link.go +++ b/internal/command/user_idp_link.go @@ -160,6 +160,27 @@ func (c *Commands) MigrateUserIDP(ctx context.Context, userID, orgID, idpConfigI return err } +func (c *Commands) UpdateUserIDPLinkUsername(ctx context.Context, userID, orgID, idpConfigID, externalID, newUsername string) (err error) { + if userID == "" { + return zerrors.ThrowInvalidArgument(nil, "COMMAND-SFegz", "Errors.IDMissing") + } + + writeModel, err := c.userIDPLinkWriteModelByID(ctx, userID, idpConfigID, externalID, orgID) + if err != nil { + return err + } + if writeModel.State != domain.UserIDPLinkStateActive { + return zerrors.ThrowPreconditionFailed(nil, "COMMAND-DGhre", "Errors.User.ExternalIDP.NotFound") + } + if writeModel.DisplayName == newUsername { + return nil + } + + userAgg := UserAggregateFromWriteModel(&writeModel.WriteModel) //nolint:contextcheck + _, err = c.eventstore.Push(ctx, user.NewUserIDPExternalUsernameEvent(ctx, userAgg, idpConfigID, externalID, newUsername)) + return err +} + func (c *Commands) userIDPLinkWriteModelByID(ctx context.Context, userID, idpConfigID, externalUserID, resourceOwner string) (writeModel *UserIDPLinkWriteModel, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() diff --git a/internal/command/user_idp_link_test.go b/internal/command/user_idp_link_test.go index 4fd8c10824..f1f7929686 100644 --- a/internal/command/user_idp_link_test.go +++ b/internal/command/user_idp_link_test.go @@ -937,3 +937,141 @@ func TestCommandSide_MigrateUserIDP(t *testing.T) { }) } } + +func TestCommandSide_UpdateUserIDPLinkUsername(t *testing.T) { + type fields struct { + eventstore func(t *testing.T) *eventstore.Eventstore + } + type args struct { + ctx context.Context + userID string + orgID string + idpConfigID string + externalUserID string + newUsername string + } + type res struct { + err error + } + tests := []struct { + name string + fields fields + args args + res res + }{ + { + name: "userid missing, invalid argument error", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + ctx: context.Background(), + userID: "", + orgID: "org1", + idpConfigID: "idpConfig1", + externalUserID: "externalUserID", + newUsername: "newUsername", + }, + res: res{ + err: zerrors.ThrowInvalidArgument(nil, "COMMAND-SFegz", "Errors.IDMissing"), + }, + }, + { + name: "idp link not active, precondition failed error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewUserIDPLinkAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "idpConfig1", + "displayName", + "externalUserID", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + orgID: "org1", + idpConfigID: "idpConfig1", + externalUserID: "otherID", + newUsername: "newUsername", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-DGhre", "Errors.User.ExternalIDP.NotFound"), + }, + }, + { + name: "external username not changed, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewUserIDPLinkAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "idpConfig1", + "displayName", + "externalUserID", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + orgID: "org1", + idpConfigID: "idpConfig1", + externalUserID: "externalUserID", + newUsername: "displayName", + }, + res: res{}, + }, + { + name: "external username update, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewUserIDPLinkAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "idpConfig1", + "displayName", + "externalUserID", + ), + ), + ), + expectPush( + user.NewUserIDPExternalUsernameEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "idpConfig1", + "externalUserID", + "newUsername", + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + orgID: "org1", + idpConfigID: "idpConfig1", + externalUserID: "externalUserID", + newUsername: "newUsername", + }, + res: res{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &Commands{ + eventstore: tt.fields.eventstore(t), + } + err := r.UpdateUserIDPLinkUsername(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.idpConfigID, tt.args.externalUserID, tt.args.newUsername) + assert.ErrorIs(t, err, tt.res.err) + }) + } +} diff --git a/internal/query/projection/idp_user_link.go b/internal/query/projection/idp_user_link.go index 0449d9d0b0..edcaf4fc86 100644 --- a/internal/query/projection/idp_user_link.go +++ b/internal/query/projection/idp_user_link.go @@ -82,6 +82,10 @@ func (p *idpUserLinkProjection) Reducers() []handler.AggregateReducer { Event: user.UserIDPExternalIDMigratedType, Reduce: p.reduceExternalIDMigrated, }, + { + Event: user.UserIDPExternalUsernameChangedType, + Reduce: p.reduceExternalUsernameChanged, + }, }, }, { @@ -216,6 +220,27 @@ func (p *idpUserLinkProjection) reduceExternalIDMigrated(event eventstore.Event) ), nil } +func (p *idpUserLinkProjection) reduceExternalUsernameChanged(event eventstore.Event) (*handler.Statement, error) { + e, err := assertEvent[*user.UserIDPExternalUsernameEvent](event) + if err != nil { + return nil, err + } + + return handler.NewUpdateStatement(e, + []handler.Column{ + handler.NewCol(IDPUserLinkChangeDateCol, e.CreationDate()), + handler.NewCol(IDPUserLinkSequenceCol, e.Sequence()), + handler.NewCol(IDPUserLinkDisplayNameCol, e.ExternalUsername), + }, + []handler.Condition{ + handler.NewCond(IDPUserLinkIDPIDCol, e.IDPConfigID), + handler.NewCond(IDPUserLinkUserIDCol, e.Aggregate().ID), + handler.NewCond(IDPUserLinkExternalUserIDCol, e.ExternalUserID), + handler.NewCond(IDPUserLinkInstanceIDCol, e.Aggregate().InstanceID), + }, + ), nil +} + func (p *idpUserLinkProjection) reduceIDPConfigRemoved(event eventstore.Event) (*handler.Statement, error) { var idpID string diff --git a/internal/query/projection/idp_user_link_test.go b/internal/query/projection/idp_user_link_test.go index 6024cbcbe3..6545d30b40 100644 --- a/internal/query/projection/idp_user_link_test.go +++ b/internal/query/projection/idp_user_link_test.go @@ -238,6 +238,41 @@ func TestIDPUserLinkProjection_reduces(t *testing.T) { }, }, }, + { + name: "reduceExternalUsernameChanged", + args: args{ + event: getEvent(testEvent( + user.UserIDPExternalUsernameChangedType, + user.AggregateType, + []byte(`{ + "idpConfigId": "idp-config-id", + "userId": "external-user-id", + "username": "new-username" +}`), + ), eventstore.GenericEventMapper[user.UserIDPExternalUsernameEvent]), + }, + reduce: (&idpUserLinkProjection{}).reduceExternalUsernameChanged, + want: wantReduce{ + aggregateType: user.AggregateType, + sequence: 15, + executer: &testExecuter{ + executions: []execution{ + { + expectedStmt: "UPDATE projections.idp_user_links3 SET (change_date, sequence, display_name) = ($1, $2, $3) WHERE (idp_id = $4) AND (user_id = $5) AND (external_user_id = $6) AND (instance_id = $7)", + expectedArgs: []interface{}{ + anyArg{}, + uint64(15), + "new-username", + "idp-config-id", + "agg-id", + "external-user-id", + "instance-id", + }, + }, + }, + }, + }, + }, { name: "org IDPConfigRemovedEvent", args: args{ diff --git a/internal/repository/user/eventstore.go b/internal/repository/user/eventstore.go index 414895d5a2..289d8af40a 100644 --- a/internal/repository/user/eventstore.go +++ b/internal/repository/user/eventstore.go @@ -68,6 +68,7 @@ func RegisterEventMappers(es *eventstore.Eventstore) { RegisterFilterEventMapper(AggregateType, UserIDPLinkCascadeRemovedType, UserIDPLinkCascadeRemovedEventMapper). RegisterFilterEventMapper(AggregateType, UserIDPLoginCheckSucceededType, UserIDPCheckSucceededEventMapper). RegisterFilterEventMapper(AggregateType, UserIDPExternalIDMigratedType, eventstore.GenericEventMapper[UserIDPExternalIDMigratedEvent]). + RegisterFilterEventMapper(AggregateType, UserIDPExternalUsernameChangedType, eventstore.GenericEventMapper[UserIDPExternalUsernameEvent]). RegisterFilterEventMapper(AggregateType, HumanEmailChangedType, HumanEmailChangedEventMapper). RegisterFilterEventMapper(AggregateType, HumanEmailVerifiedType, HumanEmailVerifiedEventMapper). RegisterFilterEventMapper(AggregateType, HumanEmailVerificationFailedType, HumanEmailVerificationFailedEventMapper). diff --git a/internal/repository/user/human_external_idp.go b/internal/repository/user/human_external_idp.go index e82334b81f..b5665ea4da 100644 --- a/internal/repository/user/human_external_idp.go +++ b/internal/repository/user/human_external_idp.go @@ -12,10 +12,11 @@ const ( UserIDPLinkEventPrefix = humanEventPrefix + "externalidp." idpLoginEventPrefix = humanEventPrefix + "externallogin." - UserIDPLinkAddedType = UserIDPLinkEventPrefix + "added" - UserIDPLinkRemovedType = UserIDPLinkEventPrefix + "removed" - UserIDPLinkCascadeRemovedType = UserIDPLinkEventPrefix + "cascade.removed" - UserIDPExternalIDMigratedType = UserIDPLinkEventPrefix + "id.migrated" + UserIDPLinkAddedType = UserIDPLinkEventPrefix + "added" + UserIDPLinkRemovedType = UserIDPLinkEventPrefix + "removed" + UserIDPLinkCascadeRemovedType = UserIDPLinkEventPrefix + "cascade.removed" + UserIDPExternalIDMigratedType = UserIDPLinkEventPrefix + "id.migrated" + UserIDPExternalUsernameChangedType = UserIDPLinkEventPrefix + "username.changed" UserIDPLoginCheckSucceededType = idpLoginEventPrefix + "check.succeeded" ) @@ -248,3 +249,41 @@ func NewUserIDPExternalIDMigratedEvent( NewID: newID, } } + +type UserIDPExternalUsernameEvent struct { + eventstore.BaseEvent `json:"-"` + IDPConfigID string `json:"idpConfigId"` + ExternalUserID string `json:"userId"` + ExternalUsername string `json:"username"` +} + +func (e *UserIDPExternalUsernameEvent) Payload() interface{} { + return e +} + +func (e *UserIDPExternalUsernameEvent) UniqueConstraints() []*eventstore.UniqueConstraint { + return nil +} + +func (e *UserIDPExternalUsernameEvent) SetBaseEvent(event *eventstore.BaseEvent) { + e.BaseEvent = *event +} + +func NewUserIDPExternalUsernameEvent( + ctx context.Context, + aggregate *eventstore.Aggregate, + idpConfigID, + externalUserID, + externalUsername string, +) *UserIDPExternalUsernameEvent { + return &UserIDPExternalUsernameEvent{ + BaseEvent: *eventstore.NewBaseEventForPush( + ctx, + aggregate, + UserIDPExternalUsernameChangedType, + ), + IDPConfigID: idpConfigID, + ExternalUserID: externalUserID, + ExternalUsername: externalUsername, + } +}