feat(permissions): fix user metadata lookup flow for email-based user… #118
Conversation
… management Update project permission system to use correct NATS endpoints for user information retrieval: - Add USER_METADATA_READ NATS subject for lfx.auth-service.user_metadata.read - Fix getUserInfo to use email_to_username result for user metadata lookup - Maintain email_to_sub for backend storage consistency (auditors/writers) - Handle proper response format from user metadata service - Map picture field to avatar and construct names from metadata 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Jordan Evans <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughController no longer resolves emails; it forwards the original username/email to the service. Service now normalizes emails to backend identifier (sub), updates permission operations to use sub, adds email→sub/username helpers, adjusts NATS subjects and metadata retrieval. Shared NATS enum gains USER_METADATA_READ. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client
participant C as ProjectController
participant S as ProjectService
participant N as NATS Bus
participant Auth as Auth Service
participant Store as Project Store
UI->>C: add/update/remove permissions(usernameOrEmail, role, manualUserInfo?)
C->>S: updateProjectPermissions(req, projectId, op, usernameOrEmail, role, manualUserInfo?)
rect rgba(200,220,255,0.25)
note over S: Normalize identifier
alt Input is email
S->>N: request resolveEmailToSub(email)
N->>Auth: email → sub lookup
Auth-->>N: sub (or timeout/error)
N-->>S: sub
S->>N: USER_METADATA_READ (by sub)
N-->>S: user metadata
else Username provided
S->>N: USER_METADATA_READ (by username)
N-->>S: user metadata
end
end
note over S: Build userInfo (username := sub if email), merge manualUserInfo
alt op == remove
S->>Store: remove permission entries by backendIdentifier (sub)
else op == add/update
S->>Store: upsert writer/auditor entries by backendIdentifier (sub)
end
S-->>C: result (users, is_email, identifiers)
C-->>UI: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)apps/lfx-one/src/server/services/project.service.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Comment |
github-actions
bot
requested review from
andrest50,
asithade,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
Purpose: Align project permission user resolution with new auth service metadata endpoints, introducing USER_METADATA_READ, separating email-to-sub (backend identifier) from email-to-username (display), and updating permission update flow to consume user metadata (name/avatar) correctly.
- Add new NATS subject USER_METADATA_READ and switch metadata lookup to it
- Introduce separate resolveEmailToSub and resolveEmailToUsername flows and adjust permission update to store sub as backend identifier
- Refactor getUserInfo to use user metadata service and map avatar from picture
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/shared/src/enums/nats.enum.ts | Added USER_METADATA_READ NATS subject constant to support new metadata lookup |
| apps/lfx-one/src/server/services/project.service.ts | Refactored permission update flow, added email-to-sub + email-to-username resolution, switched to metadata read subject, updated user info shaping |
| apps/lfx-one/src/server/controllers/project.controller.ts | Simplified controller by delegating email/username resolution to service and adjusted logging/parameter passing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Run yarn format to ensure consistent code style across the project. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Jordan Evans <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/server/services/project.service.ts (1)
181-216: Prevent legacy username duplicates when normalizing tosub.Line 181 only filters by
backendIdentifier. Any existing writer/auditor stored under the old directory username (e.g.,jdoe) will survive, so an add/update via email now leaves the legacy entry in place and appends a second record keyed bysub. That regresses the permissions UI with duplicate rows and breaks removal flows that rely on a single entry. Please strip out the prior identifier as well (resolveemail_to_usernameonce and filter against both values) before pushing the new object.- // Use backendIdentifier (sub) for comparison to ensure proper removal - updatedSettings.writers = updatedSettings.writers.filter((u) => u.username !== backendIdentifier); - updatedSettings.auditors = updatedSettings.auditors.filter((u) => u.username !== backendIdentifier); + const identifiersToRemove = new Set<string>([backendIdentifier]); + const rawIdentifier = usernameOrEmail.trim(); + if (rawIdentifier) { + identifiersToRemove.add(rawIdentifier); + } + if (usernameOrEmail.includes('@')) { + const directoryUsername = await this.resolveEmailToUsername(req, usernameOrEmail); + identifiersToRemove.add(directoryUsername); + } + + updatedSettings.writers = updatedSettings.writers.filter((user) => !identifiersToRemove.has(user.username)); + updatedSettings.auditors = updatedSettings.auditors.filter((user) => !identifiersToRemove.has(user.username));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/lfx-one/src/server/controllers/project.controller.ts(1 hunks)apps/lfx-one/src/server/services/project.service.ts(8 hunks)packages/shared/src/enums/nats.enum.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/shared/src/enums/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all enums in the shared package at packages/shared/src/enums
Files:
packages/shared/src/enums/nats.enum.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
packages/shared/src/enums/nats.enum.tsapps/lfx-one/src/server/controllers/project.controller.tsapps/lfx-one/src/server/services/project.service.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
packages/shared/src/enums/nats.enum.tsapps/lfx-one/src/server/controllers/project.controller.tsapps/lfx-one/src/server/services/project.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/enums/nats.enum.tsapps/lfx-one/src/server/controllers/project.controller.tsapps/lfx-one/src/server/services/project.service.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/server/services/project.service.ts (3)
packages/shared/src/interfaces/project.interface.ts (1)
ProjectSettings(139-152)packages/shared/src/constants/api.constants.ts (1)
NATS_CONFIG(28-43)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
🪛 GitHub Actions: Quality Checks
apps/lfx-one/src/server/controllers/project.controller.ts
[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.
apps/lfx-one/src/server/services/project.service.ts
[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
… management
Update project permission system to use correct NATS endpoints for user information retrieval:
🤖 Generated with Claude Code