From b4cb34ec0c9d18beecb07e74c143a2c722c23bdf Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Wed, 26 Feb 2025 08:47:22 +0100 Subject: [PATCH] fix: also update identifiers --- selfservice/strategy/oidc/strategy_login.go | 23 ++++++++++++++++++--- selfservice/strategy/oidc/strategy_test.go | 8 ++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index 64c6dbead236..3d50509dbce5 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/selfservice/strategy/oidc/strategy_login.go @@ -143,14 +143,15 @@ func (s *Strategy) handleConflictingIdentity(ctx context.Context, w http.Respons if _, ok := existingIdentity.Credentials[s.ID()]; !ok { existingIdentity.SetCredentials(s.ID(), *creds) } else { - + newCreds := existingIdentity.Credentials[s.ID()] var conf identity.CredentialsOIDC - if err = json.Unmarshal(existingIdentity.Credentials[s.ID()].Config, &conf); err != nil { + if err = json.Unmarshal(newCreds.Config, &conf); err != nil { return ConflictingIdentityVerdictUnknown, nil, nil, s.HandleError(ctx, w, r, loginFlow, provider.Config().ID, newIdentity.Traits, err) } // If there exists a provider in the existing identity for the same provider, we // need to merge the providers, otherwise we just add the new provider. var providerWasUpdated bool + var identifierWasUpdated bool newProvider := identity.CredentialsOIDCProvider{ Subject: claims.Subject, Provider: provider.Config().ID, @@ -159,17 +160,33 @@ func (s *Strategy) handleConflictingIdentity(ctx context.Context, w http.Respons InitialRefreshToken: token.GetRefreshToken(), Organization: provider.Config().OrganizationID, } + newIdentifier := identity.OIDCUniqueID(newProvider.Provider, newProvider.Subject) for i, p := range conf.Providers { if p.Provider == newProvider.Provider { conf.Providers[i] = newProvider providerWasUpdated = true + + // Also replace the identifier in the list of identifiers + oldIdentifier := identity.OIDCUniqueID(p.Provider, p.Subject) + for j, identifier := range newCreds.Identifiers { + if identifier == oldIdentifier { + newCreds.Identifiers[j] = newIdentifier + identifierWasUpdated = true + break + } + } + break } } + if !identifierWasUpdated { + newCreds.Identifiers = append(newCreds.Identifiers, newIdentifier) + } if !providerWasUpdated { conf.Providers = append(conf.Providers, newProvider) } - if err = existingIdentity.SetCredentialsWithConfig(s.ID(), existingIdentity.Credentials[s.ID()], conf); err != nil { + + if err = existingIdentity.SetCredentialsWithConfig(s.ID(), newCreds, conf); err != nil { return ConflictingIdentityVerdictUnknown, nil, nil, s.HandleError(ctx, w, r, loginFlow, provider.Config().ID, newIdentity.Traits, err) } } diff --git a/selfservice/strategy/oidc/strategy_test.go b/selfservice/strategy/oidc/strategy_test.go index 6ee556db5961..0470d16bef39 100644 --- a/selfservice/strategy/oidc/strategy_test.go +++ b/selfservice/strategy/oidc/strategy_test.go @@ -1746,13 +1746,13 @@ func TestStrategy(t *testing.T) { }) i.SetCredentials(identity.CredentialsTypeOIDC, identity.Credentials{ Type: identity.CredentialsTypeOIDC, - Identifiers: []string{subject}, + Identifiers: []string{"valid:stub", "other:other-identifier"}, Config: sqlxx.JSONRawMessage(`{"providers": [{ - "subject": "", + "subject": "stub", "provider": "valid", "use_auto_link": true },{ - "subject": "", + "subject": "other-identifier", "provider": "other", "use_auto_link": true }]}`), @@ -1775,6 +1775,8 @@ func TestStrategy(t *testing.T) { i, err = reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, i.ID) require.NoError(t, err) assert.False(t, gjson.GetBytes(i.Credentials["oidc"].Config, "providers.0.use_auto_link").Bool()) + assert.NotContains(t, i.Credentials["oidc"].Identifiers, "valid:stub") + assert.Contains(t, i.Credentials["oidc"].Identifiers, "valid:user-with-use-auto-link@ory.sh") assert.True(t, gjson.GetBytes(i.Credentials["oidc"].Config, "providers.1.use_auto_link").Bool()) }) })