Do not issue duplicated changes in the same LDAP "transaction"

Prevents duplicate insertion of storageEncryptedSecretKey when
requesting an account recovery with opportunistic encryption enabled.

Add an integration test to cover this scenario.

backend/ldap/tx.go

@@ -43,7 +43,7 @@ type ldapTX struct {
 
 	cache   map[string][]string
 	newDNs  map[string]struct{} // nolint (it's plural DN, not DNS)
-	changes []ldapAttr
+	changes []*ldapAttr
 }
 
 func newLDAPTX(conn ldapConn) *ldapTX {
@@ -86,7 +86,19 @@ func (tx *ldapTX) setAttr(dn, attr string, values ...string) {
 	if dn == "" {
 		panic("empty dn in setAttr!")
 	}
-	tx.changes = append(tx.changes, ldapAttr{dn: dn, attr: attr, values: values})
+	// Reuse previous change, if any. Prevents value duplication
+	// in the same ModifyRequest.
+	found := false
+	for _, chg := range tx.changes {
+		if chg.dn == dn && chg.attr == attr {
+			chg.values = values
+			found = true
+			break
+		}
+	}
+	if !found {
+		tx.changes = append(tx.changes, &ldapAttr{dn: dn, attr: attr, values: values})
+	}
 }
 
 // Commit the transaction, sending all changes to the LDAP server.
@@ -154,7 +166,7 @@ func (tx *ldapTX) aggregateChanges(ctx context.Context) (map[string]*ldap.AddReq
 	return adds, mods, dns
 }
 
-func (tx *ldapTX) updateModifyRequest(ctx context.Context, mr *ldap.ModifyRequest, attr ldapAttr) {
+func (tx *ldapTX) updateModifyRequest(ctx context.Context, mr *ldap.ModifyRequest, attr *ldapAttr) {
 	old, ok := tx.cache[cacheKey(attr.dn, attr.attr)]
 
 	// Pessimistic approach: if we haven't seen this attribute

integrationtest/account_mgmt_test.go

@@ -131,29 +131,39 @@ func runChangeUserPasswordTest(t *testing.T, username string, cfg as.Config) *as
 }
 
 func TestIntegration_AccountRecovery(t *testing.T) {
-	runAccountRecoveryTest(t, "uno@investici.org", false)
+	runAccountRecoveryTest(t, "uno@investici.org", false, false)
 }
 
 func TestIntegration_AccountRecovery_WithEncryptionKeys(t *testing.T) {
-	user := runAccountRecoveryTest(t, "due@investici.org", false)
+	user := runAccountRecoveryTest(t, "due@investici.org", false, false)
 	if !user.HasEncryptionKeys {
-		t.Fatalf("encryption keys not enabled after account recovery")
+		t.Fatal("encryption keys not enabled after account recovery")
+	}
+}
+
+func TestIntegration_AccountRecovery_WithOpportunisticEncryption(t *testing.T) {
+	user := runAccountRecoveryTest(t, "uno@investici.org", false, true)
+	if !user.HasEncryptionKeys {
+		t.Fatal("encryption keys not enabled after account recovery")
 	}
 }
 
 func TestIntegration_AccountRecovery_WithCache(t *testing.T) {
-	runAccountRecoveryTest(t, "uno@investici.org", true)
+	runAccountRecoveryTest(t, "uno@investici.org", true, false)
 }
 
 func TestIntegration_AccountRecovery_WithEncryptionKeysAndCache(t *testing.T) {
-	user := runAccountRecoveryTest(t, "due@investici.org", true)
+	user := runAccountRecoveryTest(t, "due@investici.org", true, false)
 	if !user.HasEncryptionKeys {
-		t.Fatalf("encryption keys not enabled after account recovery")
+		t.Fatal("encryption keys not enabled after account recovery")
 	}
 }
 
-func runAccountRecoveryTest(t *testing.T, username string, enableCache bool) *as.RawUser {
-	stop, be, c := startService(t)
+func runAccountRecoveryTest(t *testing.T, username string, enableCache, enableOpportunisticEncryption bool) *as.RawUser {
+	cfg := as.Config{
+		EnableOpportunisticEncryption: enableOpportunisticEncryption,
+	}
+	stop, be, c := startServiceWithConfig(t, cfg)
 	defer stop()
 
 	hint := "secret code?"