fix: resolve plugin registration and cross-tenant auth issues (#59)#60
fix: resolve plugin registration and cross-tenant auth issues (#59)#60joshsmithxrm merged 6 commits intomainfrom
Conversation
- Add unsecureConfiguration field (renamed from configuration for clarity) - Add entityAlias field to image output - Add MainOperation (stage 30) support for custom actions - Add ZuluTimeConverter for consistent timestamp formatting - Fix solution component handling for updates (add even on update) - Fix cross-platform path normalization (use forward slashes) - Add JSON schema for plugin-registration.json 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When silent token acquisition fails (e.g., tokens cached under different tenant's authority), the fallback now respects the profile's original AuthMethod instead of always defaulting to device code flow. - Add preferredAuthMethod parameter to GlobalDiscoveryService - Check if interactive browser is available before falling back - Use interactive browser for profiles created with that method - Only fall back to device code for SP profiles or when interactive unavailable - Always use "organizations" authority for discovery (enables token reuse) Fixes item #9 in issue #59. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Root cause: MSAL token cache shared by all profiles, but account lookup
used `accounts.FirstOrDefault()` which picked arbitrary cached account
regardless of which tenant the profile belonged to.
Fix:
- Add HomeAccountId property to AuthProfile and ICredentialProvider
- Store account identifier (format: {objectId}.{tenantId}) after auth
- Use GetAccountAsync(homeAccountId) for precise account lookup
- Fall back to filtering by HomeAccountId.TenantId if no identifier
- Never silently use random cached account - force re-auth if needed
- Apply fix to: InteractiveBrowserCredentialProvider,
DeviceCodeCredentialProvider, GlobalDiscoveryService
The fix ensures each profile's token lookup finds only tokens from the
correct tenant, preventing cross-tenant environment discovery.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @joshsmithxrm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers significant improvements in two core areas: plugin registration and cross-tenant authentication. It enhances the reliability and usability of plugin registration by introducing a formal schema, supporting new configuration options, and fixing component management. Concurrently, it resolves a critical cross-tenant authentication bug by implementing a more precise MSAL account lookup strategy, ensuring users are always authenticated against the correct tenant and preventing unintended data access. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent pull request that addresses several important issues. The fix for the cross-tenant authentication issue is critical and the implementation is robust, correctly using HomeAccountId for precise account lookups and preventing the use of incorrect cached tokens. The addition of the FindAccountAsync method is the right approach to centralize this logic within each provider.
The improvements to plugin registration are also very valuable. Adding support for the MainOperation stage, unsecureConfiguration, and entityAlias enhances the tool's capabilities. The fixes for adding components to solutions on update, normalizing paths for cross-platform compatibility, and dynamically looking up entity type codes all contribute to a more reliable and robust tool. The new JSON schema is a great addition for validation and discoverability.
I have one suggestion regarding code duplication to improve maintainability, but overall this is a high-quality contribution.
There was a problem hiding this comment.
Pull request overview
This PR resolves plugin registration issues and fixes a critical cross-tenant authentication bug. The plugin registration improvements add missing fields and features, while the auth fix ensures MSAL correctly identifies cached accounts per tenant instead of using arbitrary cached accounts.
Key Changes:
- Add
entityAlias,unsecureConfiguration, andMainOperationstage support to plugin registration - Fix cross-tenant auth by storing and using
HomeAccountIdfor precise MSAL account lookup - Add runtime entity type code lookup for dynamic entities like
pluginpackage
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/PPDS.Cli/Plugins/Models/PluginRegistrationConfig.cs |
Added ZuluTimeConverter for timestamps, renamed Configuration to UnsecureConfiguration, added EntityAlias property |
src/PPDS.Cli/Plugins/Registration/PluginRegistrationService.cs |
Added ETC cache for runtime lookup, MainOperation stage mapping, solution component addition on UPDATE path |
src/PPDS.Cli/Plugins/Extraction/AssemblyExtractor.cs |
Added entityAlias defaulting logic and MainOperation stage support |
src/PPDS.Cli/Commands/Plugins/ExtractCommand.cs |
Normalized paths to forward slashes for cross-platform compatibility |
src/PPDS.Cli/Commands/Plugins/ListCommand.cs |
Added unsecureConfiguration and entityAlias to JSON output |
src/PPDS.Cli/Commands/Auth/AuthCommandGroup.cs |
Store HomeAccountId after authentication |
src/PPDS.Auth/Profiles/AuthProfile.cs |
Added HomeAccountId property for MSAL account tracking |
src/PPDS.Auth/Discovery/GlobalDiscoveryService.cs |
Implemented FindAccountAsync with HomeAccountId lookup and tenant filtering |
src/PPDS.Auth/Credentials/InteractiveBrowserCredentialProvider.cs |
Added FindAccountAsync method with HomeAccountId support |
src/PPDS.Auth/Credentials/DeviceCodeCredentialProvider.cs |
Added FindAccountAsync method with HomeAccountId support |
src/PPDS.Auth/Credentials/ICredentialProvider.cs |
Added HomeAccountId property to interface |
src/PPDS.Auth/Credentials/*CredentialProvider.cs |
Implemented HomeAccountId property in all credential providers |
schemas/plugin-registration.schema.json |
New JSON schema for plugin registration configuration validation |
src/PPDS.Cli/CHANGELOG.md |
Documented plugin registration additions and fixes |
src/PPDS.Auth/CHANGELOG.md |
Documented cross-tenant auth fix |
tests/PPDS.Cli.Tests/Plugins/Models/PluginRegistrationConfigTests.cs |
Updated tests for UnsecureConfiguration rename |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract MsalAccountHelper for DRY account lookup (Gemini feedback) - Remove entityAlias from schema required array (Copilot feedback) - Add CultureInfo.InvariantCulture to DateTimeOffset.Parse (Copilot feedback) Generic catch clause deferred to #61. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks for the update, @joshsmithxrm! I appreciate you addressing the DRY violation in |
Summary
Resolves all items in #59:
Plugin Registration Fixes
entityAliasfield to image definitions (defaults tonameif not specified)unsecureConfigurationfield for step configurationMainOperationstage (30) support for Custom API plugin stepsplugin-registration.jsongeneratedAttimestampspluginpackageentityCross-Tenant Auth Fix
Root cause: MSAL token cache shared by all profiles, but account lookup used
accounts.FirstOrDefault()which picked an arbitrary cached account regardless of which tenant the profile belonged to.Fix:
HomeAccountIdproperty toAuthProfileandICredentialProvider{objectId}.{tenantId}) after authenticationGetAccountAsync(homeAccountId)for precise account lookupHomeAccountId.TenantIdif no identifier storedApplied to:
InteractiveBrowserCredentialProvider,DeviceCodeCredentialProvider,GlobalDiscoveryServiceTest plan
Closes #59
🤖 Generated with Claude Code