Fix 5047

[lukaszgryglicki]

Fixes #5047

Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io

Assisted by OpenAI

Assisted by GitHub Copilot

Assisted by Claude

[coderabbitai]

Walkthrough

Replaces name-only refresh with refreshStoredUserIdentity that normalizes incoming email and conditionally updates user_name and/or lf_email; adds table-driven tests for the helper; and adds early-return guards in root init, company repo, email params, event logging, and signature conversion.

Changes

Stored identity refresh and input validation improvements

Layer / File(s)	Summary
Stored identity refresh implementation cla-backend-go/cmd/server.go	Adds refreshStoredUserIdentity that trims+lowercases claUser.LFEmail, detects name/email differences, and issues a single usersService.UpdateUser with date_modified and conditional user_name/lf_email updates; callers in createUserFromRequest now invoke this helper.
Stored identity refresh test coverage cla-backend-go/cmd/refresh_stored_username_test.go	New stub usersService and table-driven TestRefreshStoredUserIdentity covering nil inputs, unchanged claims, name-only/email-only/both changes, email trimming/lowercasing, case-only normalization, date_modified presence, and UpdateUser call assertions.
Defensive input validation guards cla-backend-go/cmd/root.go, cla-backend-go/company/repository.go, cla-backend-go/emails/params.go, cla-backend-go/events/service.go, cla-backend-go/signatures/converters.go	Adds early-return guard when running under go test; returns CompanyNotFound for empty companyID; guards against empty Projects in email params; skips event creation when UserID cannot be resolved; and avoids formatting empty signature dates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title "Fix 5047" is too vague and does not describe the actual changes or purpose of the PR.	Replace with a more descriptive title that explains the main change, such as "Sync CLA Manager email from User Service to users table" or "Add email synchronization for CLA Manager updates".
Out of Scope Changes check	❓ Inconclusive	While most changes appear related to email sync, some modifications like empty companyID validation and testing framework changes may warrant clarification.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code changes directly address issue #5047 by implementing email synchronization through the new refreshStoredUserIdentity function.
Description check	✅ Passed	The PR description references issue #5047 which relates to synchronizing updated email addresses to the cla-prod-users backend table. The changeset includes email synchronization logic in server.go, test coverage for refreshStoredUserIdentity, and defensive checks across multiple services.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch unicron-5047-verified-signature

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cla-backend-go/cmd/server.go`:
- Around line 916-930: Normalize and trim claUser.LFEmail before comparing and
persisting: create a normalized variable (e.g., normalizedLF :=
strings.ToLower(strings.TrimSpace(claUser.LFEmail))) and use that for the
emailChanged check (compare against
strings.ToLower(strings.TrimSpace(string(userModel.LfEmail))) or use
strings.EqualFold on trimmed values) instead of raw claUser.LFEmail, and when
populating updates["lf_email"] assign the normalizedLF so stored emails have no
surrounding whitespace and are lowercased.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a5151411-70f8-4e7a-a7a7-7c0858593049

📥 Commits

Reviewing files that changed from the base of the PR and between 6093f73 and 8ca7716.

📒 Files selected for processing (6)

• cla-backend-go/cmd/refresh_stored_username_test.go
• cla-backend-go/cmd/server.go
• cla-backend-go/company/repository.go
• cla-backend-go/emails/params.go
• cla-backend-go/events/service.go
• cla-backend-go/signatures/converters.go

[Copilot]

Pull request overview

This PR addresses issue #5047 by syncing updated CLA Manager identity claims (notably primary email) back into EasyCLA’s cla-prod-users (lf_email) record during subsequent interactions with the backend, preventing notifications from continuing to target a stale email address.

Changes:

• Update refreshStoredUserName to also sync lf_email (and add unit tests covering name/email change combinations).
• Add defensive guards to avoid noisy warnings/errors or panics when legacy/edge-case data is missing (empty signedOn, missing event user resolution, empty email template projects, empty companyID).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
cla-backend-go/signatures/converters.go	Avoid calling time formatting on empty legacy signature timestamps to prevent spurious warnings.
cla-backend-go/events/service.go	Skip event creation when UserID can’t be resolved from LfUsername, logging a warning instead.
cla-backend-go/emails/params.go	Prevent slice-out-of-range panics when email template params have no projects, returning safe defaults.
cla-backend-go/company/repository.go	Add an early guard for empty companyID before DynamoDB lookup.
cla-backend-go/cmd/server.go	Sync lf_email (and user_name) from auth claims into the stored user record on subsequent requests.
cla-backend-go/cmd/refresh_stored_username_test.go	Add tests for the updated refreshStoredUserName behavior across name/email change scenarios.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.