From 4846915c8079672619bd4db264c03b9971bd128d Mon Sep 17 00:00:00 2001 From: Dev Date: Tue, 7 Apr 2026 11:00:54 +0200 Subject: [PATCH] fix(site-list): fix parsing error and double-auth in SiteListService - Replace GetSitePropertiesFromSharePoint("", true) with modern GetSitePropertiesFromSharePointByFilters using null StartIndex - Use ctx.Clone(adminUrl) instead of creating new AuthenticationManager for admin URL, eliminating second browser auth prompt Resolves: UAT issue "Must specify valid information for parsing in the string" Co-Authored-By: Claude Opus 4.6 (1M context) --- .planning/debug/site-picker-parsing-error.md | 89 +++++++++++++++++++ SharepointToolbox/Services/SiteListService.cs | 56 +++++++----- 2 files changed, 125 insertions(+), 20 deletions(-) create mode 100644 .planning/debug/site-picker-parsing-error.md diff --git a/.planning/debug/site-picker-parsing-error.md b/.planning/debug/site-picker-parsing-error.md new file mode 100644 index 0000000..a081b8b --- /dev/null +++ b/.planning/debug/site-picker-parsing-error.md @@ -0,0 +1,89 @@ +--- +status: awaiting_human_verify +trigger: "SitePickerDialog shows 'Must specify valid information for parsing in the string' error when trying to load sites after a successful tenant connection." +created: 2026-04-07T00:00:00Z +updated: 2026-04-07T00:00:00Z +--- + +## Current Focus + +hypothesis: ROOT CAUSE CONFIRMED — two bugs in SiteListService.GetSitesAsync +test: code reading confirmed via PnP source +expecting: fixing both issues will resolve the error +next_action: apply fix to SiteListService.cs + +## Symptoms + +expected: After connecting to a SharePoint tenant (https://contoso.sharepoint.com format), clicking "Select Sites" opens SitePickerDialog and loads the list of tenant sites. +actual: SitePickerDialog opens but shows error "Must specify valid information for parsing in the string" instead of loading sites. +errors: "Must specify valid information for parsing in the string" — this is an ArgumentException thrown by CSOM when it tries to parse an empty string as a site URL cursor +reproduction: 1) Launch app 2) Add profile with valid tenant URL 3) Connect 4) Authenticate 5) Click Select Sites 6) Error appears in StatusText +started: First time testing this flow after Phase 6 wiring was added. + +## Eliminated + +- hypothesis: Error comes from PnP's AuthenticationManager.GetContextAsync URI parsing + evidence: GetContextAsync line 1090 does new Uri(siteUrl) which is valid for "https://contoso-admin.sharepoint.com" + timestamp: 2026-04-07 + +- hypothesis: Error from MSAL constructing auth URL with empty component + evidence: MSAL uses organizations authority or tenant-specific, both valid; no empty strings involved + timestamp: 2026-04-07 + +- hypothesis: UriFormatException from new Uri("") in our own code + evidence: No Uri.Parse or new Uri() calls in SiteListService or SessionManager + timestamp: 2026-04-07 + +## Evidence + +- timestamp: 2026-04-07 + checked: PnP Framework 1.18.0 GetContextAsync source (line 1090) + found: Calls new Uri(siteUrl) — valid for admin URL + implication: Error not from GetContextAsync itself + +- timestamp: 2026-04-07 + checked: PnP TenantExtensions.GetSiteCollections source + found: Uses GetSitePropertiesFromSharePointByFilters with StartIndex = null (for first page); OLD commented-out approach used GetSitePropertiesFromSharePoint(null, includeDetail) — note: null, not "" + implication: SiteListService passes "" which is wrong — should be null for first page + +- timestamp: 2026-04-07 + checked: Error message "Must specify valid information for parsing in the string" + found: This is ArgumentException thrown by Enum.Parse or string cursor parsing when given "" (empty string); CSOM's GetSitePropertiesFromSharePoint internally parses the startIndex string as a URL/cursor; passing "" triggers parse failure + implication: Direct cause of exception confirmed + +- timestamp: 2026-04-07 + checked: How PnP creates admin context from regular context + found: PnP uses clientContext.Clone(adminSiteUrl) — clones existing authenticated context to admin URL without triggering new auth flow + implication: SiteListService creates a SECOND AuthenticationManager and triggers second interactive login unnecessarily; should use Clone instead + +## Resolution + +root_cause: | + SiteListService.GetSitesAsync has two bugs: + + BUG 1 (direct cause of error): Line 50 calls tenant.GetSitePropertiesFromSharePoint("", true) + with empty string "". CSOM expects null for the first page (no previous cursor), not "". + Passing "" causes CSOM to attempt parsing it as a URL cursor, throwing + ArgumentException: "Must specify valid information for parsing in the string." + + BUG 2 (design problem): GetSitesAsync creates a separate TenantProfile for the admin URL + and calls SessionManager.GetOrCreateContextAsync(adminProfile) which creates a NEW + AuthenticationManager with interactive login. This triggers a SECOND browser auth flow + just to access the admin URL. The correct approach is to clone the existing authenticated + context to the admin URL using clientContext.Clone(adminUrl), which reuses the same tokens. + +fix: | + 1. Replace GetOrCreateContextAsync(adminProfile) with GetOrCreateContextAsync(profile) to + get the regular context, then clone it to the admin URL. + 2. Replace GetSitePropertiesFromSharePointByFilters with proper pagination (StartIndex=null). + + The admin URL context is obtained via: adminCtx = ctx.Clone(adminUrl) + The site listing uses: GetSitePropertiesFromSharePointByFilters with proper filter object. + +verification: | + Build succeeds (0 errors). 144 tests pass, 0 failures. + Fix addresses both root causes: + 1. No longer calls GetOrCreateContextAsync with admin profile — uses Clone() instead + 2. Uses GetSitePropertiesFromSharePointByFilters (modern API) instead of GetSitePropertiesFromSharePoint("") +files_changed: + - SharepointToolbox/Services/SiteListService.cs diff --git a/SharepointToolbox/Services/SiteListService.cs b/SharepointToolbox/Services/SiteListService.cs index 2311e97..b614116 100644 --- a/SharepointToolbox/Services/SiteListService.cs +++ b/SharepointToolbox/Services/SiteListService.cs @@ -27,18 +27,12 @@ public class SiteListService : ISiteListService ct.ThrowIfCancellationRequested(); progress.Report(OperationProgress.Indeterminate("Loading sites...")); - var adminUrl = DeriveAdminUrl(profile.TenantUrl); - var adminProfile = new TenantProfile - { - Name = profile.Name, - TenantUrl = adminUrl, - ClientId = profile.ClientId - }; - - ClientContext adminCtx; + // Obtain the already-authenticated context for the tenant URL, then clone it to + // the admin URL. Cloning reuses the existing token — no second interactive login. + ClientContext ctx; try { - adminCtx = await _sessionManager.GetOrCreateContextAsync(adminProfile, ct); + ctx = await _sessionManager.GetOrCreateContextAsync(profile, ct); } catch (ServerException ex) when (ex.Message.Contains("Access denied", StringComparison.OrdinalIgnoreCase)) { @@ -46,19 +40,41 @@ public class SiteListService : ISiteListService "Site listing requires SharePoint administrator permissions. Connect with an admin account.", ex); } + var adminUrl = DeriveAdminUrl(profile.TenantUrl); + using var adminCtx = ctx.Clone(adminUrl); + + var results = new List(); var tenant = new Tenant(adminCtx); - var siteProps = tenant.GetSitePropertiesFromSharePoint("", true); - adminCtx.Load(siteProps); - await adminCtx.ExecuteQueryAsync(); + SPOSitePropertiesEnumerable? batch = null; - ct.ThrowIfCancellationRequested(); + // Paginate through all site collections using GetSitePropertiesFromSharePointByFilters. + // StartIndex = null on the first call; subsequent calls use NextStartIndexFromSharePoint. + while (batch == null || batch.NextStartIndexFromSharePoint != null) + { + ct.ThrowIfCancellationRequested(); - return siteProps - .Where(s => s.Status == "Active" - && !s.Url.Contains("-my.sharepoint.com", StringComparison.OrdinalIgnoreCase)) - .Select(s => new SiteInfo(s.Url, s.Title)) - .OrderBy(s => s.Url) - .ToList(); + var filter = new SPOSitePropertiesEnumerableFilter + { + IncludePersonalSite = PersonalSiteFilter.UseServerDefault, + StartIndex = batch?.NextStartIndexFromSharePoint, + IncludeDetail = true + }; + + batch = tenant.GetSitePropertiesFromSharePointByFilters(filter); + adminCtx.Load(batch); + await adminCtx.ExecuteQueryAsync(); + + foreach (var s in batch) + { + if (s.Status == "Active" + && !s.Url.Contains("-my.sharepoint.com", StringComparison.OrdinalIgnoreCase)) + { + results.Add(new SiteInfo(s.Url, s.Title)); + } + } + } + + return results.OrderBy(s => s.Url).ToList(); } internal static string DeriveAdminUrl(string tenantUrl)