Files

Dev 3479fff4c3 docs(18): complete phase research, validation, and plans

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

2026-04-09 14:17:00 +02:00

24 KiB

Raw Blame History

Phase 18: Auto-Take Ownership - Research

Researched: 2026-04-09 Domain: CSOM Tenant Administration, Access-Denied Detection, Settings Persistence, WPF MVVM Confidence: HIGH

<phase_requirements>

Phase Requirements

ID	Description	Research Support
OWN-01	User can enable/disable auto-take-ownership in application settings (global toggle, OFF by default)	AppSettings + SettingsService + SettingsRepository pattern directly applicable; toggle persisted alongside Lang/DataFolder
OWN-02	App automatically takes site collection admin ownership when encountering access denied during scans (when toggle is ON)	Tenant.SetSiteAdmin CSOM call requires tenant admin ClientContext; access denied caught as ServerUnauthorizedAccessException; retry pattern matches ExecuteQueryRetryHelper style
</phase_requirements>

Summary

Phase 18 adds two user-visible features: a global settings toggle (AutoTakeOwnership, default OFF) and automatic site collection admin elevation inside the permission scan loop when that toggle is ON.

The settings layer is already fully established: AppSettings is a simple POCO persisted by SettingsRepository (JSON, atomic tmp-then-move), surfaced through SettingsService, and bound in SettingsViewModel. Adding AutoTakeOwnership: bool to AppSettings plus a SetAutoTakeOwnershipAsync method to SettingsService follows the exact same pattern as SetLanguageAsync/SetDataFolderAsync.

