Conversation
Implements the org-role-grants BFF rewrite for LFXV2-3029: a bidirectional connected-component walk discovers every organization reachable from a direct grant (ascend parent_uid, descend parent_b2b_org_uid), and the platform authorizer (not the walk) classifies each discovered candidate as writer/auditor/denied. Classification failure surfaces as an explicit degraded flag rather than silently reclassifying an org as denied. Role precedence is now authority-first (direct-writer > inherited-writer > direct-auditor > inherited-auditor), so a mixed grant resolves to editor. Every organization-edit gate (profile, key contacts, membership detail, projects, committee/board members, access management) now checks the widened editor set instead of the direct-only writer set. The org selector's role badge gets settled "(Original)"/"(Inherited)" labels as a single per-caller decision, with corrected icon/tooltip copy now that an inherited editor grant confers real edit authority. Jira: LFXV2-3029 Link: https://linuxfoundation.atlassian.net/browse/LFXV2-3029 Signed-off-by: Prabodh Chaudhari <pchaudhari@linuxfoundation.org> Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds hierarchy-aware Org Lens access discovery, enabling editor permissions to cascade across parent-child organization relationships.
Changes:
- Adds bidirectional hierarchy traversal and authoritative role classification.
- Extends backend and UI edit gates to inherited writers.
- Adds degraded-state contracts and updates selector labels, icons, and tooltips.
Review findings:
- Critical (2 votes):
org-lens-read-access.helper.ts:74rejects confirmed grants whenever any result is degraded; confirmed grants should remain authoritative. - Critical (2 votes):
org-lens-access.service.ts:322returns 403 instead of 503 when degraded classification omits a potential inherited editor. - Moderate (1 vote): The Angular role-grants service ignores
response.degraded, presenting incomplete results as verified denials. - Moderate (2 votes): Role classification can generate roughly 5,000 concurrent upstream requests; add global bounds or bounded batching.
- Moderate (1 vote): Missing direct organization documents do not mark results degraded despite preventing complete traversal.
- Moderate (1 vote): The new shared degraded flag is not propagated into client navigation and failure states.
- Nit (2 votes): Add unit coverage for traversal, mixed grants, cycles/caps, and strict-classification failures.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/org-selector.interface.ts |
Adds hierarchy metadata and degraded-state contracts. |
packages/shared/src/interfaces/access-check.interface.ts |
Adds b2b_org access-check support. |
apps/lfx-one/src/server/services/org-role-grants.service.ts |
Implements hierarchy traversal, classification, precedence, and caching. |
apps/lfx-one/src/server/services/org-lens-access.service.ts |
Widens server-side editor authorization. |
apps/lfx-one/src/server/helpers/org-lens-read-access.helper.ts |
Handles degraded grant resolution. |
apps/lfx-one/src/app/shared/services/org-role-grants.service.ts |
Exposes the combined editor signal. |
apps/lfx-one/src/app/shared/components/org-selector/org-selector.component.ts |
Updates role precedence, labels, icons, and tooltips. |
apps/lfx-one/src/app/modules/dashboards/org/org-projects/org-projects.component.ts |
Widens project-management access. |
apps/lfx-one/src/app/modules/dashboards/org/org-profile/org-profile.component.ts |
Widens profile-editing access. |
apps/lfx-one/src/app/modules/dashboards/org/org-profile/org-profile.component.spec.ts |
Updates the role-grants test mock. |
apps/lfx-one/src/app/modules/dashboards/org/org-people/components/key-contacts/key-contacts.component.ts |
Widens key-contact editing access. |
apps/lfx-one/src/app/modules/dashboards/org/org-people/components/committee-members/committee-members.component.ts |
Widens committee-seat editing access. |
apps/lfx-one/src/app/modules/dashboards/org/org-people/components/board-members/board-members.component.ts |
Widens board-seat editing access. |
apps/lfx-one/src/app/modules/dashboards/org/org-membership-detail/org-membership-detail.component.ts |
Widens membership editing access. |
Suppressed comments (3)
apps/lfx-one/src/server/services/org-role-grants.service.ts:573
- Cached parent documents are filtered out of
parentUidsToFetch, but only fetched parents are added tonextFrontier. SincedocByUidstarts with every directly granted org and is shared across roots, a walk from a child stops when its parent is already cached, so the connected component and nearest provenance can be incomplete. Build the traversal parent set independently from the subset that still needs fetching, and enqueue both cached and fetched parents.
const parentUidsToFetch = [...new Set(frontier.map((uid) => docByUid.get(uid)?.parent_uid).filter((uid): uid is string => !!uid && !docByUid.has(uid)))];
apps/lfx-one/src/server/services/org-role-grants.service.ts:608
- Excluding every direct UID from
candidatesmakes the new mixed-grant precedence unreachable. If org B is a direct auditor but is connected to a writer grant on org A, B must still be authoritatively classified soinherited-writercan outrankdirect-auditor; this condition leaves B as a viewer. Exclude only the current walk's own root, not direct orgs reached from another root.
if (!directUids.has(uid)) {
apps/lfx-one/src/server/services/org-role-grants.service.ts:659
- The current canonical
lfx-v2-helmmodel still definesb2b_org.writeras direct user/owner/global-admin only and explicitly states that writer does not cascade; onlyauditortraverses parent/child. Consequently these writer checks cannot populatecascadingWriters, so the editor-access feature remains inactive unless the model change is landed and deployed with this PR. Please make that cross-repo dependency explicit/atomic, or defer the widened editor gates until the authorizer contract supports it.
requests.push({ resource: 'b2b_org', id: uid, access: 'writer' });
requests.push({ resource: 'b2b_org', id: uid, access: 'auditor' });
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+74
to
+77
| // `getAccessAwareOrgs` degrades to an empty/partial grant map on upstream failure or | ||
| // unverifiable roll-up classification instead of throwing, so an unverified lookup is | ||
| // indistinguishable from "no grants" unless one of these flags is checked. | ||
| degraded = upstreamFailed || classificationDegraded; |
Comment on lines
322
to
+323
| const grants = await this.roleGrants.getRoleGrants(req, username); | ||
| isWriter = grants.writers.includes(orgUid); | ||
| isWriter = OrgRoleGrantsService.hasEditorAccess(grants, orgUid); |
Comment on lines
+40
to
+47
| /** | ||
| * LFXV2-3029 — "editor from any source": `writerSet` (direct) union `inheritedWriterSet` | ||
| * (roll-up-derived). Every organization-edit capability gate should read this, not the | ||
| * direct-only `writerSet` — every edit surface a direct editor can reach is meant to also open | ||
| * for a roll-up editor. `writerSet` itself is kept direct-only for callers that still need that | ||
| * narrower, direct-only answer specifically. | ||
| */ | ||
| public readonly editorSet: Signal<Set<string>> = computed(() => new Set([...this.writerSetInternal(), ...this.inheritedWriterSetInternal()])); |
| const resolved = this.buildResolvedMap(directWriters, directAuditors, classified); | ||
| const orgDocByUid = this.mergeOrgDocs(directOrgDocs, classified, walk.docByUid); | ||
|
|
||
| return { resolved, orgDocByUid, upstreamFailed: false, loadedAt, username, isStaff, degraded: classificationDegraded || walk.truncated }; |
Comment on lines
+655
to
+659
| const uids = [...candidates.keys()]; | ||
| const requests: AccessCheckRequest[] = []; | ||
| for (const uid of uids) { | ||
| requests.push({ resource: 'b2b_org', id: uid, access: 'writer' }); | ||
| requests.push({ resource: 'b2b_org', id: uid, access: 'auditor' }); |
Comment on lines
+110
to
+111
| /** LFXV2-3029 — true when authoritative classification could not be completed for one or more discovered organizations, so the list is a lower bound rather than the caller's full resolved set. Lets the client say the lookup broke rather than that the caller has no organizations. Always present. */ | ||
| degraded: boolean; |
Comment on lines
+548
to
+552
| private async expandConnectedComponents( | ||
| req: Request, | ||
| directUids: Set<string>, | ||
| directOrgDocs: Map<string, B2bOrgIndexedDoc> | ||
| ): Promise<{ candidates: Map<string, { rootUid: string; rootName: string }>; docByUid: Map<string, B2bOrgIndexedDoc>; truncated: boolean }> { |
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.
Summary
Implements the org-role-grants BFF rewrite for LFXV2-3029: a bidirectional connected-component walk discovers every organization reachable from a direct grant (ascend
parent_uid, descendparent_b2b_org_uid), and the platform authorizer (not the walk) classifies each discovered candidate as writer/auditor/denied. Classification failure surfaces as an explicitdegradedflag rather than silently reclassifying an org as denied.Role precedence is now authority-first (direct-writer > inherited-writer > direct-auditor > inherited-auditor), so a mixed grant resolves to editor.
Changes
org-role-grants.service.ts): bidirectional connected-component walk, authoritative classification via the strict access-check,degradedfailure semantics, authority-first precedence merge, deterministic nearest-provenance selection, and a statichasEditorAccesshelper.org-lens-access.service.tsandorg-lens-read-access.helper.tsroute through the widened editor set / degraded signal; 6 UI edit gates (profile, key-contacts, membership-detail, projects, committee-members, board-members) switch from the direct-only writer set to the new editor set.org-selector.component.ts): authority-first persona precedence, settled "(Original)"/"(Inherited)" copy as a single per-caller suffix decision, corrected icon/tooltip for inherited editors.degradedfield onRoleGrantsResponse/AccessAwareOrgsResult/cache entry;parent_uid/parent_detailon the indexed org doc;b2b_orgadded to the access-check resource union.Testing
yarn build,yarn test:server,yarn test:app,yarn lint:check— all clean.parentandchildtuples exist (already published by member-service today forauditor).Related
Jira: LFXV2-3029
Link: https://linuxfoundation.atlassian.net/browse/LFXV2-3029
Status
Draft — remaining e2e coverage (mixed-grant/revocation/reparenting scenarios, label matrix) and the p95 performance baseline are still open and tracked separately.
Made with Cursor