docs(19-01): complete AppRegistrationService plan execution
- 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
This commit is contained in:
Dev
@@ -10,11 +10,11 @@ Requirements for v2.3 Tenant Management & Report Enhancements. Each maps to road
|
||||
### App Registration
|
||||
|
||||
- [ ] **APPREG-01**: User can register the app on a target tenant from the profile create/edit dialog
|
||||
- [ ] **APPREG-02**: App auto-detects if user has Global Admin permissions before attempting registration
|
||||
- [ ] **APPREG-03**: App creates Azure AD application + service principal + grants required permissions atomically (with rollback on failure)
|
||||
- [x] **APPREG-02**: App auto-detects if user has Global Admin permissions before attempting registration
|
||||
- [x] **APPREG-03**: App creates Azure AD application + service principal + grants required permissions atomically (with rollback on failure)
|
||||
- [ ] **APPREG-04**: User sees guided fallback instructions when auto-registration is not possible (insufficient permissions)
|
||||
- [ ] **APPREG-05**: User can remove the app registration from a target tenant
|
||||
- [ ] **APPREG-06**: App clears cached tokens and sessions when app registration is removed
|
||||
- [x] **APPREG-06**: App clears cached tokens and sessions when app registration is removed
|
||||
|
||||
### Site Ownership
|
||||
|
||||
@@ -49,11 +49,11 @@ Requirements for v2.3 Tenant Management & Report Enhancements. Each maps to road
|
||||
| Requirement | Phase | Status |
|
||||
|-------------|-------|--------|
|
||||
| APPREG-01 | Phase 19 | Pending |
|
||||
| APPREG-02 | Phase 19 | Pending |
|
||||
| APPREG-03 | Phase 19 | Pending |
|
||||
| APPREG-02 | Phase 19 | Complete |
|
||||
| APPREG-03 | Phase 19 | Complete |
|
||||
| APPREG-04 | Phase 19 | Pending |
|
||||
| APPREG-05 | Phase 19 | Pending |
|
||||
| APPREG-06 | Phase 19 | Pending |
|
||||
| APPREG-06 | Phase 19 | Complete |
|
||||
| OWN-01 | Phase 18 | Complete |
|
||||
| OWN-02 | Phase 18 | Complete |
|
||||
| RPT-01 | Phase 17 | Complete |
|
||||
|
||||
@@ -117,7 +117,7 @@ Plans:
|
||||
3. Registration creates the Azure AD application, service principal, and grants all required API permissions in a single atomic operation — if any step fails, all partial changes are rolled back and the user sees a specific error explaining what failed and why
|
||||
4. A "Remove App" action in the profile dialog removes the Azure AD application registration from the target tenant
|
||||
5. After removal, all cached MSAL tokens and session state for that tenant are cleared, and subsequent operations require re-authentication
|
||||
**Plans:** 2 plans
|
||||
**Plans:** 1/2 plans executed
|
||||
Plans:
|
||||
- [ ] 19-01-PLAN.md — IAppRegistrationService + AppRegistrationResult model + TenantProfile.AppId + service implementation + unit tests
|
||||
- [ ] 19-02-PLAN.md — ViewModel RegisterApp/RemoveApp commands + XAML dialog UI + fallback panel + localization + VM tests
|
||||
@@ -133,4 +133,4 @@ Plans:
|
||||
| 16. Report Consolidation Toggle | v2.3 | 2/2 | Complete | 2026-04-09 |
|
||||
| 17. Group Expansion in HTML Reports | 2/2 | Complete | 2026-04-09 | — |
|
||||
| 18. Auto-Take Ownership | 2/2 | Complete | 2026-04-09 | — |
|
||||
| 19. App Registration & Removal | v2.3 | 0/2 | Planned | — |
|
||||
| 19. App Registration & Removal | 1/2 | In Progress| | — |
|
||||
|
||||
@@ -3,14 +3,14 @@ gsd_state_version: 1.0
|
||||
milestone: v2.3
|
||||
milestone_name: Tenant Management & Report Enhancements
|
||||
status: planning
|
||||
stopped_at: Completed 18-02-PLAN.md
|
||||
last_updated: "2026-04-09T12:37:02.452Z"
|
||||
stopped_at: Completed 19-01-PLAN.md
|
||||
last_updated: "2026-04-09T13:15:04.040Z"
|
||||
last_activity: 2026-04-09 — Roadmap created for v2.3 (phases 15-19)
|
||||
progress:
|
||||
total_phases: 5
|
||||
completed_phases: 4
|
||||
total_plans: 8
|
||||
completed_plans: 8
|
||||
total_plans: 10
|
||||
completed_plans: 9
|
||||
---
|
||||
|
||||
# Project State
|
||||
@@ -76,6 +76,9 @@ Decisions are logged in PROJECT.md Key Decisions table.
|
||||
- [Phase 18-auto-take-ownership]: AutoTakeOwnership ViewModel setter uses fire-and-forget pattern matching DataFolder
|
||||
- [Phase 18-auto-take-ownership]: Toggle read before scan loop (not in exception filter) — await in when clause unsupported; pre-read bool preserves semantics
|
||||
- [Phase 18-auto-take-ownership]: WasAutoElevated DataTrigger last in RowStyle.Triggers — amber wins over RiskLevel color
|
||||
- [Phase 19-app-registration-removal]: AppRegistrationService uses AppGraphClientFactory alias to disambiguate from Microsoft.Graph.GraphClientFactory
|
||||
- [Phase 19-app-registration-removal]: BuildRequiredResourceAccess declared internal to enable direct unit testing without live Graph calls
|
||||
- [Phase 19-app-registration-removal]: SharePoint AllSites.FullControl GUID marked LOW confidence — must be verified against live tenant
|
||||
|
||||
### Pending Todos
|
||||
|
||||
@@ -87,7 +90,7 @@ None.
|
||||
|
||||
## Session Continuity
|
||||
|
||||
Last session: 2026-04-09T12:34:21.588Z
|
||||
Stopped at: Completed 18-02-PLAN.md
|
||||
Last session: 2026-04-09T13:15:04.038Z
|
||||
Stopped at: Completed 19-01-PLAN.md
|
||||
Resume file: None
|
||||
Next step: `/gsd:plan-phase 15`
|
||||
|
||||
118
.planning/phases/19-app-registration-removal/19-01-SUMMARY.md
Normal file
118
.planning/phases/19-app-registration-removal/19-01-SUMMARY.md
Normal file
@@ -0,0 +1,118 @@
|
||||
---
|
||||
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
|
||||
Reference in New Issue
Block a user