Skip to content

Commit 236bb33

Browse files
joshsmithxrmclaude
joshsmithxrm
and
claude
authored
fix: ServiceClient org metadata and multi-layer environment resolution (#98)
* fix: add SkipDiscovery and multi-layer environment resolution Issue #86: Added SkipDiscovery = false to ConnectionOptions in 5 credential providers (InteractiveBrowser, DeviceCode, ManagedIdentity, GitHubFederated, AzureDevOpsFederated) to force org metadata discovery. Issues #89 + #88: Added EnvironmentResolutionService for multi-layer resolution: - Direct Dataverse connection first (works for service principals) - Global Discovery fallback (for interactive users) - Updated ppds env select and ppds auth update --environment to use new resolver 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: force eager org metadata discovery before clone ServiceClient org metadata discovery is lazy - properties are only populated when first accessed. Since the connection pool clones the seed client before properties are accessed, the clones had empty org metadata. Fix: Access ConnectedOrgFriendlyName immediately after creating the ServiceClient to trigger discovery while values can still be copied to clones. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: remove unnecessary SkipDiscovery setting Testing confirmed that SkipDiscovery = false is not required. The eager property access alone triggers org metadata discovery. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: improve comments explaining eager property access pattern Added detailed comments explaining why we access ConnectedOrgFriendlyName immediately after ServiceClient creation. Referenced that PAC CLI uses the same pattern (ConnectedOrgToSelectedOrganizationInfo after Connect). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: remove inappropriate external code reference from comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address bot review comments - Add token cache priming with cancellationToken in GitHubFederated, AzureDevOpsFederated, and ManagedIdentity credential providers - Simplify CanUseGlobalDiscovery to use 'is or' pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: fix XML documentation for ResolveAsync - Fixed return description to match actual Result type - Removed incorrect exception documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 96f693d commit 236bb33

11 files changed

+423
-50
lines changed

‎src/PPDS.Auth/CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12-
- **ServiceClient org metadata not populated** - Credential providers now use `ConnectionOptions` constructor instead of token provider constructor, which triggers org metadata discovery. This populates `ConnectedOrgFriendlyName`, `ConnectedOrgUniqueName`, and `ConnectedOrgId` properties. ([#86](https://github.com/joshsmithxrm/ppds-sdk/issues/86))
12+
- **ServiceClient org metadata not populated** - Credential providers now force eager org metadata discovery by accessing `ConnectedOrgFriendlyName` immediately after creating the ServiceClient. Discovery is lazy by default, and the connection pool clones clients before properties are accessed, resulting in empty metadata. This fix ensures `ConnectedOrgFriendlyName`, `ConnectedOrgUniqueName`, and `ConnectedOrgId` are populated. ([#86](https://github.com/joshsmithxrm/ppds-sdk/issues/86))
1313

1414
### Added
1515

16+
- **`EnvironmentResolutionService`** - Multi-layer environment resolution that tries direct Dataverse connection first (works for service principals), then falls back to Global Discovery for user authentication. Returns full org metadata. ([#89](https://github.com/joshsmithxrm/ppds-sdk/issues/89), [#88](https://github.com/joshsmithxrm/ppds-sdk/issues/88))
1617
- **Integration tests for credential providers** - Live tests for `ClientSecretCredentialProvider`, `CertificateFileCredentialProvider`, and `GitHubFederatedCredentialProvider` ([#55](https://github.com/joshsmithxrm/ppds-sdk/issues/55))
1718
- Manual test procedures documentation for interactive browser and device code authentication
1819

‎src/PPDS.Auth/Credentials/AzureDevOpsFederatedCredentialProvider.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,24 @@ public async Task<ServiceClient> CreateServiceClientAsync(
7777

7878
EnsureCredentialInitialized();
7979

80-
// Create ServiceClient using ConnectionOptions to ensure org metadata discovery.
81-
// The provider function acquires tokens on demand and refreshes when needed.
80+
// Get token and prime the cache (uses cancellationToken for cancellable first request)
81+
await GetTokenAsync(environmentUrl, cancellationToken).ConfigureAwait(false);
82+
83+
// Create ServiceClient using ConnectionOptions.
84+
// The provider function uses cached tokens and refreshes when needed.
8285
var options = new ConnectionOptions
8386
{
8487
ServiceUri = new Uri(environmentUrl),
8588
AccessTokenProviderFunctionAsync = _ => GetTokenAsync(environmentUrl, CancellationToken.None)
8689
};
8790
var client = new ServiceClient(options);
8891

92+
// Force org metadata discovery before client is cloned by pool.
93+
// ServiceClient uses lazy initialization - properties like ConnectedOrgFriendlyName
94+
// are only populated when first accessed. The connection pool clones clients before
95+
// properties are accessed, so clones would have empty metadata.
96+
_ = client.ConnectedOrgFriendlyName;
97+
8998
if (!client.IsReady)
9099
{
91100
var error = client.LastError ?? "Unknown error";

‎src/PPDS.Auth/Credentials/DeviceCodeCredentialProvider.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public async Task<ServiceClient> CreateServiceClientAsync(
111111
// Get token and prime the cache (may prompt user for device code auth)
112112
await GetTokenAsync(environmentUrl, forceInteractive, cancellationToken).ConfigureAwait(false);
113113

114-
// Create ServiceClient using ConnectionOptions to ensure org metadata discovery.
114+
// Create ServiceClient using ConnectionOptions.
115115
// The provider function uses cached tokens and refreshes silently when needed.
116116
var options = new ConnectionOptions
117117
{
@@ -120,6 +120,12 @@ public async Task<ServiceClient> CreateServiceClientAsync(
120120
};
121121
var client = new ServiceClient(options);
122122

123+
// Force org metadata discovery before client is cloned by pool.
124+
// ServiceClient uses lazy initialization - properties like ConnectedOrgFriendlyName
125+
// are only populated when first accessed. The connection pool clones clients before
126+
// properties are accessed, so clones would have empty metadata.
127+
_ = client.ConnectedOrgFriendlyName;
128+
123129
if (!client.IsReady)
124130
{
125131
var error = client.LastError ?? "Unknown error";

‎src/PPDS.Auth/Credentials/GitHubFederatedCredentialProvider.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,24 @@ public async Task<ServiceClient> CreateServiceClientAsync(
7777

7878
EnsureCredentialInitialized();
7979

80-
// Create ServiceClient using ConnectionOptions to ensure org metadata discovery.
81-
// The provider function acquires tokens on demand and refreshes when needed.
80+
// Get token and prime the cache (uses cancellationToken for cancellable first request)
81+
await GetTokenAsync(environmentUrl, cancellationToken).ConfigureAwait(false);
82+
83+
// Create ServiceClient using ConnectionOptions.
84+
// The provider function uses cached tokens and refreshes when needed.
8285
var options = new ConnectionOptions
8386
{
8487
ServiceUri = new Uri(environmentUrl),
8588
AccessTokenProviderFunctionAsync = _ => GetTokenAsync(environmentUrl, CancellationToken.None)
8689
};
8790
var client = new ServiceClient(options);
8891

92+
// Force org metadata discovery before client is cloned by pool.
93+
// ServiceClient uses lazy initialization - properties like ConnectedOrgFriendlyName
94+
// are only populated when first accessed. The connection pool clones clients before
95+
// properties are accessed, so clones would have empty metadata.
96+
_ = client.ConnectedOrgFriendlyName;
97+
8998
if (!client.IsReady)
9099
{
91100
var error = client.LastError ?? "Unknown error";

‎src/PPDS.Auth/Credentials/InteractiveBrowserCredentialProvider.cs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public async Task<ServiceClient> CreateServiceClientAsync(
144144
// Get token and prime the cache (may prompt user for interactive auth)
145145
await GetTokenAsync(environmentUrl, forceInteractive, cancellationToken).ConfigureAwait(false);
146146

147-
// Create ServiceClient using ConnectionOptions to ensure org metadata discovery.
147+
// Create ServiceClient using ConnectionOptions.
148148
// The provider function uses cached tokens and refreshes silently when needed.
149149
var options = new ConnectionOptions
150150
{
@@ -153,6 +153,12 @@ public async Task<ServiceClient> CreateServiceClientAsync(
153153
};
154154
var client = new ServiceClient(options);
155155

156+
// Force org metadata discovery before client is cloned by pool.
157+
// ServiceClient uses lazy initialization - properties like ConnectedOrgFriendlyName
158+
// are only populated when first accessed. The connection pool clones clients before
159+
// properties are accessed, so clones would have empty metadata.
160+
_ = client.ConnectedOrgFriendlyName;
161+
156162
if (!client.IsReady)
157163
{
158164
var error = client.LastError ?? "Unknown error";

‎src/PPDS.Auth/Credentials/ManagedIdentityCredentialProvider.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,11 @@ public async Task<ServiceClient> CreateServiceClientAsync(
9191
// Normalize URL
9292
environmentUrl = environmentUrl.TrimEnd('/');
9393

94-
// Create ServiceClient using ConnectionOptions to ensure org metadata discovery.
95-
// The provider function acquires tokens on demand and refreshes when needed.
94+
// Get token and prime the cache (uses cancellationToken for cancellable first request)
95+
await GetTokenAsync(environmentUrl, cancellationToken).ConfigureAwait(false);
96+
97+
// Create ServiceClient using ConnectionOptions.
98+
// The provider function uses cached tokens and refreshes when needed.
9699
ServiceClient client;
97100
try
98101
{
@@ -102,6 +105,12 @@ public async Task<ServiceClient> CreateServiceClientAsync(
102105
AccessTokenProviderFunctionAsync = _ => GetTokenAsync(environmentUrl, CancellationToken.None)
103106
};
104107
client = new ServiceClient(options);
108+
109+
// Force org metadata discovery before client is cloned by pool.
110+
// ServiceClient uses lazy initialization - properties like ConnectedOrgFriendlyName
111+
// are only populated when first accessed. The connection pool clones clients before
112+
// properties are accessed, so clones would have empty metadata.
113+
_ = client.ConnectedOrgFriendlyName;
105114
}
106115
catch (Exception ex)
107116
{

‎src/PPDS.Auth/Credentials/MsalClientBuilder.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@ public static void UnregisterCache(MsalCacheHelper? cacheHelper, IPublicClientAp
113113
{
114114
cacheHelper.UnregisterCache(client.UserTokenCache);
115115
}
116-
catch
116+
catch (Exception)
117117
{
118-
// Ignore errors during cleanup
118+
// Cleanup should never throw - swallow all errors
119119
}
120120
}
121121
}

0 commit comments

Comments
 (0)