Instrument elevation path to diagnose ineffective grants

A SharePoint admin reported the grant runs without a logged error yet the
account never appears as site-collection admin on Group/Teams sites. The
failure was invisible: ElevateAsync called ExecuteQueryAsync directly (no
enrichment/logging) and the coordinator only surfaced elevate failures on the
page, not to Serilog.

- Route the admin-endpoint ExecuteQuery through ExecuteQueryRetryHelper so a
  denial there is enriched (serverErrorType/httpStatus) and logged.
- Log the resolved login and SetSiteAdmin acceptance in OwnershipElevationService.
- Log elevate failures to Serilog in the coordinator.
- Add a post-elevation verify that reads CurrentUser.IsSiteAdmin on the target
  site so logs distinguish a failed/no-op grant from a scan failing for another
  reason. Diagnostic only; never throws into the operation flow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-02 14:35:48 +02:00
parent 1c36ea89d0
commit e4125c6643
2 changed files with 35 additions and 2 deletions
+23
View File
@@ -58,6 +58,10 @@ public class ElevationCoordinator : IElevationCoordinator
await ElevateAsync(siteUrl, ct); await ElevateAsync(siteUrl, ct);
_elevatedSites.Add(key); _elevatedSites.Add(key);
// Verify the grant actually took effect for this delegated token before retrying,
// so the logs distinguish "grant failed/no-op" from "scan still fails for another reason".
await VerifyAdminAsync(siteUrl, ct);
// Re-run once. The closure re-issues its loads; the now-granted admin right applies. // Re-run once. The closure re-issues its loads; the now-granted admin right applies.
return await operation(ct); return await operation(ct);
} }
@@ -88,12 +92,31 @@ public class ElevationCoordinator : IElevationCoordinator
} }
catch (Exception ex) when (ex is not OperationCanceledException) catch (Exception ex) when (ex is not OperationCanceledException)
{ {
Log.Error(ex, "Auto-elevate ownership failed for {Site}", siteUrl);
throw new InvalidOperationException( throw new InvalidOperationException(
$"Auto-elevate ownership failed for {siteUrl}. Granting site-collection admin requires " + $"Auto-elevate ownership failed for {siteUrl}. Granting site-collection admin requires " +
$"SharePoint tenant administrator rights on the signed-in account. ({ex.Message})", ex); $"SharePoint tenant administrator rights on the signed-in account. ({ex.Message})", ex);
} }
} }
// Reads the current user's site-admin flag on the target site right after elevation.
// Diagnostic only — never throws into the operation flow.
private async Task VerifyAdminAsync(string siteUrl, CancellationToken ct)
{
try
{
var ctx = await _sessionManager.GetOrCreateContextAsync(siteUrl, _session.CurrentProfile!, ct);
ctx.Load(ctx.Web.CurrentUser, u => u.LoginName, u => u.IsSiteAdmin);
await ctx.ExecuteQueryAsync();
Log.Information("Post-elevation check {Site}: user={Login} IsSiteAdmin={IsAdmin}",
siteUrl, ctx.Web.CurrentUser.LoginName, ctx.Web.CurrentUser.IsSiteAdmin);
}
catch (Exception ex)
{
Log.Warning("Post-elevation check failed for {Site}: {Error}", siteUrl, ex.Message);
}
}
// https://abcube.sharepoint.com/sites/Foo → https://abcube-admin.sharepoint.com // https://abcube.sharepoint.com/sites/Foo → https://abcube-admin.sharepoint.com
private static string BuildAdminUrl(string siteUrl) private static string BuildAdminUrl(string siteUrl)
{ {
+12 -2
View File
@@ -1,5 +1,7 @@
using Microsoft.Online.SharePoint.TenantAdministration; using Microsoft.Online.SharePoint.TenantAdministration;
using Microsoft.SharePoint.Client; using Microsoft.SharePoint.Client;
using Serilog;
using SharepointToolbox.Web.Core.Helpers;
using SharepointToolbox.Web.Services.Audit; using SharepointToolbox.Web.Services.Audit;
namespace SharepointToolbox.Web.Services; namespace SharepointToolbox.Web.Services;
@@ -15,12 +17,20 @@ public class OwnershipElevationService : IOwnershipElevationService
if (string.IsNullOrWhiteSpace(loginName)) if (string.IsNullOrWhiteSpace(loginName))
{ {
tenantAdminCtx.Load(tenantAdminCtx.Web.CurrentUser, u => u.LoginName); tenantAdminCtx.Load(tenantAdminCtx.Web.CurrentUser, u => u.LoginName);
await tenantAdminCtx.ExecuteQueryAsync(); await ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(tenantAdminCtx, null, ct);
loginName = tenantAdminCtx.Web.CurrentUser.LoginName; loginName = tenantAdminCtx.Web.CurrentUser.LoginName;
} }
Log.Information("SetSiteAdmin: granting {Login} site-collection admin on {Site} via admin endpoint {Admin}",
loginName, siteUrl, tenantAdminCtx.Url);
var tenant = new Tenant(tenantAdminCtx); var tenant = new Tenant(tenantAdminCtx);
tenant.SetSiteAdmin(siteUrl, loginName, isSiteAdmin: true); tenant.SetSiteAdmin(siteUrl, loginName, isSiteAdmin: true);
await tenantAdminCtx.ExecuteQueryAsync(); // Route through the enricher so a denial on the admin endpoint surfaces the real reason
// (and gets logged) instead of a bare 403 the caller has to guess at.
await ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(tenantAdminCtx, null, ct);
Log.Information("SetSiteAdmin call accepted for {Site} (login {Login})", siteUrl, loginName);
await _audit.LogAsync("ElevateOwnership", tenantAdminCtx.Url, new[] { siteUrl }, await _audit.LogAsync("ElevateOwnership", tenantAdminCtx.Url, new[] { siteUrl },
$"Site admin granted to {loginName}"); $"Site admin granted to {loginName}");
} }