69c9d77be3
- 19-01-SUMMARY.md: service layer implementation with rollback pattern - STATE.md: progress 98%, decisions added, session updated - ROADMAP.md: phase 19 in-progress (1/2 plans) - REQUIREMENTS.md: APPREG-02, APPREG-03, APPREG-06 marked complete
119 lines
5.6 KiB
Markdown
119 lines
5.6 KiB
Markdown
---
|
|
phase: 19-app-registration-removal
|
|
plan: 01
|
|
subsystem: Services / Models
|
|
tags: [graph-api, app-registration, msal, unit-tests]
|
|
dependency_graph:
|
|
requires: [GraphClientFactory, MsalClientFactory, ISessionManager]
|
|
provides: [IAppRegistrationService, AppRegistrationService, AppRegistrationResult]
|
|
affects: [TenantProfile]
|
|
tech_stack:
|
|
added: []
|
|
patterns: [sequential-registration-with-rollback, transitiveMemberOf-admin-check, MSAL-session-eviction]
|
|
key_files:
|
|
created:
|
|
- SharepointToolbox/Core/Models/AppRegistrationResult.cs
|
|
- SharepointToolbox/Services/IAppRegistrationService.cs
|
|
- SharepointToolbox/Services/AppRegistrationService.cs
|
|
- SharepointToolbox.Tests/Services/AppRegistrationServiceTests.cs
|
|
modified:
|
|
- SharepointToolbox/Core/Models/TenantProfile.cs
|
|
decisions:
|
|
- "AppRegistrationService uses AppGraphClientFactory alias to disambiguate from Microsoft.Graph.GraphClientFactory (same pattern as GraphUserDirectoryService)"
|
|
- "PostAsync/DeleteAsync calls use named cancellationToken: ct parameter — Kiota-based Graph SDK v5 signatures have requestConfig as second positional arg"
|
|
- "BuildRequiredResourceAccess declared internal (not private) to enable direct unit testing without live Graph calls"
|
|
- "SharePoint AllSites.FullControl GUID (56680e0d-d2a3-4ae1-80d8-3c4a5c70c4a6) marked LOW confidence in code comment — must be verified against live tenant"
|
|
metrics:
|
|
duration: 4 minutes
|
|
completed: 2026-04-09
|
|
tasks_completed: 2
|
|
files_created: 4
|
|
files_modified: 1
|
|
---
|
|
|
|
# Phase 19 Plan 01: AppRegistrationService — Models, Interface, Implementation, and Tests Summary
|
|
|
|
**One-liner:** AppRegistrationService with atomic Graph API registration/rollback using transitiveMemberOf admin check, MSAL eviction, and AppRegistrationResult discriminated result type.
|
|
|
|
## Tasks Completed
|
|
|
|
| # | Name | Commit | Key Files |
|
|
|---|------|--------|-----------|
|
|
| 1 | Models + Interface + Service implementation | 93dbb8c | AppRegistrationResult.cs, TenantProfile.cs, IAppRegistrationService.cs, AppRegistrationService.cs |
|
|
| 2 | Unit tests for AppRegistrationService | 8083cdf | AppRegistrationServiceTests.cs |
|
|
|
|
## What Was Built
|
|
|
|
### AppRegistrationResult (Core/Models)
|
|
Discriminated result type with three static factory methods:
|
|
- `Success(appId)` — IsSuccess=true, carries appId
|
|
- `Failure(message)` — IsSuccess=false, carries error message
|
|
- `FallbackRequired()` — IsFallback=true, neither success nor error
|
|
|
|
### TenantProfile (Core/Models)
|
|
Added nullable `AppId` property. Defaults to null; stored to JSON via System.Text.Json when ProfileService persists profiles.
|
|
|
|
### IAppRegistrationService (Services)
|
|
Four-method interface:
|
|
- `IsGlobalAdminAsync` — transitiveMemberOf check, returns false on any exception
|
|
- `RegisterAsync` — sequential 4-step registration with rollback on failure
|
|
- `RemoveAsync` — deletes by appId, swallows exceptions (logs warning)
|
|
- `ClearMsalSessionAsync` — SessionManager + MSAL account eviction + cache unregister
|
|
|
|
### AppRegistrationService (Services)
|
|
Full implementation using GraphClientFactory (identical alias pattern to GraphUserDirectoryService). Registration flow:
|
|
1. Create Application object with RequiredResourceAccess (Graph + SharePoint scopes)
|
|
2. Create ServicePrincipal with AppId
|
|
3. Look up Microsoft Graph resource SP by filter
|
|
4. Look up SharePoint Online resource SP by filter
|
|
5. Post OAuth2PermissionGrant for Graph delegated scopes
|
|
6. Post OAuth2PermissionGrant for SharePoint delegated scopes
|
|
7. Rollback (best-effort DELETE) if any step fails
|
|
|
|
### Unit Tests (12 passing)
|
|
- AppRegistrationResult: 3 factory method tests
|
|
- TenantProfile.AppId: null default + JSON round-trip (null and non-null)
|
|
- Service constructor/interface check
|
|
- BuildRequiredResourceAccess: 2 resources, 4 Graph scopes, 1 SharePoint scope, all Type="Scope"
|
|
|
|
## Deviations from Plan
|
|
|
|
### Auto-fixed Issues
|
|
|
|
**1. [Rule 3 - Blocking] GraphClientFactory namespace ambiguity**
|
|
- **Found during:** Task 1 build
|
|
- **Issue:** `GraphClientFactory` is ambiguous between `SharepointToolbox.Infrastructure.Auth.GraphClientFactory` and `Microsoft.Graph.GraphClientFactory`
|
|
- **Fix:** Applied `AppGraphClientFactory` alias (same pattern used in GraphUserDirectoryService)
|
|
- **Files modified:** AppRegistrationService.cs
|
|
- **Commit:** 93dbb8c
|
|
|
|
**2. [Rule 3 - Blocking] Graph SDK v5 PostAsync CancellationToken position**
|
|
- **Found during:** Task 1 build
|
|
- **Issue:** `PostAsync(body, ct)` fails — Kiota-based SDK expects `(body, requestConfig?, cancellationToken)` so `ct` was matched to `requestConfig` parameter
|
|
- **Fix:** Used named parameter `cancellationToken: ct` on all PostAsync calls
|
|
- **Files modified:** AppRegistrationService.cs
|
|
- **Commit:** 93dbb8c
|
|
|
|
**3. [Rule 3 - Blocking] Using alias placement in test file**
|
|
- **Found during:** Task 2 compile
|
|
- **Issue:** Placed `using` aliases at bottom of file — C# requires all `using` declarations before namespace body
|
|
- **Fix:** Moved aliases to top of file with other using directives
|
|
- **Files modified:** AppRegistrationServiceTests.cs
|
|
- **Commit:** 8083cdf
|
|
|
|
## Self-Check: PASSED
|
|
|
|
Files exist:
|
|
- SharepointToolbox/Core/Models/AppRegistrationResult.cs — FOUND
|
|
- SharepointToolbox/Core/Models/TenantProfile.cs — FOUND (modified)
|
|
- SharepointToolbox/Services/IAppRegistrationService.cs — FOUND
|
|
- SharepointToolbox/Services/AppRegistrationService.cs — FOUND
|
|
- SharepointToolbox.Tests/Services/AppRegistrationServiceTests.cs — FOUND
|
|
|
|
Commits verified:
|
|
- 93dbb8c — feat(19-01): add AppRegistrationService with rollback, model, and interface
|
|
- 8083cdf — test(19-01): add unit tests for AppRegistrationService and models
|
|
|
|
Tests: 12 passed, 0 failed
|
|
Build: 0 errors, 0 warnings
|