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) <noreply@anthropic.com>
This commit is contained in:
89
.planning/debug/site-picker-parsing-error.md
Normal file
89
.planning/debug/site-picker-parsing-error.md
Normal file
@@ -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
|
||||||
@@ -27,18 +27,12 @@ public class SiteListService : ISiteListService
|
|||||||
ct.ThrowIfCancellationRequested();
|
ct.ThrowIfCancellationRequested();
|
||||||
progress.Report(OperationProgress.Indeterminate("Loading sites..."));
|
progress.Report(OperationProgress.Indeterminate("Loading sites..."));
|
||||||
|
|
||||||
var adminUrl = DeriveAdminUrl(profile.TenantUrl);
|
// Obtain the already-authenticated context for the tenant URL, then clone it to
|
||||||
var adminProfile = new TenantProfile
|
// the admin URL. Cloning reuses the existing token — no second interactive login.
|
||||||
{
|
ClientContext ctx;
|
||||||
Name = profile.Name,
|
|
||||||
TenantUrl = adminUrl,
|
|
||||||
ClientId = profile.ClientId
|
|
||||||
};
|
|
||||||
|
|
||||||
ClientContext adminCtx;
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
adminCtx = await _sessionManager.GetOrCreateContextAsync(adminProfile, ct);
|
ctx = await _sessionManager.GetOrCreateContextAsync(profile, ct);
|
||||||
}
|
}
|
||||||
catch (ServerException ex) when (ex.Message.Contains("Access denied", StringComparison.OrdinalIgnoreCase))
|
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);
|
"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<SiteInfo>();
|
||||||
var tenant = new Tenant(adminCtx);
|
var tenant = new Tenant(adminCtx);
|
||||||
var siteProps = tenant.GetSitePropertiesFromSharePoint("", true);
|
SPOSitePropertiesEnumerable? batch = null;
|
||||||
adminCtx.Load(siteProps);
|
|
||||||
await adminCtx.ExecuteQueryAsync();
|
|
||||||
|
|
||||||
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
|
var filter = new SPOSitePropertiesEnumerableFilter
|
||||||
.Where(s => s.Status == "Active"
|
{
|
||||||
&& !s.Url.Contains("-my.sharepoint.com", StringComparison.OrdinalIgnoreCase))
|
IncludePersonalSite = PersonalSiteFilter.UseServerDefault,
|
||||||
.Select(s => new SiteInfo(s.Url, s.Title))
|
StartIndex = batch?.NextStartIndexFromSharePoint,
|
||||||
.OrderBy(s => s.Url)
|
IncludeDetail = true
|
||||||
.ToList();
|
};
|
||||||
|
|
||||||
|
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)
|
internal static string DeriveAdminUrl(string tenantUrl)
|
||||||
|
|||||||
Reference in New Issue
Block a user