chore: complete v1.0 milestone
All checks were successful
Release zip package / release (push) Successful in 10s
All checks were successful
Release zip package / release (push) Successful in 10s
Archive 5 phases (36 plans) to milestones/v1.0-phases/. Archive roadmap, requirements, and audit to milestones/. Evolve PROJECT.md with shipped state and validated requirements. Collapse ROADMAP.md to one-line milestone summary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Dev
266
.planning/milestones/v1.0-phases/01-foundation/01-04-PLAN.md
Normal file
266
.planning/milestones/v1.0-phases/01-foundation/01-04-PLAN.md
Normal file
@@ -0,0 +1,266 @@
|
||||
---
|
||||
phase: 01-foundation
|
||||
plan: 04
|
||||
type: execute
|
||||
wave: 4
|
||||
depends_on:
|
||||
- 01-02
|
||||
- 01-03
|
||||
files_modified:
|
||||
- SharepointToolbox/Infrastructure/Auth/MsalClientFactory.cs
|
||||
- SharepointToolbox/Services/SessionManager.cs
|
||||
- SharepointToolbox.Tests/Auth/MsalClientFactoryTests.cs
|
||||
- SharepointToolbox.Tests/Auth/SessionManagerTests.cs
|
||||
autonomous: true
|
||||
requirements:
|
||||
- FOUND-03
|
||||
- FOUND-04
|
||||
must_haves:
|
||||
truths:
|
||||
- "MsalClientFactory creates one IPublicClientApplication per ClientId — never shares instances across tenants"
|
||||
- "MsalCacheHelper persists token cache to %AppData%\\SharepointToolbox\\auth\\msal_{clientId}.cache"
|
||||
- "SessionManager.GetOrCreateContextAsync returns a cached ClientContext on second call without interactive login"
|
||||
- "SessionManager.ClearSessionAsync removes MSAL accounts and disposes ClientContext for the specified tenant"
|
||||
- "SessionManager is the only class in the codebase holding ClientContext instances"
|
||||
artifacts:
|
||||
- path: "SharepointToolbox/Infrastructure/Auth/MsalClientFactory.cs"
|
||||
provides: "Per-ClientId IPublicClientApplication with MsalCacheHelper"
|
||||
contains: "MsalCacheHelper"
|
||||
- path: "SharepointToolbox/Services/SessionManager.cs"
|
||||
provides: "Singleton holding all ClientContext instances and auth state"
|
||||
exports: ["GetOrCreateContextAsync", "ClearSessionAsync", "IsAuthenticated"]
|
||||
key_links:
|
||||
- from: "SharepointToolbox/Services/SessionManager.cs"
|
||||
to: "SharepointToolbox/Infrastructure/Auth/MsalClientFactory.cs"
|
||||
via: "Injected dependency — SessionManager calls MsalClientFactory.GetOrCreateAsync(clientId)"
|
||||
pattern: "GetOrCreateAsync"
|
||||
- from: "SharepointToolbox/Services/SessionManager.cs"
|
||||
to: "PnP.Framework AuthenticationManager"
|
||||
via: "CreateWithInteractiveLogin using MSAL PCA"
|
||||
pattern: "AuthenticationManager"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Build the authentication layer: MsalClientFactory (per-tenant MSAL client with persistent cache) and SessionManager (singleton holding all live ClientContext instances). This is the security-critical component — one IPublicClientApplication per ClientId, never shared.
|
||||
|
||||
Purpose: Every SharePoint operation in Phases 2-4 goes through SessionManager. Getting the per-tenant isolation and token cache correct now prevents auth token bleed between client tenants — a critical security property for MSP use.
|
||||
Output: MsalClientFactory + SessionManager + unit tests validating per-tenant isolation.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@C:/Users/SebastienQUEROL/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@C:/Users/SebastienQUEROL/.claude/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/PROJECT.md
|
||||
@.planning/phases/01-foundation/01-CONTEXT.md
|
||||
@.planning/phases/01-foundation/01-RESEARCH.md
|
||||
@.planning/phases/01-foundation/01-02-SUMMARY.md
|
||||
@.planning/phases/01-foundation/01-03-SUMMARY.md
|
||||
|
||||
<interfaces>
|
||||
<!-- From Core/Models/TenantProfile.cs (plan 01-02) -->
|
||||
```csharp
|
||||
public class TenantProfile
|
||||
{
|
||||
public string Name { get; set; }
|
||||
public string TenantUrl { get; set; }
|
||||
public string ClientId { get; set; }
|
||||
}
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 1: MsalClientFactory — per-ClientId PCA with MsalCacheHelper</name>
|
||||
<files>
|
||||
SharepointToolbox/Infrastructure/Auth/MsalClientFactory.cs,
|
||||
SharepointToolbox.Tests/Auth/MsalClientFactoryTests.cs
|
||||
</files>
|
||||
<behavior>
|
||||
- Test: GetOrCreateAsync("clientA") and GetOrCreateAsync("clientA") return the same instance (no duplicate creation)
|
||||
- Test: GetOrCreateAsync("clientA") and GetOrCreateAsync("clientB") return different instances (per-tenant isolation)
|
||||
- Test: Concurrent calls to GetOrCreateAsync with same clientId do not create duplicate instances (SemaphoreSlim)
|
||||
- Test: Cache directory path resolves to %AppData%\SharepointToolbox\auth\ (not a hardcoded path)
|
||||
</behavior>
|
||||
<action>
|
||||
Create `Infrastructure/Auth/` directory.
|
||||
|
||||
**MsalClientFactory.cs** — implement exactly as per research Pattern 3:
|
||||
```csharp
|
||||
namespace SharepointToolbox.Infrastructure.Auth;
|
||||
|
||||
public class MsalClientFactory
|
||||
{
|
||||
private readonly Dictionary<string, IPublicClientApplication> _clients = new();
|
||||
private readonly SemaphoreSlim _lock = new(1, 1);
|
||||
private readonly string _cacheDir = Path.Combine(
|
||||
Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData),
|
||||
"SharepointToolbox", "auth");
|
||||
|
||||
public async Task<IPublicClientApplication> GetOrCreateAsync(string clientId)
|
||||
{
|
||||
await _lock.WaitAsync();
|
||||
try
|
||||
{
|
||||
if (_clients.TryGetValue(clientId, out var existing))
|
||||
return existing;
|
||||
|
||||
var storageProps = new StorageCreationPropertiesBuilder(
|
||||
$"msal_{clientId}.cache", _cacheDir)
|
||||
.Build();
|
||||
|
||||
var pca = PublicClientApplicationBuilder
|
||||
.Create(clientId)
|
||||
.WithDefaultRedirectUri()
|
||||
.WithLegacyCacheCompatibility(false)
|
||||
.Build();
|
||||
|
||||
var helper = await MsalCacheHelper.CreateAsync(storageProps);
|
||||
helper.RegisterCache(pca.UserTokenCache);
|
||||
|
||||
_clients[clientId] = pca;
|
||||
return pca;
|
||||
}
|
||||
finally { _lock.Release(); }
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**MsalClientFactoryTests.cs** — Replace stub. Tests for per-ClientId isolation and idempotency.
|
||||
Since MsalCacheHelper creates real files, tests must use a temp directory and clean up.
|
||||
Use `[Trait("Category", "Unit")]` on all tests.
|
||||
Mock or subclass `MsalClientFactory` for the concurrent test to avoid real MSAL overhead.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd "C:\Users\dev\Documents\projets\Sharepoint" && dotnet test SharepointToolbox.Tests/SharepointToolbox.Tests.csproj --filter "FullyQualifiedName~MsalClientFactoryTests" 2>&1 | tail -10</automated>
|
||||
</verify>
|
||||
<done>MsalClientFactoryTests pass. Different clientIds produce different instances. Same clientId produces same instance on second call.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
<name>Task 2: SessionManager — singleton ClientContext holder</name>
|
||||
<files>
|
||||
SharepointToolbox/Services/SessionManager.cs,
|
||||
SharepointToolbox.Tests/Auth/SessionManagerTests.cs
|
||||
</files>
|
||||
<behavior>
|
||||
- Test: IsAuthenticated(tenantUrl) returns false before any authentication
|
||||
- Test: After GetOrCreateContextAsync succeeds, IsAuthenticated(tenantUrl) returns true
|
||||
- Test: ClearSessionAsync removes authentication state for the specified tenant
|
||||
- Test: ClearSessionAsync on unknown tenantUrl does not throw (idempotent)
|
||||
- Test: ClientContext is disposed on ClearSessionAsync (verify via mock/wrapper)
|
||||
- Test: GetOrCreateContextAsync throws ArgumentException for null/empty tenantUrl or clientId
|
||||
</behavior>
|
||||
<action>
|
||||
**SessionManager.cs** — singleton, owns all ClientContext instances:
|
||||
```csharp
|
||||
namespace SharepointToolbox.Services;
|
||||
|
||||
public class SessionManager
|
||||
{
|
||||
private readonly MsalClientFactory _msalFactory;
|
||||
private readonly Dictionary<string, ClientContext> _contexts = new();
|
||||
private readonly SemaphoreSlim _lock = new(1, 1);
|
||||
|
||||
public SessionManager(MsalClientFactory msalFactory)
|
||||
{
|
||||
_msalFactory = msalFactory;
|
||||
}
|
||||
|
||||
public bool IsAuthenticated(string tenantUrl) =>
|
||||
_contexts.ContainsKey(NormalizeUrl(tenantUrl));
|
||||
|
||||
/// <summary>
|
||||
/// Returns existing ClientContext or creates a new one via interactive MSAL login.
|
||||
/// Only SessionManager holds ClientContext instances — never return to callers for storage.
|
||||
/// </summary>
|
||||
public async Task<ClientContext> GetOrCreateContextAsync(
|
||||
TenantProfile profile,
|
||||
CancellationToken ct = default)
|
||||
{
|
||||
ArgumentException.ThrowIfNullOrEmpty(profile.TenantUrl);
|
||||
ArgumentException.ThrowIfNullOrEmpty(profile.ClientId);
|
||||
|
||||
var key = NormalizeUrl(profile.TenantUrl);
|
||||
|
||||
await _lock.WaitAsync(ct);
|
||||
try
|
||||
{
|
||||
if (_contexts.TryGetValue(key, out var existing))
|
||||
return existing;
|
||||
|
||||
var pca = await _msalFactory.GetOrCreateAsync(profile.ClientId);
|
||||
var authManager = AuthenticationManager.CreateWithInteractiveLogin(
|
||||
profile.ClientId,
|
||||
(url, port) =>
|
||||
{
|
||||
// WAM/browser-based interactive login
|
||||
return pca.AcquireTokenInteractive(
|
||||
new[] { "https://graph.microsoft.com/.default" })
|
||||
.ExecuteAsync(ct);
|
||||
});
|
||||
|
||||
var ctx = await authManager.GetContextAsync(profile.TenantUrl);
|
||||
_contexts[key] = ctx;
|
||||
return ctx;
|
||||
}
|
||||
finally { _lock.Release(); }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Clears MSAL accounts and disposes the ClientContext for the given tenant.
|
||||
/// Called by "Clear Session" button and on tenant profile deletion.
|
||||
/// </summary>
|
||||
public async Task ClearSessionAsync(string tenantUrl)
|
||||
{
|
||||
var key = NormalizeUrl(tenantUrl);
|
||||
await _lock.WaitAsync();
|
||||
try
|
||||
{
|
||||
if (_contexts.TryGetValue(key, out var ctx))
|
||||
{
|
||||
ctx.Dispose();
|
||||
_contexts.Remove(key);
|
||||
}
|
||||
}
|
||||
finally { _lock.Release(); }
|
||||
}
|
||||
|
||||
private static string NormalizeUrl(string url) =>
|
||||
url.TrimEnd('/').ToLowerInvariant();
|
||||
}
|
||||
```
|
||||
|
||||
Note on PnP AuthenticationManager: The exact API for `CreateWithInteractiveLogin` with MSAL PCA may vary in PnP.Framework 1.18.0. The implementation above is a skeleton — executor should verify the PnP API surface and adjust accordingly. The key invariant is: `MsalClientFactory.GetOrCreateAsync` is called first, then PnP creates the context using the returned PCA. Do NOT call `PublicClientApplicationBuilder.Create` directly in SessionManager.
|
||||
|
||||
**SessionManagerTests.cs** — Replace stub. Use Moq to mock `MsalClientFactory`.
|
||||
Test `IsAuthenticated`, `ClearSessionAsync` idempotency, and argument validation.
|
||||
Interactive login cannot be tested in unit tests — mark `GetOrCreateContextAsync_CreatesContext` as `[Fact(Skip = "Requires interactive MSAL — integration test only")]`.
|
||||
All other tests in `[Trait("Category", "Unit")]`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd "C:\Users\dev\Documents\projets\Sharepoint" && dotnet test SharepointToolbox.Tests/SharepointToolbox.Tests.csproj --filter "FullyQualifiedName~SessionManagerTests" 2>&1 | tail -10</automated>
|
||||
</verify>
|
||||
<done>SessionManagerTests pass (interactive login test skipped). IsAuthenticated, ClearSessionAsync, and argument validation tests are green. SessionManager is the only holder of ClientContext.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
- `dotnet test --filter "Category=Unit"` passes
|
||||
- MsalClientFactory._clients dictionary holds one entry per unique clientId
|
||||
- SessionManager.ClearSessionAsync calls ctx.Dispose() (verified via test)
|
||||
- No class outside SessionManager stores a ClientContext reference
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
Auth layer unit tests green. Per-tenant isolation (one PCA per ClientId, one context per tenantUrl) confirmed by tests. SessionManager is the single source of truth for authenticated connections.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/01-foundation/01-04-SUMMARY.md`
|
||||
</output>
|
||||
Reference in New Issue
Block a user