Merge pull request #132 from linuxfoundation/lfxv2-2662-email-sync-edge-cases

Author: emsearcy

cmd/lfx-v1-sync-helper/backfill_email_profile.go

@@ -257,31 +257,71 @@ func backfillEmailsForUser(ctx context.Context, auth0UserID, username string, dr
 		return fmt.Errorf("parsing email SFIDs for %s: %w", userSfid, err)
 	}
 
+	// Single pass: collect candidate (non-primary, active, verified) emails to
+	// link, while also counting "qualifying" emails (active and either
+	// verified or primary) across ALL of the user's alternate email rows. If
+	// only one email qualifies, that row is a lone alternate_email__c acting
+	// as a de-facto primary — v1 lazy-sync may not have created/synced the
+	// primary row yet — so it should not be linked as a secondary identity,
+	// even if it was collected as a link candidate (see LFXV2-2662; this
+	// mirrors auth0-db-sync.js's heuristic).
+	type emailCandidate struct {
+		emailSfid string
+		email     string
+	}
+	var candidates []emailCandidate
+	qualifying := 0
+	sawPrimary := false
 	for _, emailSfid := range emailSfids {
 		email, isPrimary, isVerified, isActive, err := getAlternateEmailDetailsFn(ctx, emailSfid)
 		if err != nil {
-			logger.With("error", err, "email_sfid", emailSfid, "auth0_user_id", auth0UserID).
-				Warn("failed to get alternate email details, skipping")
-			result.emailsSkipped++
-			continue
+			// A read failure makes the qualifying count unreliable, and
+			// guessing either way (sole vs. non-sole) risks a wrong link
+			// decision, so abort rather than silently proceed. This user
+			// will be retried on the next run (the cursor doesn't advance
+			// past them); a persistently-bad row requires a manual data fix
+			// to unblock, which is preferable to silently mislinking.
+			return fmt.Errorf("getting alternate email details for %s (auth0 user %s): %w", emailSfid, auth0UserID, err)
+		}
+		if isActive && (isVerified || isPrimary) {
+			qualifying++
+			if isPrimary {
+				sawPrimary = true
+			}
 		}
 		if isPrimary || !isActive || !isVerified || email == "" {
 			result.emailsSkipped++
 			continue
 		}
+		candidates = append(candidates, emailCandidate{emailSfid: emailSfid, email: email})
+	}
 
+	if qualifying <= 1 {
+		logger.With("user_sfid", userSfid, "auth0_user_id", auth0UserID).
+			Debug("sole qualifying alternate email, treating as de-facto primary, skipping link candidates")
+		result.emailsSkipped += len(candidates)
+		return nil
+	}
+	if !sawPrimary {
+		// More than one row qualifies but none is flagged primary: which one
+		// is the de-facto primary is genuinely ambiguous, so abort rather
+		// than linking all of them as secondary identities.
+		return fmt.Errorf("user %s (auth0 user %s) has %d qualifying alternate emails and none is flagged primary; cannot determine de-facto primary", userSfid, auth0UserID, qualifying)
+	}
+
+	for _, c := range candidates {
 		if dryRun {
 			logger.With(
 				"auth0_user_id", auth0UserID,
-				"email", email,
-				"email_sfid", emailSfid,
+				"email", c.email,
+				"email_sfid", c.emailSfid,
 			).Info("[dry-run] would link alternate email to Auth0 user")
 			result.emailsLinked++
 			continue
 		}
 
-		if err := linkEmailIdentityFn(ctx, auth0UserID, email); err != nil {
-			return fmt.Errorf("linking email %s: %w", email, err)
+		if err := linkEmailIdentityFn(ctx, auth0UserID, c.email); err != nil {
+			return fmt.Errorf("linking email %s: %w", c.email, err)
 		}
 		result.emailsLinked++
 	}
@@ -474,11 +514,36 @@ func syncSingleUser(ctx context.Context, username string, dryRun bool) error {
 		return fmt.Errorf("parsing email SFIDs: %w", err)
 	}
 
+	// Single pass: collect candidate (non-primary, active, verified) emails to
+	// link, while also counting "qualifying" emails (active and either
+	// verified or primary) across ALL of the user's alternate email rows. If
+	// only one email qualifies, that row is a lone alternate_email__c acting
+	// as a de-facto primary — v1 lazy-sync may not have created/synced the
+	// primary row yet — so it should not be linked as a secondary identity,
+	// even if it was collected as a link candidate (see LFXV2-2662; this
+	// mirrors auth0-db-sync.js's heuristic).
+	type emailCandidate struct {
+		emailSfid string
+		email     string
+	}
+	var candidates []emailCandidate
+	qualifying := 0
+	sawPrimary := false
 	for _, emailSfid := range emailSfids {
 		email, isPrimary, isVerified, isActive, err := getAlternateEmailDetailsFn(ctx, emailSfid)
 		if err != nil {
-			logger.With("error", err, "email_sfid", emailSfid).Warn("failed to get email details, skipping")
-			continue
+			// A read failure makes the qualifying count unreliable, and
+			// guessing either way (sole vs. non-sole) risks a wrong link
+			// decision, so abort rather than silently proceed. Re-run
+			// --sync-user for this user once the underlying read problem
+			// (or bad data) is resolved.
+			return fmt.Errorf("getting alternate email details for %s (auth0 user %s): %w", emailSfid, auth0UserID, err)
+		}
+		if isActive && (isVerified || isPrimary) {
+			qualifying++
+			if isPrimary {
+				sawPrimary = true
+			}
 		}
 		if isPrimary || !isActive {
 			continue
@@ -491,18 +556,32 @@ func syncSingleUser(ctx context.Context, username string, dryRun bool) error {
 		if email == "" {
 			continue
 		}
+		candidates = append(candidates, emailCandidate{emailSfid: emailSfid, email: email})
+	}
+
+	if qualifying <= 1 {
+		logger.With("user_sfid", userSfid, "auth0_user_id", auth0UserID).
+			Debug("sole qualifying alternate email, treating as de-facto primary, skipping link candidates")
+		candidates = nil
+	} else if !sawPrimary {
+		// More than one row qualifies but none is flagged primary: which one
+		// is the de-facto primary is genuinely ambiguous, so abort rather
+		// than linking all of them as secondary identities.
+		return fmt.Errorf("user %s (auth0 user %s) has %d qualifying alternate emails and none is flagged primary; cannot determine de-facto primary", userSfid, auth0UserID, qualifying)
+	}
 
+	for _, c := range candidates {
 		if dryRun {
-			logger.With("auth0_user_id", auth0UserID, "email", email).
+			logger.With("auth0_user_id", auth0UserID, "email", c.email).
 				Info("[dry-run] would link alternate email to Auth0 user")
 			continue
 		}
 
-		if err := linkEmailIdentityFn(ctx, auth0UserID, email); err != nil {
-			logger.With("error", err, "auth0_user_id", auth0UserID, "email", email).
+		if err := linkEmailIdentityFn(ctx, auth0UserID, c.email); err != nil {
+			logger.With("error", err, "auth0_user_id", auth0UserID, "email", c.email).
 				Warn("failed to link email, skipping")
 		} else {
-			logger.With("auth0_user_id", auth0UserID, "email", email).
+			logger.With("auth0_user_id", auth0UserID, "email", c.email).
 				Info("linked email identity")
 		}
 	}