From e4125c6643cde95b1585339c65e27720ee4a8c42 Mon Sep 17 00:00:00 2001 From: kawa Date: Tue, 2 Jun 2026 14:35:48 +0200 Subject: [PATCH] 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) --- Services/ElevationCoordinator.cs | 23 +++++++++++++++++++++++ Services/OwnershipElevationService.cs | 14 ++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/Services/ElevationCoordinator.cs b/Services/ElevationCoordinator.cs index db7b7c2..3224541 100644 --- a/Services/ElevationCoordinator.cs +++ b/Services/ElevationCoordinator.cs @@ -58,6 +58,10 @@ public class ElevationCoordinator : IElevationCoordinator await ElevateAsync(siteUrl, ct); _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. return await operation(ct); } @@ -88,12 +92,31 @@ public class ElevationCoordinator : IElevationCoordinator } catch (Exception ex) when (ex is not OperationCanceledException) { + Log.Error(ex, "Auto-elevate ownership failed for {Site}", siteUrl); throw new InvalidOperationException( $"Auto-elevate ownership failed for {siteUrl}. Granting site-collection admin requires " + $"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 private static string BuildAdminUrl(string siteUrl) { diff --git a/Services/OwnershipElevationService.cs b/Services/OwnershipElevationService.cs index 46c0bd2..7fb7be9 100644 --- a/Services/OwnershipElevationService.cs +++ b/Services/OwnershipElevationService.cs @@ -1,5 +1,7 @@ using Microsoft.Online.SharePoint.TenantAdministration; using Microsoft.SharePoint.Client; +using Serilog; +using SharepointToolbox.Web.Core.Helpers; using SharepointToolbox.Web.Services.Audit; namespace SharepointToolbox.Web.Services; @@ -15,12 +17,20 @@ public class OwnershipElevationService : IOwnershipElevationService if (string.IsNullOrWhiteSpace(loginName)) { tenantAdminCtx.Load(tenantAdminCtx.Web.CurrentUser, u => u.LoginName); - await tenantAdminCtx.ExecuteQueryAsync(); + await ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(tenantAdminCtx, null, ct); 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); 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 }, $"Site admin granted to {loginName}"); }