The scan-time elevation requires calling Tenant.SetSiteAdmin on a tenant-admin ClientContext (pointed at the tenant admin site, e.g. https://tenant-admin.sharepoint.com), not the site being scanned. The existing SessionManager creates ClientContext instances per URL; a second context pointed at the admin URL can be requested via GetOrCreateContextAsync. Access denied during a scan manifests as Microsoft.SharePoint.Client.ServerUnauthorizedAccessException (a subclass of ServerException), thrown from ExecuteQueryAsync inside ExecuteQueryRetryHelper. The catch must be added in PermissionsService.ScanSiteAsync around the initial ExecuteQueryRetryAsync call that loads the web.

Visual differentiation in the results DataGrid requires a new boolean flag WasAutoElevated on PermissionEntry. Because PermissionEntry is a C# record, the flag is added as a new positional parameter with a default value of false for full backward compatibility. The DataGrid in PermissionsView.xaml already uses DataGrid.RowStyle triggers keyed on RiskLevel; a similar trigger keyed on WasAutoElevated can set a distinct background or column content.

Primary recommendation: Add AutoTakeOwnership to AppSettings, wire Tenant.SetSiteAdmin inside a try/catch (ServerUnauthorizedAccessException) in PermissionsService.ScanSiteAsync, and propagate a WasAutoElevated flag on PermissionEntry records returned for elevated sites.

Standard Stack

Core

Library	Version	Purpose	Why Standard
PnP.Framework	1.18.0	`Tenant.SetSiteAdmin` CSOM call	Already in project; Tenant class lives in `Microsoft.Online.SharePoint.TenantAdministration` namespace bundled with PnP.Framework
Microsoft.SharePoint.Client	(bundled w/ PnP.Framework)	`ServerUnauthorizedAccessException` type for access-denied detection	Already used throughout codebase
CommunityToolkit.Mvvm	8.4.2	`[ObservableProperty]` for settings toggle in ViewModel	Already in project

No New Packages Needed

All required libraries are already present. No new NuGet packages required for this phase.

Architecture Patterns

Recommended Project Structure

New files for this phase:

SharepointToolbox/
├── Core/Models/
│   └── AppSettings.cs              # ADD: AutoTakeOwnership bool property
│   └── PermissionEntry.cs          # ADD: WasAutoElevated bool parameter (default false)
├── Services/
│   └── SettingsService.cs          # ADD: SetAutoTakeOwnershipAsync method
│   └── IOwnershipElevationService.cs   # NEW: interface for testability
│   └── OwnershipElevationService.cs    # NEW: wraps Tenant.SetSiteAdmin
│   └── PermissionsService.cs       # MODIFY: catch ServerUnauthorizedAccessException, elevate, retry
├── ViewModels/Tabs/
│   └── SettingsViewModel.cs        # ADD: AutoTakeOwnership observable property + load/save
├── Views/Tabs/
│   └── SettingsView.xaml           # ADD: CheckBox for auto-take-ownership toggle
├── Localization/
│   └── Strings.resx                # ADD: new keys
│   └── Strings.fr.resx             # ADD: French translations
SharepointToolbox.Tests/
├── Services/
│   └── OwnershipElevationServiceTests.cs   # NEW
│   └── PermissionsServiceOwnershipTests.cs # NEW (tests scan-retry path)
├── ViewModels/
│   └── SettingsViewModelOwnershipTests.cs  # NEW

Pattern 1: AppSettings extension

AppSettings is a plain POCO. Add the property with a default:

// Source: existing Core/Models/AppSettings.cs pattern
public class AppSettings
{
    public string DataFolder { get; set; } = string.Empty;
    public string Lang { get; set; } = "en";
    public bool AutoTakeOwnership { get; set; } = false;  // OWN-01: OFF by default
}

SettingsRepository uses JsonSerializer.Deserialize with PropertyNameCaseInsensitive = true; the new field round-trips automatically. Old settings files without the field deserialize to false (the .NET default for bool), satisfying the "defaults to OFF" requirement.

Pattern 2: SettingsService extension

Follow the exact same pattern as SetLanguageAsync:

// Source: existing Services/SettingsService.cs pattern
public async Task SetAutoTakeOwnershipAsync(bool enabled)
{
    var settings = await _repository.LoadAsync();
    settings.AutoTakeOwnership = enabled;
    await _repository.SaveAsync(settings);
}

Pattern 3: SettingsViewModel observable property

Follow the pattern used for DataFolder (property setter triggers a service call):

// Source: existing ViewModels/Tabs/SettingsViewModel.cs pattern
private bool _autoTakeOwnership;
public bool AutoTakeOwnership
{
    get => _autoTakeOwnership;
    set
    {
        if (_autoTakeOwnership == value) return;
        _autoTakeOwnership = value;
        OnPropertyChanged();
        _ = _settingsService.SetAutoTakeOwnershipAsync(value);
    }
}

Load in LoadAsync():

_autoTakeOwnership = settings.AutoTakeOwnership;
OnPropertyChanged(nameof(AutoTakeOwnership));

Pattern 4: IOwnershipElevationService (new interface for testability)

// New file: Services/IOwnershipElevationService.cs
public interface IOwnershipElevationService
{
    /// <summary>
    /// Elevates the current user as site collection admin for the given site URL.
    /// Requires a ClientContext authenticated against the tenant admin URL.
    /// </summary>
    Task ElevateAsync(
        ClientContext tenantAdminCtx,
        string siteUrl,
        string loginName,
        CancellationToken ct);
}

Pattern 5: OwnershipElevationService — wrapping Tenant.SetSiteAdmin

Tenant.SetSiteAdmin is called on a Tenant object constructed from a tenant admin ClientContext (URL must be the -admin URL, e.g. https://contoso-admin.sharepoint.com). The call is a CSOM write operation that requires ExecuteQueryAsync.

// Source: Microsoft.Online.SharePoint.TenantAdministration.Tenant.SetSiteAdmin docs
// + PnP-Sites-Core TenantExtensions.cs usage pattern
using Microsoft.Online.SharePoint.TenantAdministration;

public class OwnershipElevationService : IOwnershipElevationService
{
    public async Task ElevateAsync(
        ClientContext tenantAdminCtx,
        string siteUrl,
        string loginName,
        CancellationToken ct)
    {
        var tenant = new Tenant(tenantAdminCtx);
        tenant.SetSiteAdmin(siteUrl, loginName, isSiteAdmin: true);
        await tenantAdminCtx.ExecuteQueryAsync();
    }
}

Critical: The ClientContext passed must be the tenant admin URL, NOT the site URL. The tenant admin URL is derived from the tenant URL stored in TenantProfile.TenantUrl. Derivation: if TenantUrl = "https://contoso.sharepoint.com", admin URL = "https://contoso-admin.sharepoint.com". If TenantUrl already contains -admin, use as-is.

The calling code in PermissionsViewModel.RunOperationAsync already has _currentProfile which holds TenantUrl. The admin URL can be derived with a simple string transformation.

Pattern 6: Access-denied detection and retry in PermissionsService

The scan loop in PermissionsViewModel.RunOperationAsync calls _permissionsService.ScanSiteAsync(ctx, ...). Access denied is thrown from ExecuteQueryRetryHelper.ExecuteQueryRetryAsync inside PermissionsService when the first CSOM round-trip executes.

Two integration strategies:

Option A (preferred): Service-level injection — inject IOwnershipElevationService and a Func<bool> (or settings accessor) into PermissionsService or pass via ScanOptions.

Option B: ViewModel-level catch — catch ServerUnauthorizedAccessException in PermissionsViewModel.RunOperationAsync around the ScanSiteAsync call, elevate via a separate service call, then retry ScanSiteAsync.

Recommendation: Option B — keeps PermissionsService unchanged (no new dependencies) and elevation logic lives in the ViewModel where the toggle state is accessible. This is consistent with the Phase 17 pattern where group resolution was also injected at the ViewModel layer.

// In PermissionsViewModel.RunOperationAsync — conceptual structure
foreach (var url in nonEmpty)
{
    ct.ThrowIfCancellationRequested();
    bool wasElevated = false;
    IReadOnlyList<PermissionEntry> siteEntries;

    try
    {
        var ctx = await _sessionManager.GetOrCreateContextAsync(profile, ct);
        siteEntries = await _permissionsService.ScanSiteAsync(ctx, scanOptions, progress, ct);
    }
    catch (ServerUnauthorizedAccessException) when (AutoTakeOwnership)
    {
        // Elevate and retry
        var adminUrl = DeriveAdminUrl(url);
        var adminProfile = profile with { TenantUrl = adminUrl };
        var adminCtx = await _sessionManager.GetOrCreateContextAsync(adminProfile, ct);
        await _ownershipService.ElevateAsync(adminCtx, url, currentUserLoginName, ct);

        var ctx = await _sessionManager.GetOrCreateContextAsync(profile, ct);
        siteEntries = await _permissionsService.ScanSiteAsync(ctx, scanOptions, progress, ct);
        wasElevated = true;
    }

    // Tag entries with elevation flag
    if (wasElevated)
        allEntries.AddRange(siteEntries.Select(e => e with { WasAutoElevated = true }));
    else
        allEntries.AddRange(siteEntries);
}

Pattern 7: PermissionEntry visual flag

PermissionEntry is a positional record. Add WasAutoElevated with a default:

// Existing record (simplified):
public record PermissionEntry(
    string ObjectType,
    string Title,
    string Url,
    bool HasUniquePermissions,
    string Users,
    string UserLogins,
    string PermissionLevels,
    string GrantedThrough,
    string PrincipalType,
    bool WasAutoElevated = false   // NEW — OWN-02 visual flag, default preserves backward compat
);

In PermissionsView.xaml, the DataGrid.RowStyle already uses DataTrigger on RiskLevel. Add a parallel trigger:

<!-- PermissionsView.xaml: existing RowStyle pattern -->
<DataTrigger Binding="{Binding WasAutoElevated}" Value="True">
    <Setter Property="Background" Value="#FFF9E6" />  <!-- light amber — "elevated" -->
</DataTrigger>

Alternatively, add a dedicated DataGridTextColumn for the flag (a small icon character or "Yes"/"No"):

<DataGridTextColumn Header="Auto-Elevated" Binding="{Binding WasAutoElevated}" Width="100" />

Pattern 8: Deriving tenant admin URL

// Helper: TenantProfile.TenantUrl → tenant admin URL
internal static string DeriveAdminUrl(string tenantUrl)
{
    // "https://contoso.sharepoint.com" → "https://contoso-admin.sharepoint.com"
    // Already an admin URL passes through unchanged.
    var uri = new Uri(tenantUrl.TrimEnd('/'));
    var host = uri.Host; // "contoso.sharepoint.com"
    if (host.Contains("-admin.sharepoint.com", StringComparison.OrdinalIgnoreCase))
        return tenantUrl;
    // Insert "-admin" before ".sharepoint.com"
    var adminHost = host.Replace(".sharepoint.com", "-admin.sharepoint.com",
        StringComparison.OrdinalIgnoreCase);
    return $"{uri.Scheme}://{adminHost}";
}

Anti-Patterns to Avoid

Elevate at every call: Do not call SetSiteAdmin if the scan already succeeds. Only elevate on ServerUnauthorizedAccessException.
Using the site URL as admin URL: Tenant constructor requires the tenant admin URL (-admin.sharepoint.com), not the site URL. Using the wrong URL causes its own 403.
Storing ClientContext references: Consistent with existing code — always request via GetOrCreateContextAsync, never cache the returned object.
Modifying PermissionsService signature for elevation: Keeps the service pure (no settings dependency). Elevation belongs at the ViewModel/orchestration layer.
Adding required parameter to PermissionEntry record: Must use = false default so all existing callsites remain valid without modification.

Don't Hand-Roll

Problem	Don't Build	Use Instead	Why
Elevating site admin	Custom REST call	`Tenant.SetSiteAdmin` via PnP.Framework	Already bundled; handles auth + CSOM write protocol
Detecting access denied	String-parsing exception messages	Catch `ServerUnauthorizedAccessException` directly	Typed exception subclass exists in CSOM SDK
Admin URL derivation	Complex URL parsing library	Simple `string.Replace` on `.sharepoint.com`	SharePoint Online admin URL is always a deterministic transformation
Settings persistence	New file/DB	Existing `SettingsRepository` (JSON, atomic write)	Already handles locking, tmp-file safety, JSON round-trip

Common Pitfalls

Pitfall 1: Wrong ClientContext for Tenant constructor

What goes wrong: new Tenant(siteCtx) where siteCtx points to a site URL (not admin URL) silently creates a Tenant object but throws ServerUnauthorizedAccessException on ExecuteQueryAsync. Why it happens: Tenant-level APIs require the -admin.sharepoint.com host. How to avoid: Always derive the admin URL before requesting a context for elevation. The DeriveAdminUrl helper handles this. Warning signs: SetSiteAdmin call succeeds (no compile error) but throws 403 at runtime.

Pitfall 2: Infinite retry loop

What goes wrong: Catching ServerUnauthorizedAccessException and retrying without tracking the retry count can loop if elevation itself fails (e.g., user is not a tenant admin). Why it happens: The when (AutoTakeOwnership) guard prevents the catch when the toggle is OFF, but a second failure after elevation is not distinguished from the first. How to avoid: The catch block only runs once per site (it's not a loop). After elevation, the second ScanSiteAsync call is outside the catch. If the second call also throws, it propagates normally. Warning signs: Test with a non-admin account — elevation will throw, and that exception must surface to the user as a status message.

Pitfall 3: `PermissionEntry` record positional parameter order

What goes wrong: Inserting WasAutoElevated at a position other than last breaks all existing new PermissionEntry(...) callsites that use positional syntax. Why it happens: C# positional records require arguments in declaration order. How to avoid: Always append new parameters at the end with a default value (= false). Warning signs: Compile errors across PermissionsService.cs and all test files.

Pitfall 4: `ServerUnauthorizedAccessException` vs `WebException`/`HttpException`

What goes wrong: Some access-denied scenarios in CSOM produce WebException (network-level 403) rather than ServerUnauthorizedAccessException (CSOM-level). Why it happens: Different CSOM layers surface different exception types depending on where auth fails. How to avoid: Catch both: catch (Exception ex) when ((ex is ServerUnauthorizedAccessException || IsAccessDeniedWebException(ex)) && AutoTakeOwnership). The existing ExecuteQueryRetryHelper.IsThrottleException pattern shows this style. Warning signs: Access denied on certain sites silently bypasses the catch block.

Pitfall 5: Localization completeness test will fail

What goes wrong: Adding new keys to Strings.resx without adding them to Strings.fr.resx causes the LocaleCompletenessTests to fail. Why it happens: The existing test (LocaleCompletenessTests.cs) verifies all English keys exist in French. How to avoid: Always add French translations for every new key in the same task.

Code Examples

Tenant.SetSiteAdmin call pattern

// Source: Microsoft.Online.SharePoint.TenantAdministration.Tenant.SetSiteAdmin docs
// + PnP-Sites-Core TenantExtensions.cs: tenant.SetSiteAdmin(siteUrlString, admin.LoginName, true)
using Microsoft.Online.SharePoint.TenantAdministration;

var tenant = new Tenant(tenantAdminCtx);  // ctx must point to -admin.sharepoint.com
tenant.SetSiteAdmin(siteUrl, loginName, isSiteAdmin: true);
await tenantAdminCtx.ExecuteQueryAsync();

ServerUnauthorizedAccessException detection

// Source: Microsoft.SharePoint.Client.ServerUnauthorizedAccessException (CSOM SDK)
// Inherits from ServerException — typed catch is reliable
catch (Microsoft.SharePoint.Client.ServerUnauthorizedAccessException ex)
{
    // Access denied: user is not a site admin
}

PermissionEntry with default parameter

// Source: existing Core/Models/PermissionEntry.cs pattern
public record PermissionEntry(
    string ObjectType,
    string Title,
    string Url,
    bool   HasUniquePermissions,
    string Users,
    string UserLogins,
    string PermissionLevels,
    string GrantedThrough,
    string PrincipalType,
    bool   WasAutoElevated = false   // appended with default — zero callsite breakage
);

AppSettings with new field

// Source: existing Core/Models/AppSettings.cs pattern
public class AppSettings
{
    public string DataFolder { get; set; } = string.Empty;
    public string Lang { get; set; } = "en";
    public bool AutoTakeOwnership { get; set; } = false;
}

SettingsViewModel AutoTakeOwnership property

// Source: existing ViewModels/Tabs/SettingsViewModel.cs DataFolder pattern
private bool _autoTakeOwnership;
public bool AutoTakeOwnership
{
    get => _autoTakeOwnership;
    set
    {
        if (_autoTakeOwnership == value) return;
        _autoTakeOwnership = value;
        OnPropertyChanged();
        _ = _settingsService.SetAutoTakeOwnershipAsync(value);
    }
}

State of the Art

Old Approach	Current Approach	When Changed	Impact
Manual PowerShell `Set-PnPTenantSite -Owners`	`Tenant.SetSiteAdmin` via CSOM in-process	Available since SPO CSOM	Can call programmatically without separate shell
No per-site access-denied recovery	Auto-elevate + retry on `ServerUnauthorizedAccessException`	Phase 18 (new)	Scans no longer block on access-denied sites when toggle is ON

Validation Architecture

Test Framework

Property	Value
Framework	xUnit 2.9.3
Config file	none — implicit discovery
Quick run command	`dotnet test SharepointToolbox.Tests --filter "Category=Unit" --no-build`
Full suite command	`dotnet test SharepointToolbox.Tests --no-build`

Phase Requirements → Test Map

Req ID	Behavior	Test Type	Automated Command	File Exists?
OWN-01	`AutoTakeOwnership` defaults to `false` in `AppSettings`	unit	`dotnet test --filter "FullyQualifiedName~SettingsServiceTests"`	❌ Wave 0
OWN-01	Setting persists to JSON and round-trips via `SettingsRepository`	unit	`dotnet test --filter "FullyQualifiedName~SettingsServiceTests"`	❌ Wave 0
OWN-01	`SettingsViewModel.AutoTakeOwnership` loads from settings on `LoadAsync`	unit	`dotnet test --filter "FullyQualifiedName~SettingsViewModelOwnershipTests"`	❌ Wave 0
OWN-01	Setting toggle to `true` calls `SetAutoTakeOwnershipAsync`	unit	`dotnet test --filter "FullyQualifiedName~SettingsViewModelOwnershipTests"`	❌ Wave 0
OWN-02	When toggle OFF, `ServerUnauthorizedAccessException` propagates normally (no elevation)	unit	`dotnet test --filter "FullyQualifiedName~PermissionsViewModelOwnershipTests"`	❌ Wave 0
OWN-02	When toggle ON and access denied, `ElevateAsync` is called once then `ScanSiteAsync` retried	unit	`dotnet test --filter "FullyQualifiedName~PermissionsViewModelOwnershipTests"`	❌ Wave 0
OWN-02	Successful (non-denied) scans never call `ElevateAsync`	unit	`dotnet test --filter "FullyQualifiedName~PermissionsViewModelOwnershipTests"`	❌ Wave 0
OWN-02	`PermissionEntry.WasAutoElevated` defaults to `false`; elevated sites return `true`	unit	`dotnet test --filter "FullyQualifiedName~PermissionsViewModelOwnershipTests"`	❌ Wave 0
OWN-02	`WasAutoElevated = false` on `PermissionEntry` does not break any existing test	unit	`dotnet test SharepointToolbox.Tests --no-build`	✅ existing

Sampling Rate

Per task commit: dotnet test SharepointToolbox.Tests --filter "Category=Unit" --no-build
Per wave merge: dotnet test SharepointToolbox.Tests --no-build
Phase gate: Full suite green before /gsd:verify-work

Wave 0 Gaps

SharepointToolbox.Tests/Services/OwnershipElevationServiceTests.cs — covers IOwnershipElevationService contract and ElevateAsync behavior
SharepointToolbox.Tests/ViewModels/SettingsViewModelOwnershipTests.cs — covers OWN-01 toggle persistence
SharepointToolbox.Tests/ViewModels/PermissionsViewModelOwnershipTests.cs — covers OWN-02 scan elevation + retry logic

Sources

Primary (HIGH confidence)

Microsoft Docs — Tenant.SetSiteAdmin method signature: https://learn.microsoft.com/en-us/previous-versions/office/sharepoint-csom/dn140313(v=office.15)
Microsoft Docs — ServerUnauthorizedAccessException class: https://learn.microsoft.com/en-us/dotnet/api/microsoft.sharepoint.client.serverunauthorizedaccessexception?view=sharepoint-csom
Codebase — AppSettings, SettingsRepository, SettingsService, SettingsViewModel (read directly)
Codebase — PermissionEntry record, PermissionsService.ScanSiteAsync (read directly)
Codebase — ExecuteQueryRetryHelper exception handling pattern (read directly)
Codebase — PermissionsViewModel.RunOperationAsync scan loop (read directly)

Secondary (MEDIUM confidence)

PnP-Sites-Core TenantExtensions.cs — tenant.SetSiteAdmin(siteUrlString, admin.LoginName, true) call pattern (via WebFetch on GitHub)
PnP Core SDK docs — SetSiteCollectionAdmins requires tenant admin scope: https://pnp.github.io/pnpcore/using-the-sdk/admin-sharepoint-security.html

Metadata

Confidence breakdown:

Standard stack: HIGH — all libraries already in project; Tenant.SetSiteAdmin confirmed via official MS docs
Architecture: HIGH — patterns directly observed in existing codebase (settings, scan loop, exception helper)
Pitfalls: HIGH — wrong-URL and record-parameter pitfalls are deterministic; access-denied exception type confirmed via MS docs

Research date: 2026-04-09 Valid until: 2026-07-09 (stable APIs)

24 KiB Raw Blame History