docs(19): create phase plan for app registration and removal

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-09 14:48:49 +02:00
parent 0d087ae4cd
commit 7d200ecf3f
3 changed files with 574 additions and 3 deletions
--- a/.planning/phases/19-app-registration-removal/19-01-PLAN.md
+++ b/.planning/phases/19-app-registration-removal/19-01-PLAN.md
@@ -0,0 +1,239 @@
+---
+phase: 19-app-registration-removal
+plan: 01
+type: execute
+wave: 1
+depends_on: []
+files_modified:
+  - SharepointToolbox/Core/Models/AppRegistrationResult.cs
+  - SharepointToolbox/Core/Models/TenantProfile.cs
+  - SharepointToolbox/Services/IAppRegistrationService.cs
+  - SharepointToolbox/Services/AppRegistrationService.cs
+  - SharepointToolbox.Tests/Services/AppRegistrationServiceTests.cs
+autonomous: true
+requirements: [APPREG-02, APPREG-03, APPREG-06]
+
+must_haves:
+  truths:
+    - "IsGlobalAdminAsync returns true when user has Global Admin directory role"
+    - "IsGlobalAdminAsync returns false (not throws) when user lacks role or gets 403"
+    - "RegisterAsync creates Application + ServicePrincipal + OAuth2PermissionGrants in sequence"
+    - "RegisterAsync rolls back (deletes Application) when any intermediate step fails"
+    - "RemoveAsync deletes the Application by appId and clears MSAL session"
+    - "TenantProfile.AppId is nullable and round-trips through JSON serialization"
+    - "AppRegistrationResult discriminates Success (with appId), Failure (with message), and Fallback"
+  artifacts:
+    - path: "SharepointToolbox/Core/Models/AppRegistrationResult.cs"
+      provides: "Discriminated result type for registration outcomes"
+      contains: "class AppRegistrationResult"
+    - path: "SharepointToolbox/Core/Models/TenantProfile.cs"
+      provides: "AppId nullable property for storing registered app ID"
+      contains: "AppId"
+    - path: "SharepointToolbox/Services/IAppRegistrationService.cs"
+      provides: "Service interface with IsGlobalAdminAsync, RegisterAsync, RemoveAsync, ClearMsalSessionAsync"
+      exports: ["IAppRegistrationService"]
+    - path: "SharepointToolbox/Services/AppRegistrationService.cs"
+      provides: "Implementation using GraphServiceClient"
+      contains: "class AppRegistrationService"
+    - path: "SharepointToolbox.Tests/Services/AppRegistrationServiceTests.cs"
+      provides: "Unit tests covering admin detection, registration, rollback, removal, session clear"
+      min_lines: 80
+  key_links:
+    - from: "SharepointToolbox/Services/AppRegistrationService.cs"
+      to: "GraphServiceClient"
+      via: "constructor injection of GraphClientFactory"
+      pattern: "GraphClientFactory"
+    - from: "SharepointToolbox/Services/AppRegistrationService.cs"
+      to: "MsalClientFactory"
+      via: "constructor injection for session eviction"
+      pattern: "MsalClientFactory"
+---
+
+<objective>
+Create the AppRegistrationService with full Graph API registration/removal logic, the AppRegistrationResult model, and add AppId to TenantProfile. All unit-tested with mocked Graph calls.
+
+Purpose: The service layer is the foundation for all Entra app registration operations. It must be fully testable before any UI is wired.
+Output: IAppRegistrationService + implementation + AppRegistrationResult model + TenantProfile.AppId field + unit tests
+</objective>
+
+<execution_context>
+@C:/Users/dev/.claude/get-shit-done/workflows/execute-plan.md
+@C:/Users/dev/.claude/get-shit-done/templates/summary.md
+</execution_context>
+
+<context>
+@.planning/PROJECT.md
+@.planning/ROADMAP.md
+@.planning/STATE.md
+@.planning/phases/19-app-registration-removal/19-RESEARCH.md
+
+<interfaces>
+<!-- Existing code the executor needs -->
+
+From SharepointToolbox/Infrastructure/Auth/GraphClientFactory.cs:
+```csharp
+public class GraphClientFactory
+{
+    public GraphClientFactory(MsalClientFactory msalFactory) { }
+    public async Task<GraphServiceClient> CreateClientAsync(string clientId, CancellationToken ct) { }
+}
+```
+
+From SharepointToolbox/Infrastructure/Auth/MsalClientFactory.cs:
+```csharp
+public class MsalClientFactory
+{
+    public string CacheDirectory { get; }
+    public async Task<IPublicClientApplication> GetOrCreateAsync(string clientId) { }
+    public MsalCacheHelper GetCacheHelper(string clientId) { }
+}
+```
+
+From SharepointToolbox/Services/ISessionManager.cs:
+```csharp
+public interface ISessionManager
+{
+    Task<ClientContext> GetOrCreateContextAsync(TenantProfile profile, CancellationToken ct = default);
+    Task ClearSessionAsync(string tenantUrl);
+    bool IsAuthenticated(string tenantUrl);
+}
+```
+
+From SharepointToolbox/Core/Models/TenantProfile.cs:
+```csharp
+public class TenantProfile
+{
+    public string Name { get; set; } = string.Empty;
+    public string TenantUrl { get; set; } = string.Empty;
+    public string ClientId { get; set; } = string.Empty;
+    public LogoData? ClientLogo { get; set; }
+}
+```
+</interfaces>
+</context>
+
+<tasks>
+
+<task type="auto" tdd="true">
+  <name>Task 1: Models + Interface + Service implementation</name>
+  <files>
+    SharepointToolbox/Core/Models/AppRegistrationResult.cs,
+    SharepointToolbox/Core/Models/TenantProfile.cs,
+    SharepointToolbox/Services/IAppRegistrationService.cs,
+    SharepointToolbox/Services/AppRegistrationService.cs
+  </files>
+  <behavior>
+    - AppRegistrationResult.Success("appId123") carries appId, IsSuccess=true
+    - AppRegistrationResult.Failure("msg") carries message, IsSuccess=false
+    - AppRegistrationResult.FallbackRequired() signals fallback path, IsSuccess=false, IsFallback=true
+    - TenantProfile.AppId is nullable string, defaults to null, serializes/deserializes via System.Text.Json
+  </behavior>
+  <action>
+    1. Create `AppRegistrationResult.cs` in `Core/Models/`:
+       - Static factory methods: `Success(string appId)`, `Failure(string errorMessage)`, `FallbackRequired()`
+       - Properties: `bool IsSuccess`, `bool IsFallback`, `string? AppId`, `string? ErrorMessage`
+       - Use a private constructor pattern (not record, for consistency with other models in the project)
+
+    2. Update `TenantProfile.cs`:
+       - Add `public string? AppId { get; set; }` property (nullable, defaults to null)
+       - Existing properties unchanged
+
+    3. Create `IAppRegistrationService.cs` in `Services/`:
+       ```csharp
+       public interface IAppRegistrationService
+       {
+           Task<bool> IsGlobalAdminAsync(string clientId, CancellationToken ct);
+           Task<AppRegistrationResult> RegisterAsync(string clientId, string tenantDisplayName, CancellationToken ct);
+           Task RemoveAsync(string clientId, string appId, CancellationToken ct);
+           Task ClearMsalSessionAsync(string clientId, string tenantUrl);
+       }
+       ```
+
+    4. Create `AppRegistrationService.cs` in `Services/`:
+       - Constructor takes `GraphClientFactory`, `MsalClientFactory`, `ISessionManager`, `ILogger<AppRegistrationService>`
+       - `IsGlobalAdminAsync`: calls `graphClient.Me.TransitiveMemberOf.GetAsync()` filtered on `microsoft.graph.directoryRole`, checks for roleTemplateId `62e90394-69f5-4237-9190-012177145e10`. On any exception (including 403), return false and log warning.
+       - `RegisterAsync`:
+         a. Create Application object with `DisplayName = "SharePoint Toolbox - {tenantDisplayName}"`, `SignInAudience = "AzureADMyOrg"`, `IsFallbackPublicClient = true`, `PublicClient.RedirectUris = ["https://login.microsoftonline.com/common/oauth2/nativeclient"]`, `RequiredResourceAccess` for Graph (User.Read, User.Read.All, Group.Read.All, Directory.Read.All) and SharePoint (AllSites.FullControl) using the GUIDs from research.
+         b. Create ServicePrincipal with `AppId = createdApp.AppId`
+         c. Look up Microsoft Graph resource SP via filter `appId eq '00000003-0000-0000-c000-000000000000'`, get its `Id`
+         d. Look up SharePoint Online resource SP via filter `appId eq '00000003-0000-0ff1-ce00-000000000000'`, get its `Id`
+         e. Post `OAuth2PermissionGrant` for Graph scopes (`User.Read User.Read.All Group.Read.All Directory.Read.All`) with `ConsentType = "AllPrincipals"`, `ClientId = sp.Id`, `ResourceId = graphResourceSp.Id`
+         f. Post `OAuth2PermissionGrant` for SharePoint scopes (`AllSites.FullControl`) same pattern
+         g. On any exception after Application creation: try `DELETE /applications/{createdApp.Id}` (best-effort rollback, log warning on rollback failure), return `AppRegistrationResult.Failure(ex.Message)`
+         h. On success: return `AppRegistrationResult.Success(createdApp.AppId!)`
+       - `RemoveAsync`: calls `graphClient.Applications[$"(appId='{appId}')"].DeleteAsync(cancellationToken: ct)`. Log warning on failure but don't throw.
+       - `ClearMsalSessionAsync`:
+         a. Call `_sessionManager.ClearSessionAsync(tenantUrl)`
+         b. Get PCA via `_msalFactory.GetOrCreateAsync(clientId)`, loop `RemoveAsync` on all accounts
+         c. Call `_msalFactory.GetCacheHelper(clientId).UnregisterCache(pca.UserTokenCache)`
+
+    Use `private static List<RequiredResourceAccess> BuildRequiredResourceAccess()` as a helper. Use GUIDs from research doc (Graph permissions are HIGH confidence). For SharePoint AllSites.FullControl, use GUID `56680e0d-d2a3-4ae1-80d8-3c4a5c70c4a6` from research (LOW confidence — add a comment noting it should be verified against live tenant).
+  </action>
+  <verify>
+    <automated>dotnet build SharepointToolbox/SharepointToolbox.csproj --no-restore 2>&1 | tail -5</automated>
+  </verify>
+  <done>All 4 files exist, solution builds clean, AppRegistrationResult has 3 factory methods, TenantProfile has AppId, IAppRegistrationService has 4 methods, AppRegistrationService implements all 4 with rollback pattern</done>
+</task>
+
+<task type="auto" tdd="true">
+  <name>Task 2: Unit tests for AppRegistrationService</name>
+  <files>SharepointToolbox.Tests/Services/AppRegistrationServiceTests.cs</files>
+  <behavior>
+    - IsGlobalAdminAsync returns true when transitiveMemberOf contains DirectoryRole with Global Admin templateId
+    - IsGlobalAdminAsync returns false when no matching role
+    - IsGlobalAdminAsync returns false (not throws) on ServiceException/403
+    - RegisterAsync returns Success with appId on full happy path
+    - RegisterAsync calls DELETE on Application when ServicePrincipal creation fails (rollback)
+    - RegisterAsync calls DELETE on Application when OAuth2PermissionGrant fails (rollback)
+    - RemoveAsync calls DELETE on Application by appId
+    - ClearMsalSessionAsync calls ClearSessionAsync + removes all MSAL accounts
+    - AppRegistrationResult.Success carries appId, .Failure carries message, .FallbackRequired sets IsFallback
+    - TenantProfile.AppId round-trips through JSON (null and non-null)
+  </behavior>
+  <action>
+    Create `SharepointToolbox.Tests/Services/AppRegistrationServiceTests.cs` with xUnit tests. Since `GraphServiceClient` is hard to mock directly (sealed/extension methods), test strategy:
+
+    1. **AppRegistrationResult model tests** (pure logic, no mocks):
+       - `Success_CarriesAppId`: verify IsSuccess=true, AppId set
+       - `Failure_CarriesMessage`: verify IsSuccess=false, ErrorMessage set
+       - `FallbackRequired_SetsFallback`: verify IsFallback=true
+
+    2. **TenantProfile.AppId tests**:
+       - `AppId_DefaultsToNull`
+       - `AppId_RoundTrips_ViaJson`: serialize+deserialize with System.Text.Json, verify AppId preserved
+       - `AppId_Null_RoundTrips_ViaJson`: verify null survives serialization
+
+    3. **AppRegistrationService tests** — For methods that call GraphServiceClient, use the project's existing pattern: if the project uses Moq or NSubstitute (check test csproj), mock `GraphClientFactory` to return a mock `GraphServiceClient`. If mocking Graph SDK is too complex, test the logic by:
+       - Extracting `BuildRequiredResourceAccess()` as internal and testing the scope GUIDs/structure directly
+       - Testing that the service constructor accepts the right dependencies
+       - For integration-like behavior, mark tests with `[Trait("Category","Integration")]` and skip in CI
+
+    Check the test project's existing packages first (`dotnet list SharepointToolbox.Tests package`) to see if Moq/NSubstitute is available. Use whichever mocking library the project already uses.
+
+    All tests decorated with `[Trait("Category", "Unit")]`.
+  </action>
+  <verify>
+    <automated>dotnet test SharepointToolbox.Tests --filter "FullyQualifiedName~AppRegistrationServiceTests" --no-restore --verbosity normal 2>&1 | tail -20</automated>
+  </verify>
+  <done>All unit tests pass. Coverage: AppRegistrationResult 3 factory methods tested, TenantProfile.AppId serialization tested, service constructor/dependency tests pass, BuildRequiredResourceAccess structure verified</done>
+</task>
+
+</tasks>
+
+<verification>
+1. `dotnet build` — full solution compiles
+2. `dotnet test SharepointToolbox.Tests --filter "FullyQualifiedName~AppRegistrationServiceTests"` — all tests green
+3. TenantProfile.AppId exists as nullable string
+4. IAppRegistrationService has 4 methods: IsGlobalAdminAsync, RegisterAsync, RemoveAsync, ClearMsalSessionAsync
+</verification>
+
+<success_criteria>
+- AppRegistrationService implements atomic registration with rollback
+- IsGlobalAdminAsync uses transitiveMemberOf (not memberOf) for nested role coverage
+- All unit tests pass
+- Solution builds clean with no warnings in new files
+</success_criteria>
+
+<output>
+After completion, create `.planning/phases/19-app-registration-removal/19-01-SUMMARY.md`
+</output>