feat(users): GET /v4/users/by-identity + projectName on signatures (EasyCLA M1) - #5126
Closed
mlehotskylf wants to merge 2 commits into
Closed
feat(users): GET /v4/users/by-identity + projectName on signatures (EasyCLA M1)#5126mlehotskylf wants to merge 2 commits into
mlehotskylf wants to merge 2 commits into
Conversation
Supports the EasyCLA → LFX Self Serve M1 "My CLAs" integration (linuxfoundation/lfx-self-serve#1203) by giving Self Serve the two upstream reads it needs for complete identity resolution and project display. GET /v4/users/by-identity (swagger cla.v2.yaml; handler v2/users): - Resolves the union of EasyCLA user records matching ANY of lfUsername, verified email(s), or linked GitHub numeric ID(s); deduped by user_id; empty array on no match. - Union logic added to the v1 users.Service (GetUsersByIdentity) reusing the existing GSI-backed lookups (lf-username-index, lf-email-index, github-id-index) — no table scans. Email match is against lf_email only; a match present solely in a user's secondary user_emails is intentionally not resolved (documented; would require a scan). - The v2 handler is thin translation (v1→v2 model copy). Path is not in the cla-service public allow-list, so it lands on the secured gateway router; Self Serve remains the authorization boundary. - 6 unit tests for union / dedupe / skip-on-error / blank-key / empty-input. projectName on the signature response: - Add projectName to swagger/common/signature.yaml (v1+v2 models). - GET /v4/signatures/user/{userID} enriches each signature's ProjectName from its CLA Group (cached per distinct project ID for the request; best-effort — a lookup miss leaves it empty and never fails the listing). Lets Self Serve show the project name instead of the raw CLA Group ID. make fmt/build/test/lint all green. gen/ is gitignored (regenerate via make swagger). Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
WalkthroughAdds a v2 users-by-identity endpoint backed by normalized, deduplicated lookups across usernames, emails, and GitHub IDs. Signature responses now include resolvable CLA Group display names through cached enrichment. ChangesIdentity Lookup API
Signature Project Name Enrichment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant UsersHandler
participant UsersService
Client->>UsersHandler: GET /users/by-identity
UsersHandler->>UsersService: GetUsersByIdentity(...)
UsersService->>UsersService: Query username, emails, and GitHub IDs
UsersService-->>UsersHandler: Deduplicated user list
UsersHandler-->>Client: OK response with v2 users
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds identity-based user resolution and CLA-group display names for the Self Serve “My CLAs” integration.
Changes:
- Adds the secured
/v4/users/by-identityendpoint with GSI-backed union and deduplication. - Enriches user signatures with cached CLA-group names.
- Adds service tests and updates wiring and mocks.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
cla-backend-go/v2/users/handlers.go |
Implements v2 endpoint translation. |
cla-backend-go/v2/signatures/mock_users/mock_service.go |
Regenerates the users-service mock. |
cla-backend-go/v2/signatures/handlers.go |
Adds signature project-name enrichment. |
cla-backend-go/users/service.go |
Implements identity union logic. |
cla-backend-go/users/service_identity_test.go |
Tests identity resolution behavior. |
cla-backend-go/swagger/common/signature.yaml |
Defines projectName. |
cla-backend-go/swagger/cla.v2.yaml |
Defines the identity endpoint. |
cla-backend-go/cmd/server.go |
Registers the v2 users handler. |
cla-backend-go/cmd/refresh_stored_username_test.go |
Updates the users-service test stub. |
Files not reviewed (1)
- cla-backend-go/v2/signatures/mock_users/mock_service.go: Generated file
mlehotskylf
added a commit
to linuxfoundation/lfx-self-serve
that referenced
this pull request
Jul 28, 2026
… union) Wires the My CLAs server to the new upstream endpoint (linuxfoundation/easycla#5126): - resolveIdentity now calls GET /v4/users/by-identity with the three keys (lfUsername + verified emails + linked GitHub numeric IDs), unioning all matched EasyCLA user records — replacing the interim username-only lookup. Falls back to /v3/users/username/{userName} if by-identity 404s, so the feature still works in environments where it is not yet deployed. - toMyClaAgreement now prefers the upstream-resolved projectName, falling back to the CLA Group ID only when the name is absent. - EasyClaSignature gains an optional projectName field. Tests updated to the array-returning by-identity shape; added cases for the union, the query-param wiring, the username-only fallback, and the projectName-preference. 35 tests pass; tsc + eslint clean. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
cla-backend-go/swagger/cla.v2.yaml (1)
2775-2790: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the
githubIdarrays to avoid request amplification.Each array element triggers a separate GSI lookup in
GetUsersByIdentity(service layer). Without amaxItemscap, a caller can submit an arbitrarily large array and multiply downstream DynamoDB calls per request.♻️ Proposed fix
- name: email description: verified email address(es) to match; repeatable in: query type: array items: type: string collectionFormat: multi required: false + maxItems: 25 - name: githubId description: linked GitHub numeric ID(s) to match; repeatable in: query type: array items: type: string collectionFormat: multi required: false + maxItems: 25🤖 Prompt for 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. In `@cla-backend-go/swagger/cla.v2.yaml` around lines 2775 - 2790, Update the email and githubId array parameter definitions in the Swagger schema to include a maxItems constraint, using the service layer’s supported lookup bound for both fields. Keep their existing repeatable multi-value query behavior unchanged.cla-backend-go/v2/users/handlers.go (1)
24-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a handler-level unit test for the new Configure wiring.
Only service-level tests (
service_identity_test.go) exist; the v2→v1 mapping, empty-result path, and error responses (GetUsersByIdentityInternalServerError) in this handler aren't directly exercised.🤖 Prompt for 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. In `@cla-backend-go/v2/users/handlers.go` around lines 24 - 61, Add handler-level unit tests for Configure and its GetUsersByIdentityHandler wiring, covering v2-to-v1 user mapping, empty service results, and service or copier failures returning GetUsersByIdentityInternalServerError responses with the expected request ID and error payload.
🤖 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/swagger/common/signature.yaml`:
- Around line 82-84: Add x-omitempty: false to the projectName schema property
in signature.yaml so unresolved signatures serialize projectName as an empty
string instead of omitting the field.
In `@cla-backend-go/users/service.go`:
- Around line 208-213: The GetUsersByIdentity logging must not emit raw identity
values. Remove or replace the lfUsername field and per-item Debugf calls at the
referenced points with non-PII information such as counts or consistent hashes,
preserving useful debugging context without logging email addresses or other raw
identifiers.
- Around line 232-250: Remove the GetUserByEmail call from this email loop and
retain the GetUsersByLFEmail lookup, including its existing error handling and
add() iteration. Leave the email normalization and deduplication behavior
unchanged.
---
Nitpick comments:
In `@cla-backend-go/swagger/cla.v2.yaml`:
- Around line 2775-2790: Update the email and githubId array parameter
definitions in the Swagger schema to include a maxItems constraint, using the
service layer’s supported lookup bound for both fields. Keep their existing
repeatable multi-value query behavior unchanged.
In `@cla-backend-go/v2/users/handlers.go`:
- Around line 24-61: Add handler-level unit tests for Configure and its
GetUsersByIdentityHandler wiring, covering v2-to-v1 user mapping, empty service
results, and service or copier failures returning
GetUsersByIdentityInternalServerError responses with the expected request ID and
error payload.
🪄 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: bb9da78f-0141-4a3b-9903-d3798695217d
📒 Files selected for processing (9)
cla-backend-go/cmd/refresh_stored_username_test.gocla-backend-go/cmd/server.gocla-backend-go/swagger/cla.v2.yamlcla-backend-go/swagger/common/signature.yamlcla-backend-go/users/service.gocla-backend-go/users/service_identity_test.gocla-backend-go/v2/signatures/handlers.gocla-backend-go/v2/signatures/mock_users/mock_service.gocla-backend-go/v2/users/handlers.go
mlehotskylf
added a commit
to linuxfoundation/lfx-self-serve
that referenced
this pull request
Jul 28, 2026
… union) Wires the My CLAs server to the new upstream endpoint (linuxfoundation/easycla#5126): - resolveIdentity now calls GET /v4/users/by-identity with the three keys (lfUsername + verified emails + linked GitHub numeric IDs), unioning all matched EasyCLA user records — replacing the interim username-only lookup. Falls back to /v3/users/username/{userName} if by-identity 404s, so the feature still works in environments where it is not yet deployed. - toMyClaAgreement now prefers the upstream-resolved projectName, falling back to the CLA Group ID only when the name is absent. - EasyClaSignature gains an optional projectName field. Tests updated to the array-returning by-identity shape; added cases for the union, the query-param wiring, the username-only fallback, and the projectName-preference. 35 tests pass; tsc + eslint clean. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Address review comments from copilot[bot], coderabbitai: - users/service.go: drop the redundant GetUserByEmail call in GetUsersByIdentity — it queries the same lf-email-index as GetUsersByLFEmail (identical key/index/projection, first-result-only), so calling both doubled DynamoDB reads per email for zero added coverage (per copilot[bot], coderabbitai) - users/service.go: trim whitespace-only lfUsername before the GSI lookup, matching the existing email/githubID handling (per copilot[bot]) - users/service.go: keep PII (LF username, raw emails) out of the persistent log fields and per-key miss logs; record counts only (per coderabbitai) - users/service_identity_test.go: update expectations for the removed GetUserByEmail call and cover whitespace-only lfUsername skipping Resolves 3 review threads. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cla-backend-go/users/service.go (1)
233-241: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winDeduplicate normalized keys before querying the repository.
adddeduplicates returned users, but duplicate or normalization-equivalent emails and GitHub IDs still trigger repeated sequential repository reads. Deduplicate normalized keys before each loop to avoid unnecessary DynamoDB I/O and latency.Proposed fix
+ seenEmails := make(map[string]struct{}, len(emails)) for _, email := range emails { email = strings.ToLower(strings.TrimSpace(email)) if email == "" { continue } + if _, seen := seenEmails[email]; seen { + continue + } + seenEmails[email] = struct{}{} + seenGitHubIDs := make(map[string]struct{}, len(githubIDs)) for _, githubID := range githubIDs { githubID = strings.TrimSpace(githubID) if githubID == "" { continue } + if _, seen := seenGitHubIDs[githubID]; seen { + continue + } + seenGitHubIDs[githubID] = struct{}{}Also applies to: 249-255
🤖 Prompt for 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. In `@cla-backend-go/users/service.go` around lines 233 - 241, Deduplicate the normalized email keys before the email loop in the user lookup flow, and likewise deduplicate normalized GitHub ID keys before the corresponding loop. Update the logic around GetUsersByLFEmail and the analogous GitHub-ID repository query so each unique key is queried at most once while preserving the existing user aggregation behavior.
🤖 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.
Outside diff comments:
In `@cla-backend-go/users/service.go`:
- Around line 233-241: Deduplicate the normalized email keys before the email
loop in the user lookup flow, and likewise deduplicate normalized GitHub ID keys
before the corresponding loop. Update the logic around GetUsersByLFEmail and the
analogous GitHub-ID repository query so each unique key is queried at most once
while preserving the existing user aggregation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7411fe78-89eb-4275-88fa-5df2c9244f49
📒 Files selected for processing (2)
cla-backend-go/users/service.gocla-backend-go/users/service_identity_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cla-backend-go/users/service_identity_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- cla-backend-go/v2/signatures/mock_users/mock_service.go: Generated file
Comments suppressed due to low confidence (7)
cla-backend-go/users/service.go:206
- The implementation below treats every repository error as a normal miss.
GetUserByLFUserNameonly returns errors for expression/DynamoDB/unmarshal failures, while the email and GitHub methods mix not-found errors with operational failures. A throttled or unavailable GSI can therefore produce a successful but incomplete/empty response, and the handler's 500 path is effectively unreachable. Skip only recognized not-found results and propagate operational failures so callers can retry instead of silently losing CLA history.
// A lookup that fails or finds nothing for one key is logged and skipped, not fatal: this is a
// "match any" resolver, so one missing key must not fail the others. Returns an empty (non-nil)
// slice when nothing matches.
cla-backend-go/v2/users/handlers.go:33
- This field persists the supplied LF username in every handler log even though
users/service.go:208-209explicitly classifies identity values as PII and avoids logging them. Record only whether the key was supplied, as is already done for the array parameters.
"lfUsername": utils.StringValue(params.LfUsername),
cla-backend-go/swagger/cla.v2.yaml:2781
- These identity values are placed in the query string, while
cmd/server.go:908logsr.URL.String()for every request. Verified emails, LF usernames, and GitHub IDs will therefore be persisted in application logs regardless of the handler's own structured fields. Redact these query parameters in request logging (and verify gateway access-log redaction), or use a request body for this internal lookup.
- name: email
description: verified email address(es) to match; repeatable
in: query
type: array
items:
type: string
collectionFormat: multi
cla-backend-go/users/service.go:240
- The linked M1 contract and FR-005 require resolving verified emails, including matches in
user_emails, but this deliberately queries only primarylf_email. A user whose verified address exists only in the secondary list will miss that EasyCLA record and its CLA history, so the integration cannot meet its stated completeness criterion. Add an indexed secondary-email lookup, or update the linked contract/acceptance criteria to explicitly accept this limitation before merging.
// lf-email-index is keyed on lf_email; GetUsersByLFEmail queries that GSI (no scan) and
// returns every match, so it fully covers GetUserByEmail (same query, first-result-only).
if us, err := s.repo.GetUsersByLFEmail(email); err != nil {
cla-backend-go/v2/signatures/handlers.go:711
claGroupService.GetCLAGroupByIDalways requestsLoadRepoDetails(project/service/service.go:78-105), which launches GitHub and Gerrit repository lookups in addition to DynamoDB (project/repository/repository.go:875-905). Doing that once per distinct signature project is unnecessary for a name-only enrichment and can substantially increase latency and failure exposure. Use the already-injected repository withDontLoadRepoDetails.
if claGroupModel, cgErr := claGroupService.GetCLAGroupByID(ctx, sig.ProjectID); cgErr != nil || claGroupModel == nil {
cla-backend-go/swagger/cla.v2.yaml:2781
- This repeatable array has no
maxItems, so one authenticated request can trigger an unbounded number of sequential DynamoDB email-index queries inGetUsersByIdentity. Add a small contract-level limit (and ideally deduplicate normalized values before lookup) to bound Lambda duration and read cost.
type: array
items:
type: string
collectionFormat: multi
cla-backend-go/swagger/cla.v2.yaml:2789
- This array is also unbounded, and each value causes a sequential GitHub-ID GSI query. Add
maxItemsplus a numeric item pattern/length bound so malformed or oversized requests are rejected before consuming arbitrary DynamoDB reads.
type: array
items:
type: string
collectionFormat: multi
Collaborator
Author
Review Feedback AddressedCommit: e804728 Changes Made
Declined
Threads Resolved5 of 5 unresolved threads addressed. |
mlehotskylf
added a commit
to linuxfoundation/lfx-self-serve
that referenced
this pull request
Jul 29, 2026
… union) Wires the My CLAs server to the new upstream endpoint (linuxfoundation/easycla#5126): - resolveIdentity now calls GET /v4/users/by-identity with the three keys (lfUsername + verified emails + linked GitHub numeric IDs), unioning all matched EasyCLA user records — replacing the interim username-only lookup. Falls back to /v3/users/username/{userName} if by-identity 404s, so the feature still works in environments where it is not yet deployed. - toMyClaAgreement now prefers the upstream-resolved projectName, falling back to the CLA Group ID only when the name is absent. - EasyClaSignature gains an optional projectName field. Tests updated to the array-returning by-identity shape; added cases for the union, the query-param wiring, the username-only fallback, and the projectName-preference. 35 tests pass; tsc + eslint clean. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Collaborator
Author
|
We no longer need this as @lukaszgryglicki implemented this endpoint and this was only for testing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two upstream reads that the EasyCLA → LFX Self Serve Milestone-1 "My CLAs" integration needs (Self Serve PR: linuxfoundation/lfx-self-serve#1203). Read-only, no schema changes, no writes.
1.
GET /v4/users/by-identity?lfUsername=…&email=…&githubId=…Resolves the union of EasyCLA user records matching ANY supplied identity key — LF username, verified email(s), or linked GitHub numeric ID(s) — deduped by
user_id, empty array on no match. This lets Self Serve map one logged-in identity to its one-or-more EasyCLA records (including pre-LF-login GitHub-derived history).users.Service(GetUsersByIdentity), reusing existing GSI-backed lookups (lf-username-index,lf-email-index,github-id-index) — no table scans.v2/usershandler is thin request/response translation (v1→v2 model copy), mirroringv2/current_user.cla-servicepublic allow-list (lfx-gateway/dynamic/services/cla-service.yaml), so it lands on the secured router. Self Serve remains the authorization boundary.Known limitation (documented): email matching is against the primary
lf_emailGSI only. A verified email present solely in a user's secondaryuser_emailslist is not resolved — that would require a table scan, which is unsuitable per-request. Username + GitHub ID + primary email cover the common cases.2.
projectNameon the signature responseGET /v4/signatures/user/{userID}now returns each signature's CLA-Group display name (previously only the CLA-Group ID /projectIDwas available). AddedprojectNametoswagger/common/signature.yaml; the v2 handler resolves it from the CLA Group, cached per distinct project ID per request, best-effort (a miss leaves it empty and never fails the listing).Testing
make fmt && go build ./... && go test (users, v2/signatures, cmd) && make lint— all green.gen/is gitignored; regenerate withmake swagger.Notes
GET /v4/signatures/user/{userID}applies here too — treat as an internal/service API; the caller is the authorization boundary.🤖 Generated with Claude Code