diff --git a/.planning/phases/17-group-expansion-html-reports/17-RESEARCH.md b/.planning/phases/17-group-expansion-html-reports/17-RESEARCH.md new file mode 100644 index 0000000..6375627 --- /dev/null +++ b/.planning/phases/17-group-expansion-html-reports/17-RESEARCH.md @@ -0,0 +1,559 @@ +# Phase 17: Group Expansion in HTML Reports - Research + +**Researched:** 2026-04-09 +**Domain:** SharePoint CSOM group member resolution / HTML export rendering / async pre-resolution service +**Confidence:** HIGH + +## Summary + +Phase 17 adds expandable SharePoint group rows to the site-centric permissions HTML report (`HtmlExportService`). In the existing report, a SharePoint group appears as an opaque user-pill in the "Users/Groups" column (rendered from `PermissionEntry.Users`, `PrincipalType = "SharePointGroup"`). Phase 17 makes that pill clickable — clicking expands a hidden sub-row listing the group's members. + +Member resolution is a pre-render step: the ViewModel resolves all group members via CSOM before calling `BuildHtml`, then passes a resolution dictionary into the service. The HTML service itself remains pure (no async I/O). Transitive membership is achieved by recursively resolving any AAD group members found in a SharePoint group via Graph `groups/{id}/transitiveMembers`. When CSOM or Graph cannot resolve members (throttling, insufficient scope), the group pill renders with a "members unavailable" label rather than failing the export. + +The user access audit report (`UserAccessHtmlExportService`) is NOT in scope: that report is user-centric, and groups appear only as `GrantedThrough` text — there are no expandable group rows to add there. + +**Primary recommendation:** Create a new `ISharePointGroupResolver` service that uses CSOM (`ClientContext.Web.SiteGroups.GetByName(name).Users`) plus optional Graph transitive lookup for nested AAD groups. The ViewModel calls this service before export and passes `IReadOnlyDictionary> groupMembers` into `HtmlExportService.BuildHtml`. The HTML uses the existing `toggleGroup()` JS pattern with a new `grpmem{idx}` ID namespace for member sub-rows. + +--- + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|-----------------| +| RPT-01 | User can expand SharePoint groups in HTML reports to see group members | Group pill in site-centric `HtmlExportService` becomes clickable; hidden sub-rows per member revealed via `toggleGroup()`; member data passed as pre-resolved dictionary from ViewModel | +| RPT-02 | Group member resolution uses transitive membership to include nested group members | CSOM resolves direct SharePoint group users; for user entries that are AAD groups (login matches `c:0t.c|tenant|` prefix), Graph `groups/{id}/transitiveMembers/microsoft.graph.user` is called recursively; leaf users merged into final member list | + + +--- + +## Standard Stack + +### Core +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| Microsoft.SharePoint.Client (CSOM) | Already referenced | `ctx.Web.SiteGroups.GetByName(name).Users` — only supported API for classic SharePoint group members | Graph v1.0 and beta DO NOT support classic SharePoint group membership enumeration; CSOM is the only supported path | +| Microsoft.Graph (Graph SDK) | Already referenced | `graphClient.Groups[aadGroupId].TransitiveMembers.GraphUser.GetAsync()` — resolves nested AAD groups transitively | Existing Graph SDK usage throughout codebase (`GraphUserDirectoryService`, `GraphUserSearchService`, `BulkMemberService`) | +| `GraphClientFactory` | Project-internal | Creates `GraphServiceClient` with MSAL tokens | Already used in all Graph-calling services; same pattern | +| `ExecuteQueryRetryHelper` | Project-internal | CSOM retry with throttle handling | Already used in `PermissionsService`, `BulkMemberService`; handles 429 and 503 | + +### No New NuGet Packages +Phase 17 requires zero new NuGet packages. All required APIs are already referenced. + +### Critical API Finding: SharePoint Classic Groups vs Graph +**SharePoint classic groups** (e.g. "Site Members", "HR Team") are managed by SharePoint, not Azure AD. Microsoft Graph v1.0 has no endpoint for their membership. The Graph beta `sharePointGroup` resource is for SharePoint Embedded containers only — not for classic on-site groups. The only supported enumeration path is CSOM: + +```csharp +// Source: Microsoft CSOM documentation (learn.microsoft.com/sharepoint/dev) +var group = ctx.Web.SiteGroups.GetByName("Site Members"); +ctx.Load(group.Users); +await ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(ctx, progress, ct); +foreach (var user in group.Users) { ... } +``` + +A SharePoint group member can itself be an AAD/M365 group (login contains `c:0t.c|tenant|` pattern). For these nested AAD groups, Graph `transitiveMembers` resolves all leaf users in a single call — satisfying RPT-02. + +## Architecture Patterns + +### Recommended Project Structure Changes + +``` +SharepointToolbox/ +├── Services/ +│ ├── ISharePointGroupResolver.cs ← NEW interface +│ └── SharePointGroupResolver.cs ← NEW service +├── Services/Export/ +│ └── HtmlExportService.cs ← BuildHtml overload + group pill rendering +└── ViewModels/Tabs/ + └── PermissionsViewModel.cs ← call resolver before export, pass dict + +SharepointToolbox.Tests/ +└── Services/ + ├── SharePointGroupResolverTests.cs ← NEW (mostly skip-marked, requires live tenant) + └── Export/ + └── HtmlExportServiceTests.cs ← extend: group pill expansion tests +``` + +### Pattern 1: Pre-Resolution Architecture (ViewModel orchestrates, service stays pure) + +**What:** ViewModel calls `ISharePointGroupResolver.ResolveGroupsAsync()` to build a `Dictionary> groupMembers` keyed by group name (exact match with `PermissionEntry.Users` value for SharePoint group rows). This dict is passed to `BuildHtml`. HTML export service remains synchronous and pure — no async I/O inside string building. + +**Why:** Consistent with how `BrandingService` provides `ReportBranding` to the HTML service — the ViewModel fetches context, export service renders it. Avoids making export services async or injecting CSOM/Graph into what are currently pure string-building classes. + +**Data flow:** +``` +PermissionsViewModel.ExportHtmlAsync() + → collect group names from Results where PrincipalType == "SharePointGroup" + → ISharePointGroupResolver.ResolveGroupsAsync(ctx, clientId, groupNames, ct) + → per group: CSOM SiteGroups.GetByName(name).Users → List + → per nested AAD group: Graph transitiveMembers → leaf users + → on throttle/error: return empty list (graceful fallback) + → HtmlExportService.BuildHtml(entries, groupMembers, branding) + → for group pills: if groupMembers contains name → render expandable pill + hidden sub-rows + → else if name not in dict → render plain pill (no expand) + → if dict has empty list → render pill + "members unavailable" sub-row +``` + +### Pattern 2: ISharePointGroupResolver Interface + +**What:** A focused single-method async service that accepts a `ClientContext` (for CSOM) and `clientId` string (for Graph factory), resolves a set of group names to their transitive leaf-user members. + +```csharp +// NEW: Services/ISharePointGroupResolver.cs +public interface ISharePointGroupResolver +{ + /// + /// Resolves SharePoint group names to their transitive member display names. + /// Returns empty list for any group that cannot be resolved (throttled, not found, etc.). + /// Never throws — failures are surfaced as empty member lists. + /// + Task>> ResolveGroupsAsync( + ClientContext ctx, + string clientId, + IReadOnlyList groupNames, + CancellationToken ct); +} + +// Simple value record for a resolved leaf member +public record ResolvedMember(string DisplayName, string Login); +``` + +**Why a dict keyed by group name:** The `PermissionEntry.Users` field for a SharePoint group row IS the group display name (set to `member.Title` in `PermissionsService.ExtractPermissionsAsync`). The dict lookup is O(1) at render time. + +### Pattern 3: SharePointGroupResolver — CSOM + Graph Resolution + +**What:** CSOM loads group users. Each user login is inspected for the AAD group prefix pattern (`c:0t.c|tenant|`). For matching entries, the GUID is extracted and used to call `graphClient.Groups[guid].TransitiveMembers.GraphUser.GetAsync()`. All leaf users merged and de-duplicated. + +```csharp +// Source: CSOM pattern from Microsoft docs + existing ExecuteQueryRetryHelper usage +public async Task>> + ResolveGroupsAsync(ClientContext ctx, string clientId, + IReadOnlyList groupNames, CancellationToken ct) +{ + var result = new Dictionary>( + StringComparer.OrdinalIgnoreCase); + var graphClient = await _graphClientFactory.CreateClientAsync(clientId, ct); + + foreach (var groupName in groupNames.Distinct(StringComparer.OrdinalIgnoreCase)) + { + ct.ThrowIfCancellationRequested(); + try + { + var group = ctx.Web.SiteGroups.GetByName(groupName); + ctx.Load(group.Users, users => users.Include( + u => u.Title, u => u.LoginName, u => u.PrincipalType)); + await ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(ctx, null, ct); + + var members = new List(); + foreach (var user in group.Users) + { + if (IsAadGroup(user.LoginName)) + { + // Expand nested AAD group transitively via Graph + var aadId = ExtractAadGroupId(user.LoginName); + var leafUsers = await ResolveAadGroupAsync(graphClient, aadId, ct); + members.AddRange(leafUsers); + } + else + { + members.Add(new ResolvedMember( + user.Title ?? user.LoginName, + StripClaims(user.LoginName))); + } + } + result[groupName] = members + .DistinctBy(m => m.Login, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + catch (Exception ex) + { + Log.Warning("Could not resolve SP group '{Group}': {Error}", groupName, ex.Message); + result[groupName] = Array.Empty(); // graceful fallback + } + } + return result; +} + +// AAD group login pattern: "c:0t.c|tenant|" +private static bool IsAadGroup(string login) => + login.StartsWith("c:0t.c|", StringComparison.OrdinalIgnoreCase); + +private static string ExtractAadGroupId(string login) => + login[(login.LastIndexOf('|') + 1)..]; +``` + +### Pattern 4: Graph Transitive Members for Nested AAD Groups + +**What:** Call `graphClient.Groups[aadGroupId].TransitiveMembers.GraphUser.GetAsync()` to get all leaf users recursively in one call. The `/microsoft.graph.user` cast filters to only user objects (excludes sub-groups, devices, etc.). + +```csharp +// Source: Graph SDK pattern — consistent with GraphUserDirectoryService.cs PageIterator usage +private static async Task> ResolveAadGroupAsync( + GraphServiceClient graphClient, string aadGroupId, CancellationToken ct) +{ + try + { + var response = await graphClient + .Groups[aadGroupId] + .TransitiveMembers + .GraphUser + .GetAsync(config => + { + config.QueryParameters.Select = new[] { "displayName", "userPrincipalName" }; + config.QueryParameters.Top = 999; + }, ct); + + if (response?.Value is null) return Enumerable.Empty(); + + var members = new List(); + var pageIterator = PageIterator.CreatePageIterator( + graphClient, response, + user => + { + if (ct.IsCancellationRequested) return false; + members.Add(new ResolvedMember( + user.DisplayName ?? user.UserPrincipalName ?? "Unknown", + user.UserPrincipalName ?? string.Empty)); + return true; + }); + await pageIterator.IterateAsync(ct); + return members; + } + catch (Exception ex) + { + Log.Warning("Could not resolve AAD group '{Id}' transitively: {Error}", aadGroupId, ex.Message); + return Enumerable.Empty(); // graceful fallback + } +} +``` + +### Pattern 5: HTML Rendering — Expandable Group Pill + +**What:** When a `PermissionEntry` has `PrincipalType = "SharePointGroup"` and the group name is in `groupMembers`: +- If members list is non-empty: render clickable span with toggle + hidden member sub-rows +- If members list is empty (resolution failed): render span + "members unavailable" sub-row +- If group name NOT in dict (resolver wasn't called, or group was skipped): render plain pill (existing behavior) + +Uses the SAME `toggleGroup()` JS function already in both `BuildHtml` (standard) and `BuildConsolidatedHtml`. New ID namespace: `grpmem{idx}` (distinct from `ugrp`, `sgrp`, `loc`). + +```html + +HR Site Members ▼ + + + + Alice Smith <alice@contoso.com> • + Bob Jones <bob@contoso.com> + + + + +Visitors ▼ + + + members unavailable + + +``` + +**CSS addition (inline in BuildHtml):** +```css +.group-expandable { cursor: pointer; } +.group-expandable:hover { opacity: 0.8; } +``` + +**`toggleGroup()` JS**: Reuse the EXISTING function from `HtmlExportService.BuildHtml` — it is currently absent from `HtmlExportService` (unlike `UserAccessHtmlExportService`). The standard `HtmlExportService` only has a `filterTable()` function. Phase 17 must add `toggleGroup()` to `HtmlExportService`'s inline JS. + +### Pattern 6: PermissionsViewModel — Identify Groups and Call Resolver + +**What:** In `ExportHtmlAsync()`, before calling `_htmlExportService.WriteAsync()`: +1. Collect distinct group names from `Results` where `PrincipalType == "SharePointGroup"` +2. If no groups → pass empty dict (existing behavior, no CSOM calls) +3. If groups exist → call `_sharePointGroupResolver.ResolveGroupsAsync(ctx, clientId, groupNames, ct)` +4. Pass resolved dict to `BuildHtml` + +The ViewModel already has `_currentProfile` (for `ClientId`) and `_sessionManager` (for `ClientContext`). A `ClientContext` for the current site is available via `sessionManager.GetOrCreateContextAsync(profile, ct)`. + +**Complication**: The permissions scan may span multiple sites. Groups are site-scoped — a group named "Site Members" may exist on every site. Phase 17 resolves group names using the FIRST site's context (or the selected site if single-site). This is acceptable because group names in the HTML report are also site-scoped in the display — the group name badge appears inline per row. + +**Alternative (simpler)**: Resolve using the primary context from `_currentProfile` (the site collection admin context used for scanning). SharePoint group names are per-site, so for multi-site scans, group names may collide between sites. The simpler approach: resolve using the first available context. This is flagged as an open question. + +### Pattern 7: BuildHtml Signature Extension + +**What:** Add an optional `IReadOnlyDictionary>? groupMembers = null` parameter to both `BuildHtml` overloads and `WriteAsync`. Passing `null` (or omitting) preserves 100% existing behavior — existing callers need zero changes. + +```csharp +// HtmlExportService.cs — add parameter to both BuildHtml overloads and WriteAsync +public string BuildHtml( + IReadOnlyList entries, + ReportBranding? branding = null, + IReadOnlyDictionary>? groupMembers = null) + +public async Task WriteAsync( + IReadOnlyList entries, + string filePath, + CancellationToken ct, + ReportBranding? branding = null, + IReadOnlyDictionary>? groupMembers = null) +``` + +### Anti-Patterns to Avoid + +- **Making `HtmlExportService.BuildHtml` async**: Breaks the pure/testable design. Pre-resolve in ViewModel, pass dict. +- **Injecting `ISharePointGroupResolver` into `HtmlExportService`**: Export service should remain infrastructure-free. Resolver belongs in the ViewModel layer. +- **Using Graph v1.0 `groups/{id}/members` to get SharePoint classic group members**: Graph does NOT support classic SharePoint group membership. Only CSOM works. +- **Calling resolver for every export regardless of whether groups exist**: Always check `Results.Any(r => r.PrincipalType == "SharePointGroup")` first — skip resolver entirely if no group rows present (zero CSOM calls overhead). +- **Re-using `ugrp`/`sgrp`/`loc` ID prefixes for group member sub-rows**: Collisions with existing toggleGroup IDs in `UserAccessHtmlExportService`. Use `grpmem{idx}`. +- **Blocking the UI thread during group resolution**: Resolution must be `await`-ed in an async command, same pattern as existing `ExportHtmlAsync` in `PermissionsViewModel`. + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| CSOM retry / throttle handling | Custom retry loop | `ExecuteQueryRetryHelper.ExecuteQueryRetryAsync` | Already handles 429/503 with exponential backoff; identical to how `PermissionsService` calls CSOM | +| Graph auth token acquisition | Custom HTTP client | `GraphClientFactory.CreateClientAsync(clientId, ct)` | MSAL + Graph SDK already wired; same factory used in `GraphUserDirectoryService` and `BulkMemberService` | +| Transitive AAD group expansion | Recursive Graph calls | `groups/{id}/transitiveMembers/microsoft.graph.user` + `PageIterator` | Single Graph call returns all leaf users regardless of nesting depth; `PageIterator` handles pagination | +| JavaScript expand/collapse for member rows | New JS function | Existing `toggleGroup(id)` function — add it to `HtmlExportService` | Identical implementation already in `UserAccessHtmlExportService`; just needs to be added to `HtmlExportService` inline JS block | +| HTML encoding | Custom escaping | `HtmlExportService.HtmlEncode()` private method (already exists) | Already handles `&`, `<`, `>`, `"`, `'` | + +## Common Pitfalls + +### Pitfall 1: Confusing SharePoint Classic Groups with AAD/M365 Groups +**What goes wrong:** Developer calls `graphClient.Groups[id]` for a SharePoint classic group (login `c:0(.s|true` prefix or similar) — this throws a 404 because SharePoint groups are not AAD groups. +**Why it happens:** SharePoint groups have login names like `c:0(.s|true` or `c:0t.c|tenant|` — not the same as AAD group objects. +**How to avoid:** Only call Graph for logins that match the AAD group pattern `c:0t.c|tenant|` (a valid GUID in the final segment). All other members are treated as leaf users directly. +**Warning signs:** `ServiceException: Resource not found` from Graph SDK during member resolution. + +### Pitfall 2: `toggleGroup()` Missing from `HtmlExportService` Inline JS +**What goes wrong:** The group pill onclick calls `toggleGroup('grpmem0')` but `HtmlExportService` currently only has `filterTable()` in its inline JS — not `toggleGroup()`. +**Why it happens:** `toggleGroup()` was added to `UserAccessHtmlExportService` (Phase 7) but never added to `HtmlExportService` (which renders the site-centric permissions report). +**How to avoid:** Add `toggleGroup()` to `HtmlExportService`'s `