Fix missing Messaging Bot API permissions and incorrect permission inheritance#68
Merged
Merged
Conversation
…heritance This change fixes two major issues in the permissions setup flow: Messaging Bot API permissions were not being added due to a regression. This PR corrects that and ensures the required scopes are consistently applied. Permission inheritance was not being set correctly across all resources. Updated the logic to properly apply inheritable permissions without gaps. The rest of the refactor is implementation cleanup — moved to a unified permissions model, introduced the new ResourceConsent class, and consolidated all permission handling into EnsureResourcePermissionsAsync with better retries, verification, and logging. Updated related commands, tests, and docs to match the new model while keeping backward compatibility.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the permission configuration system to use a unified model and fixes two critical issues: missing Messaging Bot API permissions and incorrect permission inheritance setup. The changes introduce a new ResourceConsent class to track per-resource consent state and consolidate all permission handling into EnsureResourcePermissionsAsync with built-in verification and retry logic.
Key Changes:
- Introduced
ResourceConsentmodel to replace scattered consent properties with structured per-resource tracking - Created unified
EnsureResourcePermissionsAsyncmethod that handles OAuth2 grants, required resource access, and inheritable permissions in one flow with verification - Fixed Messaging Bot API permissions regression by ensuring proper configuration through the unified method
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
src/Microsoft.Agents.A365.DevTools.Cli/Models/ResourceConsent.cs |
New model class to track consent state, scopes, and inheritable permissions per resource |
src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs |
Replaced individual consent properties with ResourceConsents collection and added IsInheritanceConfigured() helper |
src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs |
Added VerifyInheritablePermissionsAsync and AddRequiredResourceAccessAsync methods; renamed method for clarity |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs |
Replaced separate permission methods with unified EnsureResourcePermissionsAsync including verification with retry logic |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs |
Updated to use unified permission configuration method for both MCP and Bot API permissions |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs |
Removed Connectivity consent flow; updated to use ResourceConsents collection |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs |
Updated to pass setupResults to permission methods and use new IsInheritanceConfigured() |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/ConfigCommand.cs |
Added resource consents table display; removed specific exception handlers |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs |
Updated to clear ResourceConsents collection instead of individual properties |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/DeployCommand.cs |
Removed direct property updates for inheritance state (now tracked in ResourceConsents) |
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs |
Added helper to check inheritance from ResourceConsents collection |
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceVerifyInheritablePermissionsTests.cs |
New test file with comprehensive coverage for inheritable permissions verification |
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Agent365ConfigServiceTests.cs |
Updated tests to use ResourceConsents instead of deprecated consent properties |
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Models/Agent365ConfigTests.cs |
Updated tests to validate ResourceConsents collection behavior |
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/ConfigCommandStaticDynamicSeparationTests.cs |
Updated tests to verify resourceConsents is treated as dynamic property |
src/DEVELOPER.md |
Updated documentation to describe three-layer permissions architecture and unified configuration approach |
mengyimicro
previously approved these changes
Dec 4, 2025
Updated `ConfigCommand` to refine resource consents table display logic based on flags. Simplified `SetupHelpers` by removing redundant error handling and fixing typographical errors. Refactored `GraphApiService` to improve null safety, streamline JSON processing with LINQ, and enhance permission validation. Added robust error handling for missing or invalid application properties. Introduced `GraphApiServiceAddRequiredResourceAccessTests` with comprehensive test coverage for various scenarios, including null safety, invalid scopes, and permission merging. Improved code readability, maintainability, and logging for better diagnostics. Added new unit tests to ensure robustness and prevent regressions.
mengyimicro
approved these changes
Dec 4, 2025
rahuldevikar761
approved these changes
Dec 4, 2025
sellakumaran
added a commit
that referenced
this pull request
Feb 27, 2026
…heritance (#68) * Fix missing Messaging Bot API permissions and incorrect permission inheritance This change fixes two major issues in the permissions setup flow: Messaging Bot API permissions were not being added due to a regression. This PR corrects that and ensures the required scopes are consistently applied. Permission inheritance was not being set correctly across all resources. Updated the logic to properly apply inheritable permissions without gaps. The rest of the refactor is implementation cleanup — moved to a unified permissions model, introduced the new ResourceConsent class, and consolidated all permission handling into EnsureResourcePermissionsAsync with better retries, verification, and logging. Updated related commands, tests, and docs to match the new model while keeping backward compatibility. * Refactor and enhance permission handling logic Updated `ConfigCommand` to refine resource consents table display logic based on flags. Simplified `SetupHelpers` by removing redundant error handling and fixing typographical errors. Refactored `GraphApiService` to improve null safety, streamline JSON processing with LINQ, and enhance permission validation. Added robust error handling for missing or invalid application properties. Introduced `GraphApiServiceAddRequiredResourceAccessTests` with comprehensive test coverage for various scenarios, including null safety, invalid scopes, and permission merging. Improved code readability, maintainability, and logging for better diagnostics. Added new unit tests to ensure robustness and prevent regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change fixes two major issues in the setup flow:
Messaging Bot API permissions were not being added due to a regression. This PR corrects that and ensures the required scopes are consistently applied.
Permission inheritance was not being set correctly across all resources. Updated the logic to properly apply inheritable permissions without gaps.
The rest of the refactor is implementation cleanup — moved to a unified permissions model, introduced the new ResourceConsent class, and consolidated all permission handling into EnsureResourcePermissionsAsync with better retries, verification, and logging. Updated related commands, tests, and docs to match the new model while keeping backward compatibility.
Fixes #55