fix: also update identifiers

identity/identity.go

@@ -8,6 +8,7 @@ import (
 	"database/sql/driver"
 	"encoding/json"
 	"fmt"
+	"slices"
 	"sync"
 	"time"
 
@@ -303,6 +304,56 @@ func (i *Identity) CopyWithoutCredentials() *Identity {
 	return &ii
 }
 
+// MergeOIDCCredentials merges the new credentials into the existing credentials.
+// If the provider already exists, the provider is replace and the identifier is
+// updated. This function requires the new credentials to have exactly one
+// provider and one identifier, as returned by identity.NewCredentialsOIDC.
+func (i *Identity) MergeOIDCCredentials(t CredentialsType, newCreds Credentials) (err error) {
+	creds, ok := i.Credentials[t]
+	if !ok {
+		i.SetCredentials(t, newCreds)
+		return nil
+	}
+
+	var conf CredentialsOIDC
+	if err = json.Unmarshal(creds.Config, &conf); err != nil {
+		return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to decode old %s credentials from JSON: %s", creds.Config, err))
+	}
+
+	var newConf CredentialsOIDC
+	if err = json.Unmarshal(newCreds.Config, &newConf); err != nil {
+		return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Unable to decode new %s credentials from JSON: %s", newCreds.Config, err))
+	}
+
+	if len(newConf.Providers) != 1 {
+		return errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected exactly one provider to merge credentials."))
+	}
+	newProvider := newConf.Providers[0]
+
+	// The identifier should have been set already, but we set it here just in case.
+	if len(newCreds.Identifiers) != 1 {
+		newCreds.Identifiers = []string{OIDCUniqueID(newProvider.Provider, newProvider.Subject)}
+	}
+
+	// Delete `use_auto_link` providers and their identifiers
+	var obsoleteIdentifiers []string
+	conf.Providers = slices.DeleteFunc(conf.Providers, func(p CredentialsOIDCProvider) bool {
+		if p.Provider == newProvider.Provider && p.UseAutoLink {
+			obsoleteIdentifiers = append(obsoleteIdentifiers, OIDCUniqueID(p.Provider, p.Subject))
+			return true
+		}
+		return false
+	})
+	creds.Identifiers = slices.DeleteFunc(creds.Identifiers, func(identifier string) bool {
+		return slices.Contains(obsoleteIdentifiers, identifier)
+	})
+
+	creds.Identifiers = append(creds.Identifiers, newCreds.Identifiers...)
+	conf.Providers = append(conf.Providers, newProvider)
+
+	return i.SetCredentialsWithConfig(t, creds, conf)
+}
+
 func NewIdentity(traitsSchemaID string) *Identity {
 	if traitsSchemaID == "" {
 		traitsSchemaID = config.DefaultIdentityTraitsSchemaID

identity/identity_test.go

@@ -437,3 +437,176 @@ func TestDeleteCredentialOIDCFromIdentity(t *testing.T) {
 	require.NoError(t, err)
 	assert.EqualValues(t, CredentialsOIDC{Providers: []CredentialsOIDCProvider{{Provider: "baz", Subject: "5678"}}}, cfg)
 }
+
+func TestMergeOIDCCredentials(t *testing.T) {
+	for _, tc := range []struct {
+		name           string
+		identity       *Identity
+		newCredentials Credentials
+
+		expectedIdentity *Identity
+		assertErr        assert.ErrorAssertionFunc
+	}{
+		{
+			name:     "adds OIDC credential if not exists",
+			identity: &Identity{},
+			newCredentials: Credentials{
+				Type:        CredentialsTypeOIDC,
+				Identifiers: []string{"oidc:1234"},
+				Config:      sqlxx.JSONRawMessage(`{"providers":[{"provider":"oidc","subject":"1234"}]}`),
+			},
+
+			expectedIdentity: &Identity{
+				Credentials: map[CredentialsType]Credentials{
+					CredentialsTypeOIDC: {
+						Type:        CredentialsTypeOIDC,
+						Identifiers: []string{"oidc:1234"},
+						Config:      sqlxx.JSONRawMessage(`{"providers":[{"provider":"oidc","subject":"1234"}]}`),
+					},
+				},
+			},
+		},
+		{
+			name: "merges OIDC credential if exists",
+			identity: &Identity{
+				Credentials: map[CredentialsType]Credentials{
+					CredentialsTypePassword: {
+						Type:        CredentialsTypePassword,
+						Identifiers: []string{"[email protected]"},
+					},
+					CredentialsTypeOIDC: {
+						Type:        CredentialsTypeOIDC,
+						Identifiers: []string{"foo", "replace:1234", "bar", "baz", "replace:abc", "replace:dont-replace"},
+						Config: sqlxx.JSONRawMessage(`{"providers": [
+	{"provider": "replace", "subject": "1234", "use_auto_link": true},
+	{"provider": "dont-touch", "subject": "foo"},
+	{"provider": "replace", "subject": "abc", "use_auto_link": true},
+	{"provider": "also-dont-touch", "subject": "bar", "use_auto_link": true},
+	{"provider": "replace", "subject": "dont-replace", "use_auto_link": false}
+]}`),
+					},
+				},
+			},
+			newCredentials: Credentials{
+				Type:        CredentialsTypeOIDC,
+				Identifiers: []string{},
+				Config:      sqlxx.JSONRawMessage(`{"providers": [{"provider": "replace", "subject": "new-subject"}]}`),
+			},
+
+			expectedIdentity: &Identity{
+				Credentials: map[CredentialsType]Credentials{
+					CredentialsTypeOIDC: {
+						Type:        CredentialsTypeOIDC,
+						Identifiers: []string{"foo", "bar", "baz", "replace:dont-replace", "replace:new-subject"},
+						Config: sqlxx.JSONRawMessage(`{
+  "providers" : [ {
+    "subject" : "foo",
+    "provider" : "dont-touch",
+    "initial_id_token" : "",
+    "initial_access_token" : "",
+    "initial_refresh_token" : ""
+  }, {
+    "subject" : "bar",
+    "provider" : "also-dont-touch",
+    "initial_id_token" : "",
+    "initial_access_token" : "",
+    "initial_refresh_token" : "",
+    "use_auto_link": true
+  }, {
+    "subject" : "dont-replace",
+    "provider" : "replace",
+    "initial_id_token" : "",
+    "initial_access_token" : "",
+    "initial_refresh_token" : ""
+  }, {
+    "subject" : "new-subject",
+    "provider" : "replace",
+    "initial_id_token" : "",
+    "initial_access_token" : "",
+    "initial_refresh_token" : ""
+  } ]
+}`),
+					},
+					CredentialsTypePassword: {
+						Type:        CredentialsTypePassword,
+						Identifiers: []string{"[email protected]"},
+					},
+				},
+			},
+		},
+		{
+			name: "errs if new credential has no provider",
+			identity: &Identity{
+				Credentials: map[CredentialsType]Credentials{
+					CredentialsTypeOIDC: {
+						Type:        CredentialsTypeOIDC,
+						Identifiers: []string{"oidc:1234"},
+						Config:      sqlxx.JSONRawMessage(`{"providers": [{"provider": "oidc", "subject": "1234"}]}`),
+					},
+				},
+			},
+			newCredentials: Credentials{
+				Type:        CredentialsTypeOIDC,
+				Identifiers: []string{"oidc:1234"},
+				Config:      sqlxx.JSONRawMessage(`{"providers": []}`),
+			},
+
+			assertErr: assert.Error,
+		},
+		{
+			name: "errs if identity credentials are invalid",
+			identity: &Identity{
+				Credentials: map[CredentialsType]Credentials{
+					CredentialsTypeOIDC: {
+						Type:        CredentialsTypeOIDC,
+						Identifiers: []string{"oidc:1234"},
+						Config:      sqlxx.JSONRawMessage("invalid"),
+					},
+				},
+			},
+			newCredentials: Credentials{
+				Type:        CredentialsTypeOIDC,
+				Identifiers: []string{"oidc:1234"},
+				Config:      sqlxx.JSONRawMessage(`{"providers": [{"provider": "replace", "subject": "new-subject"}]}`),
+			},
+
+			assertErr: assert.Error,
+		},
+		{
+			name: "errs if new credential config is invalid",
+			identity: &Identity{
+				Credentials: map[CredentialsType]Credentials{
+					CredentialsTypeOIDC: {
+						Type:        CredentialsTypeOIDC,
+						Identifiers: []string{"oidc:1234"},
+						Config:      sqlxx.JSONRawMessage(`{"providers": [{"provider": "oidc", "subject": "1234"}]}`),
+					},
+				},
+			},
+			newCredentials: Credentials{
+				Type:        CredentialsTypeOIDC,
+				Identifiers: []string{"oidc:1234"},
+				Config:      sqlxx.JSONRawMessage(`invalid`),
+			},
+
+			assertErr: assert.Error,
+		},
+	} {
+		t.Run("case="+tc.name, func(t *testing.T) {
+			err := tc.identity.MergeOIDCCredentials(CredentialsTypeOIDC, tc.newCredentials)
+
+			if tc.assertErr != nil {
+				tc.assertErr(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			if tc.expectedIdentity != nil {
+				var buf bytes.Buffer
+				require.NoError(t, json.Compact(&buf, tc.expectedIdentity.Credentials[CredentialsTypeOIDC].Config))
+				tc.expectedIdentity.UpsertCredentialsConfig(CredentialsTypeOIDC, buf.Bytes(), 0)
+				assert.EqualExportedValues(t, tc.expectedIdentity, tc.identity)
+			}
+		})
+	}
+}

selfservice/strategy/oidc/strategy_login.go

@@ -140,39 +140,10 @@
 
 	verdict = s.conflictingIdentityPolicy(ctx, existingIdentity, newIdentity, provider, claims)
 	if verdict == ConflictingIdentityVerdictMerge {
-		if _, ok := existingIdentity.Credentials[s.ID()]; !ok {
-			existingIdentity.SetCredentials(s.ID(), *creds)
-		} else {
-
-			var conf identity.CredentialsOIDC
-			if err = json.Unmarshal(existingIdentity.Credentials[s.ID()].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
-			newProvider := identity.CredentialsOIDCProvider{
-				Subject:             claims.Subject,
-				Provider:            provider.Config().ID,
-				InitialIDToken:      token.GetIDToken(),
-				InitialAccessToken:  token.GetAccessToken(),
-				InitialRefreshToken: token.GetRefreshToken(),
-				Organization:        provider.Config().OrganizationID,
-			}
-			for i, p := range conf.Providers {
-				if p.Provider == newProvider.Provider {
-					conf.Providers[i] = newProvider
-					providerWasUpdated = true
-					break
-				}
-			}
-			if !providerWasUpdated {
-				conf.Providers = append(conf.Providers, newProvider)
-			}
-			if err = existingIdentity.SetCredentialsWithConfig(s.ID(), existingIdentity.Credentials[s.ID()], conf); err != nil {
-				return ConflictingIdentityVerdictUnknown, nil, nil, s.HandleError(ctx, w, r, loginFlow, provider.Config().ID, newIdentity.Traits, err)
-			}
+		if err = existingIdentity.MergeOIDCCredentials(s.ID(), *creds); err != nil {
+			return ConflictingIdentityVerdictUnknown, nil, nil, s.HandleError(ctx, w, r, loginFlow, provider.Config().ID, newIdentity.Traits, err)
 		}
+
 		if err = s.d.PrivilegedIdentityPool().UpdateIdentity(ctx, existingIdentity); err != nil {
 			return ConflictingIdentityVerdictUnknown, nil, nil, s.HandleError(ctx, w, r, loginFlow, provider.Config().ID, newIdentity.Traits, err)
 		}

selfservice/strategy/oidc/strategy_test.go

@@ -1536,13 +1536,13 @@ func TestStrategy(t *testing.T) {
 		return res, body
 	}
 
-	checkCredentialsLinked := func(res *http.Response, body []byte, identityID uuid.UUID, provider string) {
+	checkCredentialsLinked := func(t *testing.T, res *http.Response, body []byte, identityID uuid.UUID, provider string) {
 		assert.Contains(t, res.Request.URL.String(), returnTS.URL, "%s", body)
 		assert.Equal(t, strings.ToLower(subject), gjson.GetBytes(body, "identity.traits.subject").String(), "%s", body)
 		i, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(ctx, identityID)
 		require.NoError(t, err)
 		assert.NotEmpty(t, i.Credentials["oidc"], "%+v", i.Credentials)
-		assert.Equal(t, provider, gjson.GetBytes(i.Credentials["oidc"].Config, "providers.0.provider").String(),
+		assert.True(t, gjson.GetBytes(i.Credentials["oidc"].Config, fmt.Sprintf("providers.#(provider=%q)", provider)).Exists(),
 			"%s", string(i.Credentials["oidc"].Config[:]))
 		assert.Contains(t, gjson.GetBytes(body, "authentication_methods").String(), "oidc", "%s", body)
 	}
@@ -1635,7 +1635,7 @@ func TestStrategy(t *testing.T) {
 				body, err := io.ReadAll(res.Body)
 				require.NoError(t, res.Body.Close())
 				require.NoError(t, err)
-				checkCredentialsLinked(res, body, i.ID, "valid")
+				checkCredentialsLinked(t, res, body, i.ID, "valid")
 			})
 		})
 
@@ -1684,7 +1684,7 @@ func TestStrategy(t *testing.T) {
 			subject = email1
 			t.Run("step=should link oidc credentials to existing identity", func(t *testing.T) {
 				res, body := loginWithOIDC(t, client, uuid.Must(uuid.FromString(linkingLoginFlow.ID)), "secondProvider")
-				checkCredentialsLinked(res, body, identityID, "secondProvider")
+				checkCredentialsLinked(t, res, body, identityID, "secondProvider")
 			})
 		})
 	})
@@ -1721,7 +1721,7 @@ func TestStrategy(t *testing.T) {
 				client := testhelpers.NewClientWithCookieJar(t, nil, nil)
 
 				res, body := loginWithOIDC(t, client, loginFlow.ID, "valid")
-				checkCredentialsLinked(res, body, i.ID, "valid")
+				checkCredentialsLinked(t, res, body, i.ID, "valid")
 			})
 		})
 
@@ -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
 }]}`),
@@ -1767,15 +1767,17 @@ func TestStrategy(t *testing.T) {
 				client := testhelpers.NewClientWithCookieJar(t, nil, nil)
 
 				res, body := loginWithOIDC(t, client, loginFlow.ID, "valid")
-				checkCredentialsLinked(res, body, i.ID, "valid")
+				checkCredentialsLinked(t, res, body, i.ID, "valid")
 			})
 
 			t.Run("step=should remove use_auto_link", func(t *testing.T) {
 				var err error
 				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.True(t, gjson.GetBytes(i.Credentials["oidc"].Config, "providers.1.use_auto_link").Bool())
+				assert.False(t, gjson.GetBytes(i.Credentials["oidc"].Config, `providers.#(provider=="valid").use_auto_link`).Bool())
+				assert.NotContains(t, i.Credentials["oidc"].Identifiers, "valid:stub")
+				assert.Contains(t, i.Credentials["oidc"].Identifiers, "valid:[email protected]")
+				assert.True(t, gjson.GetBytes(i.Credentials["oidc"].Config, `providers.#(provider=="other").use_auto_link`).Bool())
 			})
 		})
 	})

selfservice/strategy/password/validator_test.go

@@ -70,7 +70,7 @@ func TestDefaultPasswordValidationStrategy(t *testing.T) {
 			{pw: "[email protected]", id: "hello@exam", pass: false},
 			{id: "abcd", pw: "9d3c8a1b", pass: true},
 			{id: "a", pw: "kjOklafe", pass: true},
-			{id: "ab", pw: "0000ab0000", pass: true},
+			{id: "ab", pw: "0000ab0000123", pass: true},
 			// longest common substring with long password
 			{id: "d4f6090b-5a84", pw: "d4f6090b-5a84-2184-4404-8d1b-8da3eb00ebbe", pass: true},
 			{id: "asdflasdflasdf", pw: "asdflasdflpiuhefnciluaksdzuföfhg", pass: true},