From a374a4e1d34024cbc3f6fac58cdc9d8b3550a054 Mon Sep 17 00:00:00 2001 From: Dev Date: Thu, 9 Apr 2026 12:59:12 +0200 Subject: [PATCH] docs(17): create phase plan for group expansion in HTML reports Co-Authored-By: Claude Opus 4.6 (1M context) --- .planning/ROADMAP.md | 11 +- .../17-01-PLAN.md | 212 +++++++++++++ .../17-02-PLAN.md | 288 ++++++++++++++++++ 3 files changed, 507 insertions(+), 4 deletions(-) create mode 100644 .planning/phases/17-group-expansion-html-reports/17-01-PLAN.md create mode 100644 .planning/phases/17-group-expansion-html-reports/17-02-PLAN.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index c9a7958..6acd4ba 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -45,7 +45,7 @@ - [x] **Phase 15: Consolidation Data Model** (2 plans) — PermissionConsolidator service and merged-row model; zero API calls, pure data shapes (completed 2026-04-09) - [x] **Phase 16: Report Consolidation Toggle** (2 plans) — Export settings toggle wired to PermissionConsolidator; first user-visible consolidation behavior (completed 2026-04-09) -- [ ] **Phase 17: Group Expansion in HTML Reports** — Clickable group expansion in HTML exports with transitive membership resolution +- [ ] **Phase 17: Group Expansion in HTML Reports** (2 plans) — Clickable group expansion in HTML exports with transitive membership resolution - [ ] **Phase 18: Auto-Take Ownership** — Global toggle and automatic site collection admin elevation on access denied - [ ] **Phase 19: App Registration & Removal** — Automated Entra app registration with guided fallback and clean removal @@ -88,7 +88,10 @@ Plans: 2. Member resolution includes transitive membership: nested groups are recursively resolved so every leaf user is shown 3. Group expansion is triggered at export time via Graph API — the permission scan itself is unchanged 4. When Graph cannot resolve a group's members (throttled or insufficient scope), the report shows the group row with a "members unavailable" label rather than failing the export -**Plans**: TBD +**Plans:** 2 plans +Plans: +- [ ] 17-01-PLAN.md — ResolvedMember model + ISharePointGroupResolver service (CSOM + Graph transitive resolution) + DI registration +- [ ] 17-02-PLAN.md — HtmlExportService expandable group pills + toggleGroup JS + PermissionsViewModel wiring ### Phase 18: Auto-Take Ownership **Goal**: Users can enable automatic site collection admin elevation so that access-denied sites during scans no longer block audit progress @@ -121,7 +124,7 @@ Plans: | 6-9 | v1.1 | 25/25 | Shipped | 2026-04-08 | | 10-14 | v2.2 | 14/14 | Shipped | 2026-04-09 | | 15. Consolidation Data Model | v2.3 | 2/2 | Complete | 2026-04-09 | -| 16. Report Consolidation Toggle | 2/2 | Complete | 2026-04-09 | — | -| 17. Group Expansion in HTML Reports | v2.3 | 0/? | Not started | — | +| 16. Report Consolidation Toggle | v2.3 | 2/2 | Complete | 2026-04-09 | +| 17. Group Expansion in HTML Reports | v2.3 | 0/2 | Not started | — | | 18. Auto-Take Ownership | v2.3 | 0/? | Not started | — | | 19. App Registration & Removal | v2.3 | 0/? | Not started | — | diff --git a/.planning/phases/17-group-expansion-html-reports/17-01-PLAN.md b/.planning/phases/17-group-expansion-html-reports/17-01-PLAN.md new file mode 100644 index 0000000..e16d1d5 --- /dev/null +++ b/.planning/phases/17-group-expansion-html-reports/17-01-PLAN.md @@ -0,0 +1,212 @@ +--- +phase: 17-group-expansion-html-reports +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - SharepointToolbox/Core/Models/ResolvedMember.cs + - SharepointToolbox/Services/ISharePointGroupResolver.cs + - SharepointToolbox/Services/SharePointGroupResolver.cs + - SharepointToolbox/App.xaml.cs + - SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs +autonomous: true +requirements: [RPT-02] + +must_haves: + truths: + - "SharePointGroupResolver returns a dictionary keyed by group name with OrdinalIgnoreCase comparison" + - "SharePointGroupResolver returns empty list (never throws) when a group cannot be resolved" + - "IsAadGroup correctly identifies AAD group login patterns (c:0t.c|tenant|)" + - "AAD groups detected in SharePoint group members are resolved transitively via Graph" + - "ResolvedMember record exists in Core/Models with DisplayName and Login properties" + artifacts: + - path: "SharepointToolbox/Core/Models/ResolvedMember.cs" + provides: "Value record for resolved group member" + contains: "record ResolvedMember" + - path: "SharepointToolbox/Services/ISharePointGroupResolver.cs" + provides: "Interface contract for group resolution" + exports: ["ISharePointGroupResolver"] + - path: "SharepointToolbox/Services/SharePointGroupResolver.cs" + provides: "CSOM + Graph implementation of group resolution" + contains: "ResolveGroupsAsync" + - path: "SharepointToolbox/App.xaml.cs" + provides: "DI registration for ISharePointGroupResolver" + contains: "ISharePointGroupResolver" + - path: "SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs" + provides: "Unit tests for resolver logic" + contains: "IsAadGroup" + key_links: + - from: "SharepointToolbox/Services/SharePointGroupResolver.cs" + to: "GraphClientFactory" + via: "constructor injection" + pattern: "GraphClientFactory" + - from: "SharepointToolbox/Services/SharePointGroupResolver.cs" + to: "ExecuteQueryRetryHelper" + via: "CSOM retry" + pattern: "ExecuteQueryRetryAsync" +--- + + +Create the SharePoint group member resolution service that resolves group names to their transitive members via CSOM + Graph API. + +Purpose: This service is the data provider for Phase 17 — it pre-resolves group members before HTML export so the export service remains pure and synchronous. +Output: `ResolvedMember` model, `ISharePointGroupResolver` interface, `SharePointGroupResolver` implementation, DI registration, unit tests. + + + +@C:/Users/SebastienQUEROL/.claude/get-shit-done/workflows/execute-plan.md +@C:/Users/SebastienQUEROL/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/17-group-expansion-html-reports/17-RESEARCH.md + + + + +From SharepointToolbox/Core/Models/PermissionEntry.cs: +```csharp +public record PermissionEntry( + string ObjectType, + string Title, + string Url, + bool HasUniquePermissions, + string Users, + string UserLogins, + string PermissionLevels, + string GrantedThrough, + string PrincipalType // "SharePointGroup" | "User" | "External User" +); +``` + +From SharepointToolbox/Infrastructure/Auth/GraphClientFactory.cs: +```csharp +public class GraphClientFactory +{ + public async Task CreateClientAsync(string clientId, CancellationToken ct) +} +``` + +From SharepointToolbox/Core/Helpers/ExecuteQueryRetryHelper.cs: +```csharp +public static class ExecuteQueryRetryHelper +{ + public static async Task ExecuteQueryRetryAsync( + ClientContext ctx, + IProgress? progress = null, + CancellationToken ct = default) +} +``` + +From SharepointToolbox/App.xaml.cs (DI registration pattern): +```csharp +// Phase 4: Bulk Members +services.AddTransient(); +// Phase 7: User Access Audit +services.AddTransient(); +``` + + + + + + + Task 1: ResolvedMember model + ISharePointGroupResolver interface + SharePointGroupResolver implementation + tests + + SharepointToolbox/Core/Models/ResolvedMember.cs, + SharepointToolbox/Services/ISharePointGroupResolver.cs, + SharepointToolbox/Services/SharePointGroupResolver.cs, + SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs + + + - Test: IsAadGroup returns true for "c:0t.c|tenant|aaaabbbb-cccc-dddd-eeee-ffffgggghhhh" and false for "i:0#.f|membership|user@contoso.com" and false for "c:0(.s|true" + - Test: ExtractAadGroupId extracts GUID from "c:0t.c|tenant|aaaabbbb-cccc-dddd-eeee-ffffgggghhhh" -> "aaaabbbb-cccc-dddd-eeee-ffffgggghhhh" + - Test: StripClaims strips "i:0#.f|membership|user@contoso.com" -> "user@contoso.com" + - Test: ResolveGroupsAsync returns empty dict when groupNames list is empty + - Test: Result dictionary uses OrdinalIgnoreCase comparer (lookup "site members" matches key "Site Members") + + + 1. Create `Core/Models/ResolvedMember.cs`: + ```csharp + namespace SharepointToolbox.Core.Models; + public record ResolvedMember(string DisplayName, string Login); + ``` + + 2. Create `Services/ISharePointGroupResolver.cs`: + ```csharp + public interface ISharePointGroupResolver + { + Task>> ResolveGroupsAsync( + ClientContext ctx, string clientId, + IReadOnlyList groupNames, CancellationToken ct); + } + ``` + + 3. Create `Services/SharePointGroupResolver.cs` implementing `ISharePointGroupResolver`: + - Constructor takes `GraphClientFactory` (same pattern as `BulkMemberService`) + - `ResolveGroupsAsync` iterates group names (`.Distinct(StringComparer.OrdinalIgnoreCase)`) + - Per group: CSOM `ctx.Web.SiteGroups.GetByName(name).Users` with `ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(ctx, null, ct)` + - Per user: check `IsAadGroup(loginName)` — if true, extract GUID via `ExtractAadGroupId` and call `ResolveAadGroupAsync` via Graph + - `ResolveAadGroupAsync`: `graphClient.Groups[aadGroupId].TransitiveMembers.GraphUser.GetAsync()` with `PageIterator` for pagination — same pattern as `GraphUserDirectoryService` + - De-duplicate members by Login (OrdinalIgnoreCase) using `.DistinctBy(m => m.Login, StringComparer.OrdinalIgnoreCase)` + - Entire per-group block wrapped in try/catch — on any exception, log warning via `Serilog.Log.Warning` and set `result[groupName] = Array.Empty()` + - Result dictionary created with `StringComparer.OrdinalIgnoreCase` + - Make `IsAadGroup`, `ExtractAadGroupId`, and `StripClaims` internal static (with `InternalsVisibleTo` already set for test project) so they are testable + - `IsAadGroup` pattern: `login.StartsWith("c:0t.c|", StringComparison.OrdinalIgnoreCase)` + - `ExtractAadGroupId`: `login[(login.LastIndexOf('|') + 1)..]` + - `StripClaims`: `login[(login.LastIndexOf('|') + 1)..]` (same substring after last pipe) + + 4. Create `SharePointGroupResolverTests.cs`: + - Test `IsAadGroup` with true/false cases (see behavior above) + - Test `ExtractAadGroupId` extraction + - Test `StripClaims` extraction + - Test `ResolveGroupsAsync` with empty list returns empty dict (needs mock ClientContext — use `[Fact(Skip="Requires CSOM ClientContext mock")]` if CSOM types cannot be easily mocked; alternatively test the static helpers only and add a skip-marked integration test) + - Test case-insensitive dict: create resolver, call with known group, verify lookup with different casing works. If full resolution cannot be unit tested without live CSOM, mark as `[Fact(Skip="Requires live SP tenant")]` and focus unit tests on the three static helpers + + + dotnet test --filter "FullyQualifiedName~SharePointGroupResolverTests" --no-build + + ResolvedMember record exists, ISharePointGroupResolver interface defined, SharePointGroupResolver compiles with CSOM + Graph resolution, static helpers (IsAadGroup, ExtractAadGroupId, StripClaims) have green unit tests + + + + Task 2: DI registration in App.xaml.cs + SharepointToolbox/App.xaml.cs + + Add DI registration for `ISharePointGroupResolver` in `App.xaml.cs` after the Phase 4 Bulk Members block (or near other service registrations): + ```csharp + // Phase 17: Group Expansion + services.AddTransient(); + ``` + Add the `using SharepointToolbox.Services;` if not already present (it should be since `IPermissionsService` is already registered from the same namespace). + + + dotnet build --no-restore 2>&1 | tail -5 + + ISharePointGroupResolver registered in DI container, solution builds with 0 errors + + + + + +- `dotnet build` — 0 errors, 0 warnings +- `dotnet test --filter "FullyQualifiedName~SharePointGroupResolverTests"` — all tests pass +- `dotnet test` — full suite green (no regressions) + + + +- ResolvedMember record exists at Core/Models/ResolvedMember.cs +- ISharePointGroupResolver interface defines ResolveGroupsAsync contract +- SharePointGroupResolver implements CSOM group user loading + Graph transitive resolution +- Static helpers (IsAadGroup, ExtractAadGroupId, StripClaims) have passing unit tests +- DI registration wired in App.xaml.cs +- Full test suite green + + + +After completion, create `.planning/phases/17-group-expansion-html-reports/17-01-SUMMARY.md` + diff --git a/.planning/phases/17-group-expansion-html-reports/17-02-PLAN.md b/.planning/phases/17-group-expansion-html-reports/17-02-PLAN.md new file mode 100644 index 0000000..958375b --- /dev/null +++ b/.planning/phases/17-group-expansion-html-reports/17-02-PLAN.md @@ -0,0 +1,288 @@ +--- +phase: 17-group-expansion-html-reports +plan: 02 +type: execute +wave: 2 +depends_on: ["17-01"] +files_modified: + - SharepointToolbox/Services/Export/HtmlExportService.cs + - SharepointToolbox/ViewModels/Tabs/PermissionsViewModel.cs + - SharepointToolbox.Tests/Services/Export/HtmlExportServiceTests.cs +autonomous: true +requirements: [RPT-01, RPT-02] + +must_haves: + truths: + - "SharePoint group pills in the HTML report are clickable and expand to show group members" + - "When group members are resolved, clicking the pill reveals member names inline" + - "When group resolution fails, the pill expands to show 'members unavailable' label" + - "When no groupMembers dict is passed, HTML output is identical to pre-Phase 17 output" + - "toggleGroup() JS function exists in HtmlExportService inline JS" + - "PermissionsViewModel calls ISharePointGroupResolver before HTML export and passes results to BuildHtml" + artifacts: + - path: "SharepointToolbox/Services/Export/HtmlExportService.cs" + provides: "Expandable group pill rendering + toggleGroup JS" + contains: "toggleGroup" + - path: "SharepointToolbox/ViewModels/Tabs/PermissionsViewModel.cs" + provides: "Group resolution orchestration before export" + contains: "_groupResolver" + - path: "SharepointToolbox.Tests/Services/Export/HtmlExportServiceTests.cs" + provides: "Tests for group pill expansion and backward compatibility" + contains: "grpmem" + key_links: + - from: "SharepointToolbox/ViewModels/Tabs/PermissionsViewModel.cs" + to: "ISharePointGroupResolver" + via: "constructor injection + call in ExportHtmlAsync" + pattern: "_groupResolver.ResolveGroupsAsync" + - from: "SharepointToolbox/ViewModels/Tabs/PermissionsViewModel.cs" + to: "HtmlExportService.BuildHtml" + via: "passing groupMembers dict" + pattern: "groupMembers" + - from: "SharepointToolbox/Services/Export/HtmlExportService.cs" + to: "toggleGroup JS" + via: "inline script block" + pattern: "function toggleGroup" +--- + + +Wire group member expansion into HtmlExportService rendering and PermissionsViewModel export flow. + +Purpose: This is the user-visible feature — SharePoint group pills become clickable in HTML reports, expanding to show resolved members or a "members unavailable" fallback. +Output: Modified HtmlExportService (both overloads), modified PermissionsViewModel, new HTML export tests. + + + +@C:/Users/SebastienQUEROL/.claude/get-shit-done/workflows/execute-plan.md +@C:/Users/SebastienQUEROL/.claude/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/17-group-expansion-html-reports/17-RESEARCH.md +@.planning/phases/17-group-expansion-html-reports/17-01-SUMMARY.md + + + + +From SharepointToolbox/Core/Models/ResolvedMember.cs: +```csharp +public record ResolvedMember(string DisplayName, string Login); +``` + +From SharepointToolbox/Services/ISharePointGroupResolver.cs: +```csharp +public interface ISharePointGroupResolver +{ + Task>> ResolveGroupsAsync( + ClientContext ctx, string clientId, + IReadOnlyList groupNames, CancellationToken ct); +} +``` + + + +From SharepointToolbox/Services/Export/HtmlExportService.cs: +```csharp +public class HtmlExportService +{ + // Overload 1 — standard PermissionEntry + public string BuildHtml(IReadOnlyList entries, ReportBranding? branding = null) + public async Task WriteAsync(IReadOnlyList entries, string filePath, CancellationToken ct, ReportBranding? branding = null) + + // Overload 2 — simplified + public string BuildHtml(IReadOnlyList entries, ReportBranding? branding = null) + public async Task WriteAsync(IReadOnlyList entries, string filePath, CancellationToken ct, ReportBranding? branding = null) +} +``` + +From SharepointToolbox/ViewModels/Tabs/PermissionsViewModel.cs: +```csharp +public partial class PermissionsViewModel : FeatureViewModelBase +{ + private readonly HtmlExportService? _htmlExportService; + private readonly ISessionManager _sessionManager; + private readonly IBrandingService? _brandingService; + private TenantProfile? _currentProfile; + + public PermissionsViewModel( + IPermissionsService permissionsService, + ISiteListService siteListService, + ISessionManager sessionManager, + CsvExportService csvExportService, + HtmlExportService htmlExportService, + IBrandingService brandingService, + ILogger logger) + + // Test constructor (no export services) + internal PermissionsViewModel( + IPermissionsService permissionsService, + ISiteListService siteListService, + ISessionManager sessionManager, + ILogger logger, + IBrandingService? brandingService = null) + + private async Task ExportHtmlAsync() + { + // Currently calls: _htmlExportService.WriteAsync(entries, path, ct, branding) + } +} +``` + +From SharepointToolbox/Services/Export/UserAccessHtmlExportService.cs (toggleGroup JS to copy): +```javascript +function toggleGroup(id) { + var rows = document.querySelectorAll('tr[data-group="' + id + '"]'); + rows.forEach(function(r) { r.style.display = r.style.display === 'none' ? '' : 'none'; }); +} +``` + + + + + + + Task 1: Extend HtmlExportService with groupMembers parameter and expandable group pills + + SharepointToolbox/Services/Export/HtmlExportService.cs, + SharepointToolbox.Tests/Services/Export/HtmlExportServiceTests.cs + + + - RPT-01-a: BuildHtml with no groupMembers (null/omitted) produces output identical to pre-Phase 17 + - RPT-01-b: BuildHtml with groupMembers containing a group name renders clickable pill with onclick="toggleGroup('grpmem0')" and class "group-expandable" + - RPT-01-c: BuildHtml with resolved members renders hidden sub-row (data-group="grpmem0", display:none) containing member display names + - RPT-01-d: BuildHtml with empty member list (resolution failed) renders sub-row with "members unavailable" italic label + - RPT-01-e: BuildHtml output contains "function toggleGroup" in inline JS + - RPT-01-f: Simplified BuildHtml overload also accepts groupMembers and renders expandable pills identically + + + 1. Add `groupMembers` optional parameter to BOTH `BuildHtml` overloads and BOTH `WriteAsync` methods: + ```csharp + 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) + ``` + Same for the `SimplifiedPermissionEntry` overloads. `WriteAsync` passes `groupMembers` through to `BuildHtml`. + + 2. Add CSS for `.group-expandable` in the inline `