Fix issue caused by group-creation forwarding. (#29559)

* Set correct docs URL in duplicate report

* Fix duplicate reporting (and other possible duplicate bugs) caused by incorrect ID fixup

* Enable forwarding to primary active in all group aliases write paths

---------

Co-authored-by: Bianca Moreira <bianca.moreira@hashicorp.com>

vault/identity_store_conflicts.go

@@ -103,7 +103,7 @@ type duplicateReportingErrorResolver struct {
 	// when in case-sensitive mode.
 	//
 	// Since this is only ever called from `load*` methods on IdentityStore during
-	// an unseal we can assume that it's all from a single goroutine and does'nt
+	// an unseal we can assume that it's all from a single goroutine and doesn't
 	// need locking.
 	seenEntities     map[string][]*identity.Entity
 	seenGroups       map[string][]*identity.Group
@@ -316,8 +316,7 @@ type Warner interface {
 	Warn(msg string, args ...interface{})
 }
 
-// TODO set this correctly.
-const identityDuplicateReportUrl = "https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication"
+const identityDuplicateReportUrl = "https://developer.hashicorp.com/vault/docs/upgrading/deduplication"
 
 func (r *duplicateReportingErrorResolver) LogReport(log Warner) {
 	report := r.Report()

vault/identity_store_conflicts_test.go

@@ -65,7 +65,7 @@ func TestDuplicateReportingErrorResolver(t *testing.T) {
 	}
 
 	expectReport := `
-DUPLICATES DETECTED, see following logs for details and refer to https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication for resolution.:
+DUPLICATES DETECTED, see following logs for details and refer to https://developer.hashicorp.com/vault/docs/upgrading/deduplication for resolution.:
 1 different-case local entity alias duplicates found (potential security risk):
 local entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" canonical_id="11111111-0000-0000-0000-000000000009" force_deduplication="would merge others into this entity"
 local entity-alias "different-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000010" canonical_id="11111111-0000-0000-0000-000000000010" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000009"
@@ -99,7 +99,7 @@ group "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="
 group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would not rename"
 group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000005"
 end of group duplicates:
-end of identity duplicate report, refer to https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication for resolution.:
+end of identity duplicate report, refer to https://developer.hashicorp.com/vault/docs/upgrading/deduplication for resolution.:
 `
 
 	// Create a new errorResolver

vault/identity_store_group_aliases.go

@@ -45,8 +45,12 @@ func groupAliasPaths(i *IdentityStore) []*framework.Path {
 				},
 			},
 
-			Callbacks: map[logical.Operation]framework.OperationFunc{
-				logical.UpdateOperation: i.pathGroupAliasRegister(),
+			Operations: map[logical.Operation]framework.OperationHandler{
+				logical.UpdateOperation: &framework.PathOperation{
+					Callback:                    i.pathGroupAliasRegister(),
+					ForwardPerformanceStandby:   true,
+					ForwardPerformanceSecondary: true,
+				},
 			},
 
 			HelpSynopsis:    strings.TrimSpace(groupAliasHelp["group-alias"][0]),
@@ -85,6 +89,8 @@ func groupAliasPaths(i *IdentityStore) []*framework.Path {
 					DisplayAttrs: &framework.DisplayAttributes{
 						OperationVerb: "update",
 					},
+					ForwardPerformanceStandby:   true,
+					ForwardPerformanceSecondary: true,
 				},
 				logical.ReadOperation: &framework.PathOperation{
 					Callback: i.pathGroupAliasIDRead(),
@@ -97,6 +103,8 @@ func groupAliasPaths(i *IdentityStore) []*framework.Path {
 					DisplayAttrs: &framework.DisplayAttributes{
 						OperationVerb: "delete",
 					},
+					ForwardPerformanceStandby:   true,
+					ForwardPerformanceSecondary: true,
 				},
 			},
 

vault/identity_store_test.go

@@ -1790,12 +1790,23 @@ func TestIdentityStoreLoadingDuplicateReporting(t *testing.T) {
 	// many of these cases and seems strange to encode in a test that we want
 	// broken behavior!
 	numDupes := make(map[string]int)
+	uniqueIDs := make(map[string]struct{})
 	duplicateCountRe := regexp.MustCompile(`(\d+) (different-case( local)? entity alias|entity|group) duplicates found`)
+	// Be sure not to match attributes like alias_id= because there are dupes
+	// there. The report lines we care about always have a space before the id
+	// pair.
+	propsRe := regexp.MustCompile(`\s(id=(\S+))`)
 	for _, log := range unsealLogs {
 		if matches := duplicateCountRe.FindStringSubmatch(log); len(matches) >= 3 {
 			num, _ := strconv.Atoi(matches[1])
 			numDupes[matches[2]] = num
 		}
+		if propMatches := propsRe.FindStringSubmatch(log); len(propMatches) >= 3 {
+			artifactID := propMatches[2]
+			require.NotContains(t, uniqueIDs, artifactID,
+				"duplicate ID reported in logs for different artifacts")
+			uniqueIDs[artifactID] = struct{}{}
+		}
 	}
 	t.Logf("numDupes: %v", numDupes)
 	wantAliases, wantLocalAliases, wantEntities, wantGroups := identityStoreDuplicateReportTestWantDuplicateCounts()

vault/identity_store_util.go

@@ -2035,14 +2035,6 @@ func (i *IdentityStore) UpsertGroupInTxn(ctx context.Context, txn *memdb.Txn, gr
 		return fmt.Errorf("group is nil")
 	}
 
-	g, err := i.MemDBGroupByName(ctx, group.Name, true)
-	if err != nil {
-		return err
-	}
-	if g != nil {
-		group.ID = g.ID
-	}
-
 	// Increment the modify index of the group
 	group.ModifyIndex++