From 45262e682972723412be631adaaa1dc50ce481e9 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Fri, 4 Aug 2023 11:35:36 +0200 Subject: [PATCH] fix: migrate external id of federated users (#6312) * feat: migrate external id * implement tests and some renaming * fix projection * cleanup * i18n * fix event type * handle migration for new services as well * typo --- internal/api/idp/idp.go | 23 +++- .../api/ui/login/external_provider_handler.go | 55 +++++++- internal/auth/repository/auth_request.go | 2 +- .../eventsourcing/eventstore/auth_request.go | 7 +- internal/command/idp_intent_test.go | 89 +++++++++++++ internal/command/idp_model.go | 19 +++ internal/command/user_idp_link.go | 18 +++ internal/command/user_idp_link_model.go | 8 ++ internal/command/user_idp_link_test.go | 116 +++++++++++++++++ internal/idp/providers/azuread/azuread.go | 5 +- internal/idp/providers/azuread/session.go | 29 +++++ .../idp/providers/azuread/session_test.go | 117 +++++++++++++++++- internal/idp/session.go | 9 ++ internal/query/projection/idp_user_link.go | 25 ++++ .../query/projection/idp_user_link_test.go | 36 ++++++ internal/repository/user/eventstore.go | 1 + .../repository/user/human_external_idp.go | 39 ++++++ internal/static/i18n/bg.yaml | 2 + internal/static/i18n/de.yaml | 2 + internal/static/i18n/en.yaml | 2 + internal/static/i18n/es.yaml | 2 + internal/static/i18n/fr.yaml | 2 + internal/static/i18n/it.yaml | 2 + internal/static/i18n/ja.yaml | 2 + internal/static/i18n/mk.yaml | 2 + internal/static/i18n/pl.yaml | 2 + internal/static/i18n/pt.yaml | 2 + internal/static/i18n/zh.yaml | 2 + 28 files changed, 611 insertions(+), 9 deletions(-) create mode 100644 internal/idp/providers/azuread/session.go diff --git a/internal/api/idp/idp.go b/internal/api/idp/idp.go index 37a84f5265..f9a3cd1f3a 100644 --- a/internal/api/idp/idp.go +++ b/internal/api/idp/idp.go @@ -120,6 +120,11 @@ func (h *Handler) handleCallback(w http.ResponseWriter, r *http.Request) { userID, err := h.checkExternalUser(ctx, intent.IDPID, idpUser.GetID()) logging.WithFields("intent", intent.AggregateID).OnError(err).Error("could not check if idp user already exists") + if userID == "" { + userID, err = h.tryMigrateExternalUser(ctx, intent.IDPID, idpUser, idpSession) + logging.WithFields("intent", intent.AggregateID).OnError(err).Error("migration check failed") + } + token, err := h.commands.SucceedIDPIntent(ctx, intent, idpUser, idpSession, userID) if err != nil { redirectToFailureURLErr(w, r, intent, z_errs.ThrowInternal(err, "IDP-JdD3g", "Errors.Intent.TokenCreationFailed")) @@ -128,6 +133,22 @@ func (h *Handler) handleCallback(w http.ResponseWriter, r *http.Request) { redirectToSuccessURL(w, r, intent, token, userID) } +func (h *Handler) tryMigrateExternalUser(ctx context.Context, idpID string, idpUser idp.User, idpSession idp.Session) (userID string, err error) { + migration, ok := idpSession.(idp.SessionSupportsMigration) + if !ok { + return "", nil + } + previousID, err := migration.RetrievePreviousID() + if err != nil || previousID == "" { + return "", err + } + userID, err = h.checkExternalUser(ctx, idpID, previousID) + if err != nil { + return "", err + } + return userID, h.commands.MigrateUserIDP(ctx, userID, "", idpID, previousID, idpUser.GetID()) +} + func (h *Handler) parseCallbackRequest(r *http.Request) (*externalIDPCallbackData, error) { data := new(externalIDPCallbackData) err := h.parser.Parse(r, data) @@ -196,7 +217,7 @@ func (h *Handler) fetchIDPUser(ctx context.Context, identityProvider idp.Provide case *openid.Provider: session = &openid.Session{Provider: provider, Code: code} case *azuread.Provider: - session = &oauth.Session{Provider: provider.Provider, Code: code} + session = &azuread.Session{Session: &oauth.Session{Provider: provider.Provider, Code: code}} case *github.Provider: session = &oauth.Session{Provider: provider.Provider, Code: code} case *gitlab.Provider: diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index ee7696cc4e..ceb2653a33 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -222,7 +222,7 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.externalAuthFailed(w, r, authReq, nil, nil, err) return } - session = &oauth.Session{Provider: provider.(*azuread.Provider).Provider, Code: data.Code} + session = &azuread.Session{Session: &oauth.Session{Provider: provider.(*azuread.Provider).Provider, Code: data.Code}} case domain.IDPTypeGitHub: provider, err = l.githubProvider(r.Context(), identityProvider) if err != nil { @@ -275,6 +275,46 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.handleExternalUserAuthenticated(w, r, authReq, identityProvider, session, user, l.renderNextStep) } +func (l *Login) tryMigrateExternalUserID(r *http.Request, session idp.Session, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) (previousIDMatched bool, err error) { + migration, ok := session.(idp.SessionSupportsMigration) + if !ok { + return false, nil + } + previousID, err := migration.RetrievePreviousID() + if err != nil { + return false, err + } + return l.migrateExternalUserID(r, authReq, externalUser, previousID) +} + +func (l *Login) migrateExternalUserID(r *http.Request, authReq *domain.AuthRequest, externalUser *domain.ExternalUser, previousID string) (previousIDMatched bool, err error) { + if previousID == "" { + return false, nil + } + // save the currentID, so we're able to reset to it later on if the user is not found with the old ID as well + externalUserID := externalUser.ExternalUserID + externalUser.ExternalUserID = previousID + if err = l.authRepo.CheckExternalUserLogin(setContext(r.Context(), ""), authReq.ID, authReq.AgentID, externalUser, domain.BrowserInfoFromRequest(r), true); err != nil { + // always reset to the mapped ID + externalUser.ExternalUserID = externalUserID + // but ignore the error if the user was just not found with the previousID + if errors.IsNotFound(err) { + return false, nil + } + return false, err + } + previousIDMatched = true + if err = l.authRepo.ResetLinkingUsers(r.Context(), authReq.ID, authReq.AgentID); err != nil { + return previousIDMatched, err + } + // read current auth request state (incl. authorized user) + authReq, err = l.authRepo.AuthRequestByID(r.Context(), authReq.ID, authReq.AgentID) + if err != nil { + return previousIDMatched, err + } + return previousIDMatched, l.command.MigrateUserIDP(setContext(r.Context(), authReq.UserOrgID), authReq.UserID, authReq.UserOrgID, externalUser.IDPConfigID, previousID, externalUserID) +} + // handleExternalUserAuthenticated maps the IDP user, checks for a corresponding externalID func (l *Login) handleExternalUserAuthenticated( w http.ResponseWriter, @@ -287,11 +327,22 @@ func (l *Login) handleExternalUserAuthenticated( ) { externalUser := mapIDPUserToExternalUser(user, provider.ID) // check and fill in local linked user - externalErr := l.authRepo.CheckExternalUserLogin(setContext(r.Context(), ""), authReq.ID, authReq.AgentID, externalUser, domain.BrowserInfoFromRequest(r)) + externalErr := l.authRepo.CheckExternalUserLogin(setContext(r.Context(), ""), authReq.ID, authReq.AgentID, externalUser, domain.BrowserInfoFromRequest(r), false) if externalErr != nil && !errors.IsNotFound(externalErr) { l.renderError(w, r, authReq, externalErr) return } + if externalErr != nil && errors.IsNotFound(externalErr) { + previousIDMatched, err := l.tryMigrateExternalUserID(r, session, authReq, externalUser) + if err != nil { + l.renderError(w, r, authReq, err) + return + } + // if the old ID matched, ignore the not found error from the current ID + if previousIDMatched { + externalErr = nil + } + } var err error // read current auth request state (incl. authorized user) authReq, err = l.authRepo.AuthRequestByID(r.Context(), authReq.ID, authReq.AgentID) diff --git a/internal/auth/repository/auth_request.go b/internal/auth/repository/auth_request.go index ffa70f9d60..8851b4f67e 100644 --- a/internal/auth/repository/auth_request.go +++ b/internal/auth/repository/auth_request.go @@ -15,7 +15,7 @@ type AuthRequestRepository interface { DeleteAuthRequest(ctx context.Context, id string) error CheckLoginName(ctx context.Context, id, loginName, userAgentID string) error - CheckExternalUserLogin(ctx context.Context, authReqID, userAgentID string, user *domain.ExternalUser, info *domain.BrowserInfo) error + CheckExternalUserLogin(ctx context.Context, authReqID, userAgentID string, user *domain.ExternalUser, info *domain.BrowserInfo, migrationCheck bool) error SetExternalUserLogin(ctx context.Context, authReqID, userAgentID string, user *domain.ExternalUser) error SetLinkingUser(ctx context.Context, request *domain.AuthRequest, externalUser *domain.ExternalUser) error SelectUser(ctx context.Context, id, userID, userAgentID string) error diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 26b1388229..0f81e64148 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -238,7 +238,7 @@ func (repo *AuthRequestRepo) SelectExternalIDP(ctx context.Context, authReqID, i return repo.AuthRequests.UpdateAuthRequest(ctx, request) } -func (repo *AuthRequestRepo) CheckExternalUserLogin(ctx context.Context, authReqID, userAgentID string, externalUser *domain.ExternalUser, info *domain.BrowserInfo) (err error) { +func (repo *AuthRequestRepo) CheckExternalUserLogin(ctx context.Context, authReqID, userAgentID string, externalUser *domain.ExternalUser, info *domain.BrowserInfo, migrationCheck bool) (err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() request, err := repo.getAuthRequest(ctx, authReqID, userAgentID) @@ -249,6 +249,11 @@ func (repo *AuthRequestRepo) CheckExternalUserLogin(ctx context.Context, authReq if errors.IsNotFound(err) { // clear potential user information (e.g. when username was entered but another external user was returned) request.SetUserInfo("", "", "", "", "", request.UserOrgID) + // in case the check was done with an ID, that was retrieved by a session that allows migration, + // we do not need to set the linking user and return early + if migrationCheck { + return err + } if err := repo.setLinkingUser(ctx, request, externalUser); err != nil { return err } diff --git a/internal/command/idp_intent_test.go b/internal/command/idp_intent_test.go index 08948a2009..acbb1eebf3 100644 --- a/internal/command/idp_intent_test.go +++ b/internal/command/idp_intent_test.go @@ -348,6 +348,95 @@ func TestCommands_AuthURLFromProvider(t *testing.T) { authURL: "auth?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&state=state", }, }, + { + "migrated and push", + fields{ + secretCrypto: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + eventstore: eventstoreExpect(t, + expectFilter( + eventFromEventPusherWithInstanceID( + "instance", + instance.NewOIDCIDPAddedEvent(context.Background(), &instance.NewAggregate("instance").Aggregate, + "idp", + "name", + "issuer", + "clientID", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("clientSecret"), + }, + []string{"openid", "profile", "User.Read"}, + false, + rep_idp.Options{}, + )), + eventFromEventPusherWithInstanceID( + "instance", + instance.NewOIDCIDPMigratedAzureADEvent(context.Background(), &instance.NewAggregate("instance").Aggregate, + "idp", + "name", + "clientID", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("clientSecret"), + }, + []string{"openid", "profile", "User.Read"}, + "tenant", + true, + rep_idp.Options{}, + )), + ), + expectFilter( + eventFromEventPusherWithInstanceID( + "instance", + instance.NewOIDCIDPAddedEvent(context.Background(), &instance.NewAggregate("instance").Aggregate, + "idp", + "name", + "issuer", + "clientID", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("clientSecret"), + }, + []string{"openid", "profile", "User.Read"}, + false, + rep_idp.Options{}, + )), + eventFromEventPusherWithInstanceID( + "instance", + instance.NewOIDCIDPMigratedAzureADEvent(context.Background(), &instance.NewAggregate("instance").Aggregate, + "idp", + "name", + "clientID", + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("clientSecret"), + }, + []string{"openid", "profile", "User.Read"}, + "tenant", + true, + rep_idp.Options{}, + )), + ), + ), + }, + args{ + ctx: authz.SetCtxData(context.Background(), authz.CtxData{OrgID: "ro"}), + idpID: "idp", + state: "state", + callbackURL: "url", + }, + res{ + authURL: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize?client_id=clientID&prompt=select_account&redirect_uri=url&response_type=code&scope=openid+profile+User.Read&state=state", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/command/idp_model.go b/internal/command/idp_model.go index 0849c5bd01..417f984648 100644 --- a/internal/command/idp_model.go +++ b/internal/command/idp_model.go @@ -1653,6 +1653,14 @@ func (wm *IDPTypeWriteModel) Reduce() error { wm.reduceAdded(e.ID, domain.IDPTypeLDAP, e.Aggregate()) case *org.LDAPIDPAddedEvent: wm.reduceAdded(e.ID, domain.IDPTypeLDAP, e.Aggregate()) + case *instance.OIDCIDPMigratedAzureADEvent: + wm.reduceChanged(e.ID, domain.IDPTypeAzureAD) + case *org.OIDCIDPMigratedAzureADEvent: + wm.reduceChanged(e.ID, domain.IDPTypeAzureAD) + case *instance.OIDCIDPMigratedGoogleEvent: + wm.reduceChanged(e.ID, domain.IDPTypeGoogle) + case *org.OIDCIDPMigratedGoogleEvent: + wm.reduceChanged(e.ID, domain.IDPTypeGoogle) case *instance.IDPRemovedEvent: wm.reduceRemoved(e.ID) case *org.IDPRemovedEvent: @@ -1688,6 +1696,13 @@ func (wm *IDPTypeWriteModel) reduceAdded(id string, t domain.IDPType, agg events wm.InstanceID = agg.InstanceID } +func (wm *IDPTypeWriteModel) reduceChanged(id string, t domain.IDPType) { + if wm.ID != id { + return + } + wm.Type = t +} + func (wm *IDPTypeWriteModel) reduceRemoved(id string) { if wm.ID != id { return @@ -1713,6 +1728,8 @@ func (wm *IDPTypeWriteModel) Query() *eventstore.SearchQueryBuilder { instance.GitLabSelfHostedIDPAddedEventType, instance.GoogleIDPAddedEventType, instance.LDAPIDPAddedEventType, + instance.OIDCIDPMigratedAzureADEventType, + instance.OIDCIDPMigratedGoogleEventType, instance.IDPRemovedEventType, ). EventData(map[string]interface{}{"id": wm.ID}). @@ -1729,6 +1746,8 @@ func (wm *IDPTypeWriteModel) Query() *eventstore.SearchQueryBuilder { org.GitLabSelfHostedIDPAddedEventType, org.GoogleIDPAddedEventType, org.LDAPIDPAddedEventType, + org.OIDCIDPMigratedAzureADEventType, + org.OIDCIDPMigratedGoogleEventType, org.IDPRemovedEventType, ). EventData(map[string]interface{}{"id": wm.ID}). diff --git a/internal/command/user_idp_link.go b/internal/command/user_idp_link.go index 4c03f46436..46dd96b409 100644 --- a/internal/command/user_idp_link.go +++ b/internal/command/user_idp_link.go @@ -139,6 +139,24 @@ func (c *Commands) UserIDPLoginChecked(ctx context.Context, orgID, userID string return err } +func (c *Commands) MigrateUserIDP(ctx context.Context, userID, orgID, idpConfigID, previousID, newID string) (err error) { + if userID == "" { + return caos_errs.ThrowInvalidArgument(nil, "COMMAND-Sn3l1", "Errors.IDMissing") + } + + writeModel, err := c.userIDPLinkWriteModelByID(ctx, userID, idpConfigID, previousID, orgID) + if err != nil { + return err + } + if writeModel.State != domain.UserIDPLinkStateActive { + return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-KJH2o", "Errors.User.ExternalIDP.NotFound") + } + + userAgg := UserAggregateFromWriteModel(&writeModel.WriteModel) + _, err = c.eventstore.Push(ctx, user.NewUserIDPExternalIDMigratedEvent(ctx, userAgg, idpConfigID, previousID, newID)) + 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_model.go b/internal/command/user_idp_link_model.go index 41ff5c6d52..edcd1ebd2b 100644 --- a/internal/command/user_idp_link_model.go +++ b/internal/command/user_idp_link_model.go @@ -35,6 +35,11 @@ func (wm *UserIDPLinkWriteModel) AppendEvents(events ...eventstore.Event) { continue } wm.WriteModel.AppendEvents(e) + case *user.UserIDPExternalIDMigratedEvent: + if e.IDPConfigID != wm.IDPConfigID || e.PreviousID != wm.ExternalUserID { + continue + } + wm.WriteModel.AppendEvents(e) case *user.UserIDPLinkRemovedEvent: if e.IDPConfigID != wm.IDPConfigID || e.ExternalUserID != wm.ExternalUserID { continue @@ -59,6 +64,8 @@ func (wm *UserIDPLinkWriteModel) Reduce() error { wm.DisplayName = e.DisplayName wm.ExternalUserID = e.ExternalUserID wm.State = domain.UserIDPLinkStateActive + case *user.UserIDPExternalIDMigratedEvent: + wm.ExternalUserID = e.NewID case *user.UserIDPLinkRemovedEvent: wm.State = domain.UserIDPLinkStateRemoved case *user.UserIDPLinkCascadeRemovedEvent: @@ -77,6 +84,7 @@ func (wm *UserIDPLinkWriteModel) Query() *eventstore.SearchQueryBuilder { AggregateTypes(user.AggregateType). AggregateIDs(wm.AggregateID). EventTypes(user.UserIDPLinkAddedType, + user.UserIDPExternalIDMigratedType, user.UserIDPLinkRemovedType, user.UserIDPLinkCascadeRemovedType, user.UserRemovedType). diff --git a/internal/command/user_idp_link_test.go b/internal/command/user_idp_link_test.go index af6499785a..207a8e9df5 100644 --- a/internal/command/user_idp_link_test.go +++ b/internal/command/user_idp_link_test.go @@ -669,3 +669,119 @@ func TestCommandSide_ExternalLoginCheck(t *testing.T) { }) } } + +func TestCommandSide_MigrateUserIDP(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 + previousID string + newID 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", + previousID: "previousID", + newID: "newID", + }, + res: res{ + err: caos_errs.ThrowInvalidArgument(nil, "COMMAND-Sn3l1", "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", + previousID: "previousID", + newID: "newID", + }, + res: res{ + err: caos_errs.ThrowPreconditionFailed(nil, "COMMAND-KJH2o", "Errors.User.ExternalIDP.NotFound"), + }, + }, + { + name: "external login check, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewUserIDPLinkAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "idpConfig1", + "displayName", + "previousID", + ), + ), + ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + user.NewUserIDPExternalIDMigratedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "idpConfig1", + "previousID", + "newID", + ), + ), + }, + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + orgID: "org1", + idpConfigID: "idpConfig1", + previousID: "previousID", + newID: "newID", + }, + res: res{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &Commands{ + eventstore: tt.fields.eventstore(t), + } + err := r.MigrateUserIDP(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.idpConfigID, tt.args.previousID, tt.args.newID) + assert.ErrorIs(t, err, tt.res.err) + }) + } +} diff --git a/internal/idp/providers/azuread/azuread.go b/internal/idp/providers/azuread/azuread.go index da1ce0ead5..244f383e1c 100644 --- a/internal/idp/providers/azuread/azuread.go +++ b/internal/idp/providers/azuread/azuread.go @@ -15,7 +15,8 @@ import ( const ( authURLTemplate string = "https://login.microsoftonline.com/%s/oauth2/v2.0/authorize" tokenURLTemplate string = "https://login.microsoftonline.com/%s/oauth2/v2.0/token" - userinfoURL string = "https://graph.microsoft.com/v1.0/me" + userURL string = "https://graph.microsoft.com/v1.0/me" + userinfoEndpoint string = "https://graph.microsoft.com/oidc/userinfo" ScopeUserRead string = "User.Read" ) @@ -87,7 +88,7 @@ func New(name, clientID, clientSecret, redirectURI string, scopes []string, opts rp, err := oauth.New( config, name, - userinfoURL, + userURL, func() idp.User { return &User{isEmailVerified: provider.emailVerified} }, diff --git a/internal/idp/providers/azuread/session.go b/internal/idp/providers/azuread/session.go new file mode 100644 index 0000000000..5bc7bb84c9 --- /dev/null +++ b/internal/idp/providers/azuread/session.go @@ -0,0 +1,29 @@ +package azuread + +import ( + "net/http" + + httphelper "github.com/zitadel/oidc/v2/pkg/http" + "github.com/zitadel/oidc/v2/pkg/oidc" + + "github.com/zitadel/zitadel/internal/idp/providers/oauth" +) + +// Session extends the [oauth.Session] to extend it with the [idp.SessionSupportsMigration] functionality +type Session struct { + *oauth.Session +} + +// RetrievePreviousID implements the [idp.SessionSupportsMigration] interface by returning the `sub` from the userinfo endpoint +func (s *Session) RetrievePreviousID() (string, error) { + req, err := http.NewRequest("GET", userinfoEndpoint, nil) + if err != nil { + return "", err + } + req.Header.Set("authorization", s.Tokens.TokenType+" "+s.Tokens.AccessToken) + userinfo := new(oidc.UserInfo) + if err := httphelper.HttpRequest(s.Provider.HttpClient(), req, &userinfo); err != nil { + return "", err + } + return userinfo.Subject, nil +} diff --git a/internal/idp/providers/azuread/session_test.go b/internal/idp/providers/azuread/session_test.go index 5c6c45b4ef..531d909b92 100644 --- a/internal/idp/providers/azuread/session_test.go +++ b/internal/idp/providers/azuread/session_test.go @@ -247,12 +247,12 @@ func TestSession_FetchUser(t *testing.T) { provider, err := New(tt.fields.name, tt.fields.clientID, tt.fields.clientSecret, tt.fields.redirectURI, tt.fields.scopes, tt.fields.options...) require.NoError(t, err) - session := &oauth.Session{ + session := &Session{Session: &oauth.Session{ AuthURL: tt.fields.authURL, Code: tt.fields.code, Tokens: tt.fields.tokens, Provider: provider.Provider, - } + }} user, err := session.FetchUser(context.Background()) if tt.want.err != nil && !tt.want.err(err) { @@ -294,3 +294,116 @@ func userinfo() *User { UserPrincipalName: "username", } } + +func TestSession_RetrievePreviousID(t *testing.T) { + type fields struct { + name string + clientID string + clientSecret string + redirectURI string + scopes []string + httpMock func() + tokens *oidc.Tokens[*oidc.IDTokenClaims] + } + type res struct { + id string + err bool + } + tests := []struct { + name string + fields fields + res res + }{ + { + name: "invalid token", + fields: fields{ + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + httpMock: func() { + gock.New("https://graph.microsoft.com"). + Get("/oidc/userinfo"). + Reply(401) + }, + tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + TokenType: oidc.BearerToken, + }, + IDTokenClaims: oidc.NewIDTokenClaims( + "https://login.microsoftonline.com/consumers/oauth2/v2.0", + "sub", + []string{"clientID"}, + time.Now().Add(1*time.Hour), + time.Now().Add(-1*time.Second), + "nonce", + "", + nil, + "clientID", + 0, + ), + }, + }, + res: res{ + err: true, + }, + }, + { + name: "success", + fields: fields{ + clientID: "clientID", + clientSecret: "clientSecret", + redirectURI: "redirectURI", + httpMock: func() { + gock.New("https://graph.microsoft.com"). + Get("/oidc/userinfo"). + Reply(200). + JSON(`{"sub":"sub"}`) + }, + tokens: &oidc.Tokens[*oidc.IDTokenClaims]{ + Token: &oauth2.Token{ + AccessToken: "accessToken", + TokenType: oidc.BearerToken, + }, + IDTokenClaims: oidc.NewIDTokenClaims( + "https://login.microsoftonline.com/consumers/oauth2/v2.0", + "sub", + []string{"clientID"}, + time.Now().Add(1*time.Hour), + time.Now().Add(-1*time.Second), + "nonce", + "", + nil, + "clientID", + 0, + ), + }, + }, + res: res{ + id: "sub", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer gock.Off() + tt.fields.httpMock() + a := assert.New(t) + + provider, err := New(tt.fields.name, tt.fields.clientID, tt.fields.clientSecret, tt.fields.redirectURI, tt.fields.scopes) + require.NoError(t, err) + session := &Session{Session: &oauth.Session{ + Tokens: tt.fields.tokens, + Provider: provider.Provider, + }} + + id, err := session.RetrievePreviousID() + if tt.res.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + a.Equal(tt.res.id, id) + }) + } +} diff --git a/internal/idp/session.go b/internal/idp/session.go index 8cc2dbfef9..75f2e7a1e6 100644 --- a/internal/idp/session.go +++ b/internal/idp/session.go @@ -9,3 +9,12 @@ type Session interface { GetAuthURL() string FetchUser(ctx context.Context) (User, error) } + +// SessionSupportsMigration is an optional extension to the Session interface. +// It can be implemented to support migrating users, were the initial external id has changed because of a migration of the Provider type. +// E.g. when a user was linked on a generic OIDC provider and this provider has now been migrated to an AzureAD provider. +// In this case OIDC used the `sub` claim and Azure now uses the id of the user endpoint, which differ. +// The RetrievePreviousID will return the `sub` claim again, so that the user can be matched and safely migrated to the new id. +type SessionSupportsMigration interface { + RetrievePreviousID() (previousID string, err error) +} diff --git a/internal/query/projection/idp_user_link.go b/internal/query/projection/idp_user_link.go index d4d3fb2284..ed47ff1de5 100644 --- a/internal/query/projection/idp_user_link.go +++ b/internal/query/projection/idp_user_link.go @@ -77,6 +77,10 @@ func (p *idpUserLinkProjection) reducers() []handler.AggregateReducer { Event: user.UserRemovedType, Reduce: p.reduceUserRemoved, }, + { + Event: user.UserIDPExternalIDMigratedType, + Reduce: p.reduceExternalIDMigrated, + }, }, }, { @@ -195,6 +199,27 @@ func (p *idpUserLinkProjection) reduceUserRemoved(event eventstore.Event) (*hand ), nil } +func (p *idpUserLinkProjection) reduceExternalIDMigrated(event eventstore.Event) (*handler.Statement, error) { + e, err := assertEvent[*user.UserIDPExternalIDMigratedEvent](event) + if err != nil { + return nil, errors.ThrowInvalidArgumentf(nil, "HANDL-AS3th", "reduce.wrong.event.type %s", user.UserIDPExternalIDMigratedType) + } + + return crdb.NewUpdateStatement(e, + []handler.Column{ + handler.NewCol(IDPUserLinkChangeDateCol, e.CreationDate()), + handler.NewCol(IDPUserLinkSequenceCol, e.Sequence()), + handler.NewCol(IDPUserLinkExternalUserIDCol, e.NewID), + }, + []handler.Condition{ + handler.NewCond(IDPUserLinkIDPIDCol, e.IDPConfigID), + handler.NewCond(IDPUserLinkUserIDCol, e.Aggregate().ID), + handler.NewCond(IDPUserLinkExternalUserIDCol, e.PreviousID), + 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 cc6548a980..a343fe0b63 100644 --- a/internal/query/projection/idp_user_link_test.go +++ b/internal/query/projection/idp_user_link_test.go @@ -207,6 +207,42 @@ func TestIDPUserLinkProjection_reduces(t *testing.T) { }, }, }, + { + name: "reduceExternalIDMigrated", + args: args{ + event: getEvent(testEvent( + repository.EventType(user.UserIDPExternalIDMigratedType), + user.AggregateType, + []byte(`{ + "idpConfigId": "idp-config-id", + "previousId": "previous-id", + "newId": "new-id" +}`), + ), eventstore.GenericEventMapper[user.UserIDPExternalIDMigratedEvent]), + }, + reduce: (&idpUserLinkProjection{}).reduceExternalIDMigrated, + want: wantReduce{ + aggregateType: user.AggregateType, + sequence: 15, + previousSequence: 10, + executer: &testExecuter{ + executions: []execution{ + { + expectedStmt: "UPDATE projections.idp_user_links3 SET (change_date, sequence, external_user_id) = ($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-id", + "idp-config-id", + "agg-id", + "previous-id", + "instance-id", + }, + }, + }, + }, + }, + }, { name: "org IDPConfigRemovedEvent", args: args{ diff --git a/internal/repository/user/eventstore.go b/internal/repository/user/eventstore.go index 3a1247e524..8aede0d193 100644 --- a/internal/repository/user/eventstore.go +++ b/internal/repository/user/eventstore.go @@ -67,6 +67,7 @@ func RegisterEventMappers(es *eventstore.Eventstore) { RegisterFilterEventMapper(AggregateType, UserIDPLinkRemovedType, UserIDPLinkRemovedEventMapper). RegisterFilterEventMapper(AggregateType, UserIDPLinkCascadeRemovedType, UserIDPLinkCascadeRemovedEventMapper). RegisterFilterEventMapper(AggregateType, UserIDPLoginCheckSucceededType, UserIDPCheckSucceededEventMapper). + RegisterFilterEventMapper(AggregateType, UserIDPExternalIDMigratedType, eventstore.GenericEventMapper[UserIDPExternalIDMigratedEvent]). 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 6a279951a6..e5a62e00b6 100644 --- a/internal/repository/user/human_external_idp.go +++ b/internal/repository/user/human_external_idp.go @@ -18,6 +18,7 @@ const ( UserIDPLinkAddedType = UserIDPLinkEventPrefix + "added" UserIDPLinkRemovedType = UserIDPLinkEventPrefix + "removed" UserIDPLinkCascadeRemovedType = UserIDPLinkEventPrefix + "cascade.removed" + UserIDPExternalIDMigratedType = UserIDPLinkEventPrefix + "id.migrated" UserIDPLoginCheckSucceededType = idpLoginEventPrefix + "check.succeeded" ) @@ -212,3 +213,41 @@ func UserIDPCheckSucceededEventMapper(event *repository.Event) (eventstore.Event return e, nil } + +type UserIDPExternalIDMigratedEvent struct { + eventstore.BaseEvent `json:"-"` + IDPConfigID string `json:"idpConfigId"` + PreviousID string `json:"previousId"` + NewID string `json:"newId"` +} + +func (e *UserIDPExternalIDMigratedEvent) Data() interface{} { + return e +} + +func (e *UserIDPExternalIDMigratedEvent) UniqueConstraints() []*eventstore.EventUniqueConstraint { + return nil +} + +func (e *UserIDPExternalIDMigratedEvent) SetBaseEvent(event *eventstore.BaseEvent) { + e.BaseEvent = *event +} + +func NewUserIDPExternalIDMigratedEvent( + ctx context.Context, + aggregate *eventstore.Aggregate, + idpConfigID, + previousID, + newID string, +) *UserIDPExternalIDMigratedEvent { + return &UserIDPExternalIDMigratedEvent{ + BaseEvent: *eventstore.NewBaseEventForPush( + ctx, + aggregate, + UserIDPExternalIDMigratedType, + ), + IDPConfigID: idpConfigID, + PreviousID: previousID, + NewID: newID, + } +} diff --git a/internal/static/i18n/bg.yaml b/internal/static/i18n/bg.yaml index 33dd828a72..9f0d302cd4 100644 --- a/internal/static/i18n/bg.yaml +++ b/internal/static/i18n/bg.yaml @@ -607,6 +607,8 @@ EventTypes: removed: Външният IDP е премахнат cascade: removed: Външната IDP каскада е премахната + id: + migrated: Външният потребителски идентификатор на IDP беше мигриран phone: changed: Телефонният номер е променен verified: Телефонният номер е потвърден diff --git a/internal/static/i18n/de.yaml b/internal/static/i18n/de.yaml index b485f65c67..0607b9cf2b 100644 --- a/internal/static/i18n/de.yaml +++ b/internal/static/i18n/de.yaml @@ -595,6 +595,8 @@ EventTypes: removed: Externer IDP wurde gelöscht cascade: removed: Externer IDP wurde kaskadiert gelöscht + id: + migrated: Externe UserID des IDP wurde migriert phone: changed: Telefonnummer geändert verified: Telefonnummer verifiziert diff --git a/internal/static/i18n/en.yaml b/internal/static/i18n/en.yaml index 93a5290456..b9f92456d5 100644 --- a/internal/static/i18n/en.yaml +++ b/internal/static/i18n/en.yaml @@ -595,6 +595,8 @@ EventTypes: removed: External IDP removed cascade: removed: External IDP cascade removed + id: + migrated: External UserID of IDP was migrated phone: changed: Phone number changed verified: Phone number verified diff --git a/internal/static/i18n/es.yaml b/internal/static/i18n/es.yaml index 5dc4a3ab49..e45a075918 100644 --- a/internal/static/i18n/es.yaml +++ b/internal/static/i18n/es.yaml @@ -595,6 +595,8 @@ EventTypes: removed: IDP externo eliminado cascade: removed: IDP externo eliminado en cascada + id: + migrated: Se migró el ID de usuario externo del IDP phone: changed: Número de teléfono modificado verified: Número de teléfono verificado diff --git a/internal/static/i18n/fr.yaml b/internal/static/i18n/fr.yaml index b80a9ed33e..150d9b0302 100644 --- a/internal/static/i18n/fr.yaml +++ b/internal/static/i18n/fr.yaml @@ -593,6 +593,8 @@ EventTypes: removed: Externer IDP supprimé cascade: removed: Externer IDP cascade supprimé + îd: + migrated: L'ID utilisateur externe de l'IDP a été migré phone: changed: Le numéro de téléphone a changé verified: Numéro de téléphone vérifié diff --git a/internal/static/i18n/it.yaml b/internal/static/i18n/it.yaml index e1bd873bb1..fc50f082cf 100644 --- a/internal/static/i18n/it.yaml +++ b/internal/static/i18n/it.yaml @@ -593,6 +593,8 @@ EventTypes: removed: IDP esterno rimosso cascade: removed: Cascata IDP rimossa + id: + migrated: L'ID utente esterno dell'IDP è stato migrato phone: changed: Numero di telefono cambiato verified: Numero di telefono verificato diff --git a/internal/static/i18n/ja.yaml b/internal/static/i18n/ja.yaml index eadf7ad5ab..bbf565e071 100644 --- a/internal/static/i18n/ja.yaml +++ b/internal/static/i18n/ja.yaml @@ -580,6 +580,8 @@ EventTypes: removed: 外部IDPの削除 cascade: removed: 外部IDPカスケードの削除 + id: + migrated: IDP の外部ユーザー ID が移行されました phone: changed: 電話番号の変更 verified: 電話番号の検証 diff --git a/internal/static/i18n/mk.yaml b/internal/static/i18n/mk.yaml index c3fcfdf69b..78d059f1f3 100644 --- a/internal/static/i18n/mk.yaml +++ b/internal/static/i18n/mk.yaml @@ -591,6 +591,8 @@ EventTypes: removed: Отстранет надворешен IDP cascade: removed: Отстранета каскадата на надворешни IDP + id: + migrated: Надворешниот кориснички ID на IDP е мигриран phone: changed: Променет број на телефон verified: Верифициран број на телефон diff --git a/internal/static/i18n/pl.yaml b/internal/static/i18n/pl.yaml index aa84c8ca5b..fb7b80114a 100644 --- a/internal/static/i18n/pl.yaml +++ b/internal/static/i18n/pl.yaml @@ -595,6 +595,8 @@ EventTypes: removed: Usunięto zewnętrzne IDP cascade: removed: Usunięto kaskadę zewnętrznego IDP + id: + migrated: Identyfikator użytkownika zewnętrznego dostawcy tożsamości został przeniesiony phone: changed: Numer telefonu zmieniony verified: Numer telefonu zweryfikowany diff --git a/internal/static/i18n/pt.yaml b/internal/static/i18n/pt.yaml index 4f3c5af85c..932ed66543 100644 --- a/internal/static/i18n/pt.yaml +++ b/internal/static/i18n/pt.yaml @@ -586,6 +586,8 @@ EventTypes: removed: IDP externo removido cascade: removed: Cascade de IDP externo removido + id: + migrated: O ID de usuário externo do IDP foi migrado phone: changed: Número de telefone alterado verified: Número de telefone verificado diff --git a/internal/static/i18n/zh.yaml b/internal/static/i18n/zh.yaml index fb068a2337..10f7072d30 100644 --- a/internal/static/i18n/zh.yaml +++ b/internal/static/i18n/zh.yaml @@ -589,6 +589,8 @@ EventTypes: removed: 移除了外部 IDP cascade: removed: 移除了外部 IDP + id: + migrated: IDP 的外部用户 ID 已迁移 phone: changed: 修改手机号码 verified: 已验证手机号码