From d357089560d254c1c6b038526e021ae00014121c Mon Sep 17 00:00:00 2001 From: slreznit Date: Sun, 9 Aug 2026 23:32:25 +0300 Subject: [PATCH 01/14] Add Defender consent guidance --- CHANGELOG.md | 5 ++ .../Commands/QueryEntraCommand.cs | 1 + .../NonDwBlueprintSetupOrchestrator.cs | 13 ++-- .../SetupSubcommands/PermissionsSubcommand.cs | 2 + .../Commands/SetupSubcommands/SetupHelpers.cs | 66 +++++++++++++++---- .../Constants/ConfigConstants.cs | 19 ++++++ .../Services/LogRedactionService.cs | 1 + .../SetupSubcommands/PermissionSpecsTests.cs | 23 ++++++- ...tupHelpersAdminConsentInstructionsTests.cs | 16 +++++ .../Helpers/SetupHelpersConsentUrlTests.cs | 24 +++++-- 10 files changed, 145 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2b2eb14..e097c4e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,12 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g **Option B — CLI** (`a365 setup admin`) has been removed in this release. Use Option A above, or copy the PowerShell instructions printed in the `a365 setup all` summary output. +#### Existing agents: grant Defender API permissions + +Agents provisioned before this release need `AIAgentsRTP.ToolInvocation` granted as both a **delegated** and an **application** permission on the blueprint app for the Defender security integration. Requires Global Administrator. Follow the steps above, searching for `86a21212-634e-4553-b3d6-e477e4c9d9ec` in step 2 and selecting `AIAgentsRTP.ToolInvocation` in steps 3 and 4. Re-running `a365 setup all` grants it automatically. + ### Added +- `AIAgentsRTP.ToolInvocation` on the Defender API is now granted automatically during `a365 setup` as both a delegated and an application permission, enabling the Microsoft Defender security integration without manual Entra steps. - Log separator written at the start of each CLI invocation now redacts values for secret-bearing options (e.g. `--idp-client-secret`) so they are not written to the log file in plain text. - Authentication context (tenant and user) is now logged at the `Information` level whenever the resolved sign-in identity changes, giving operators a clear audit trail in the log file of who the CLI is acting as, without exposing credentials. - `a365 develop-mcp evaluate` command for evaluating MCP server tool schema quality — runs deterministic and semantic checks (via GitHub Copilot or Claude Code CLIs), computes maturity scoring, and generates an interactive HTML report diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs index 46ad66c9..c544afbf 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs @@ -616,6 +616,7 @@ private static Command CreateInstanceScopesSubcommand( AuthenticationConstants.MicrosoftGraphResourceAppId => "Microsoft Graph", ConfigConstants.MessagingBotApiAppId => "Messaging Bot API", ConfigConstants.ObservabilityApiAppId => "Observability API", + ConfigConstants.DefenderApiAppId => "Defender API", PowerPlatformConstants.PowerPlatformApiResourceAppId => "Power Platform API", "00000002-0000-0000-c000-000000000000" => "Azure Active Directory Graph", "797f4846-ba00-4fd7-ba43-dac1f8f63013" => "Azure Service Management", diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index 9a6150cc..e5b5e10b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -20,8 +20,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; /// 1. Requirements validation /// 2. Blueprint creation (shared with DW) /// 3. Batch permissions on the blueprint (shared with DW pipeline; non-DW spec set: -/// Observability API, Power Platform API, custom). MAC reads from the blueprint, -/// so stamping here gives the same set visibility there. +/// Observability API, Defender API, Power Platform API, custom). MAC reads +/// from the blueprint, so stamping here gives the same set visibility there. /// 4. Agent Identity creation via POST /beta/servicePrincipals/Microsoft.Graph.AgentIdentity /// 5. Agent Identity permission grants (same spec set as step 3) — OBO or S2S /// 6. Agent registration via Graph API (copilot/agentRegistrations) @@ -117,14 +117,15 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i logger.LogInformation(sub + "create managed identity"); } - // 3. Inheritable Permissions — non-DW spec set (Observability API, Power Platform API, custom) - // stamped on the blueprint via SetInheritablePermissionsAsync so MAC and other dependent - // systems can see them. The same set is applied to the agent identity SP in step 5. + // 3. Inheritable Permissions — non-DW spec set (Observability API, Defender API, + // Power Platform API, custom) stamped on the blueprint via SetInheritablePermissionsAsync + // so MAC and other dependent systems can see them. The same set is applied to the agent + // identity SP in step 5. var selectedAuthMode = authMode ?? config.AuthMode; var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) ? "obo" : selectedAuthMode.Trim().ToLowerInvariant(); - logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for Observability API, Power Platform API, and custom permissions (Global Administrator required; consent URL printed if absent)"); + logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for Observability API, Defender API, Power Platform API, and custom permissions (Global Administrator required; consent URL printed if absent)"); // 4. Blueprint Permission Grants — per authMode. The consent URL targets the blueprint // app, and S2S app-role assignments are persisted as grants flowing from the blueprint; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index c1b4aaea..e9d12b08 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -310,6 +310,7 @@ private static Command CreateBotSubcommand( logger.LogInformation(" - Blueprint: {BlueprintId}", dryRunConfig.AgentBlueprintId); logger.LogInformation(" - Messaging Bot API: {Scope}", ConfigConstants.MessagingBotApiAdminConsentScope); logger.LogInformation(" - Observability API: {OtelScope} (delegated + application)", ConfigConstants.ObservabilityApiOtelWriteScope); + logger.LogInformation(" - Defender API: {DefenderScope} (delegated + application)", ConfigConstants.DefenderApiToolInvocationScope); logger.LogInformation(" - Power Platform API: Connectivity.Connections.Read"); logger.LogInformation("No changes made. Run without --dry-run to execute."); return; @@ -852,6 +853,7 @@ internal static async Task RemoveStaleCustomPermissionsAsync( envAtgAppId, ConfigConstants.MessagingBotApiAppId, ConfigConstants.ObservabilityApiAppId, + ConfigConstants.DefenderApiAppId, PowerPlatformConstants.PowerPlatformApiResourceAppId, AuthenticationConstants.MicrosoftGraphResourceAppId, }; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 53e85506..a24bef0b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -49,9 +49,9 @@ internal static void PrintDryRunBlueprintReuseRows(ILogger logger, string bluepr /// Returns the fixed-scope ResourcePermissionSpecs for the platform APIs that every /// agent blueprint requires. /// - /// Observability API and Power Platform API are always included. Messaging Bot API is - /// included only when is true — non-M365 (blueprint-only) agents - /// have no messaging surface so Bot scopes serve no purpose. + /// Observability API, Defender API, and Power Platform API are always included. + /// Messaging Bot API is included only when is true — non-M365 + /// (blueprint-only) agents have no messaging surface so Bot scopes serve no purpose. /// /// internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInheritable, bool isM365) @@ -79,6 +79,12 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInhe new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, setInheritable, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); + specs.Add(new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + new[] { ConfigConstants.DefenderApiToolInvocationScope }, + setInheritable, + AppRoleScopes: new[] { ConfigConstants.DefenderApiToolInvocationScope })); specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -362,8 +368,8 @@ internal static async Task> BuildConfiguredPermissi /// /// Fixed permission specs for the non-DW admin consent flow. - /// Observability API requires both Application (app role for S2S) and Delegated (oauth2 grant for OBO). - /// Power Platform API requires Delegated only. + /// Observability API and Defender API require both Application (app role for S2S) + /// and Delegated (oauth2 grant for OBO). Power Platform API requires Delegated only. /// Extend this list or pass an override to /// when additional APIs are required (e.g. dynamic MCP scopes, custom permissions). /// @@ -371,14 +377,26 @@ internal static async Task> BuildConfiguredPermissi [ ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Application"), ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Delegated"), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiToolInvocationScope, "Application"), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiToolInvocationScope, "Delegated"), ("Power Platform API", PowerPlatformConstants.PowerPlatformApiResourceAppId, PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, "Delegated"), ]; + /// + /// Fixed platform APIs that expose an application (S2S) app role, used to render the manual + /// PowerShell hand-off when the programmatic assignment could not complete. + /// + internal static readonly IReadOnlyList<(string ResourceName, string ResourceAppId, string Role)> FixedApiAppRoleHandoffSpecs = + [ + ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiToolInvocationScope), + ]; + /// /// Logs step-by-step instructions for a Global Administrator to grant admin consent /// for the blueprint app, with two options: Entra portal and PowerShell. /// - /// Defaults to (Observability API + Power Platform API). + /// Defaults to (Observability, Defender, and Power Platform APIs). /// Pass an explicit list to support dynamic or extended permission sets. /// /// @@ -831,7 +849,7 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) { actionCount++; logger.LogInformation(""); - logger.LogInformation(" {N}. Observability API S2S app role (PowerShell):", actionCount); + logger.LogInformation(" {N}. Application (S2S) app roles (PowerShell):", actionCount); logger.LogInformation(" Required role: {Roles}", AuthenticationConstants.S2SGrantRequiredRoles); if (!string.IsNullOrWhiteSpace(results.TenantId)) logger.LogInformation(" Connect-MgGraph -TenantId '{TenantId}' -Scopes 'AppRoleAssignment.ReadWrite.All','Application.Read.All'", results.TenantId); @@ -844,9 +862,14 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) // Grant targets the agent identity SP directly (SP object ID, not an app ID). var agentSpId = results.AgentIdentityId ?? ""; logger.LogInformation(" $agentSpId = '{AgentSpId}'", agentSpId); - logger.LogInformation(" $obs = Get-MgServicePrincipal -Filter \"appId eq '{ObsApiAppId}'\"", ConfigConstants.ObservabilityApiAppId); - logger.LogInformation(" $rid = ($obs.AppRoles | Where-Object {{ $_.Value -eq '{ObsScope}' }}).Id", ConfigConstants.ObservabilityApiOtelWriteScope); - logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $agentSpId -PrincipalId $agentSpId -ResourceId $obs.Id -AppRoleId $rid"); + foreach (var (resourceName, resourceAppId, role) in FixedApiAppRoleHandoffSpecs) + { + logger.LogInformation(""); + logger.LogInformation(" # {ResourceName}: {Role}", resourceName, role); + logger.LogInformation(" $res = Get-MgServicePrincipal -Filter \"appId eq '{ResAppId}'\"", resourceAppId); + logger.LogInformation(" $rid = ($res.AppRoles | Where-Object {{ $_.Value -eq '{Role}' }}).Id", role); + logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $agentSpId -PrincipalId $agentSpId -ResourceId $res.Id -AppRoleId $rid"); + } logger.LogInformation(""); if (!string.IsNullOrWhiteSpace(results.TenantId)) logger.LogInformation(" Tenant : {TenantId}", results.TenantId); @@ -856,9 +879,14 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) { // DW: grant targets the blueprint SP (looked up by app ID). logger.LogInformation(" $bp = Get-MgServicePrincipal -Filter \"appId eq '{BlueprintAppId}'\"", blueprintAppId); - logger.LogInformation(" $obs = Get-MgServicePrincipal -Filter \"appId eq '{ObsApiAppId}'\"", ConfigConstants.ObservabilityApiAppId); - logger.LogInformation(" $rid = ($obs.AppRoles | Where-Object {{ $_.Value -eq '{ObsScope}' }}).Id", ConfigConstants.ObservabilityApiOtelWriteScope); - logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $bp.Id -PrincipalId $bp.Id -ResourceId $obs.Id -AppRoleId $rid"); + foreach (var (resourceName, resourceAppId, role) in FixedApiAppRoleHandoffSpecs) + { + logger.LogInformation(""); + logger.LogInformation(" # {ResourceName}: {Role}", resourceName, role); + logger.LogInformation(" $res = Get-MgServicePrincipal -Filter \"appId eq '{ResAppId}'\"", resourceAppId); + logger.LogInformation(" $rid = ($res.AppRoles | Where-Object {{ $_.Value -eq '{Role}' }}).Id", role); + logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $bp.Id -PrincipalId $bp.Id -ResourceId $res.Id -AppRoleId $rid"); + } logger.LogInformation(""); logger.LogInformation(" To share with your {Roles}:", AuthenticationConstants.S2SGrantRequiredRoles); logger.LogInformation(" Blueprint : {BlueprintAppId}", blueprintAppId); @@ -883,6 +911,11 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $obsSp.Id; scope = '{ObsScope}' }} | ConvertTo-Json", ConfigConstants.ObservabilityApiOtelWriteScope); logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri 'https://graph.microsoft.com/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'"); logger.LogInformation(""); + logger.LogInformation(" # Defender API"); + logger.LogInformation(" $defenderSp = Get-MgServicePrincipal -Filter \"appId eq '{DefenderAppId}'\"", ConfigConstants.DefenderApiAppId); + logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $defenderSp.Id; scope = '{DefenderScope}' }} | ConvertTo-Json", ConfigConstants.DefenderApiToolInvocationScope); + logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri 'https://graph.microsoft.com/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'"); + logger.LogInformation(""); logger.LogInformation(" # Power Platform API"); logger.LogInformation(" $ppSp = Get-MgServicePrincipal -Filter \"appId eq '{PpAppId}'\"", PowerPlatformConstants.PowerPlatformApiResourceAppId); logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $ppSp.Id; scope = '{PpScope}' }} | ConvertTo-Json", PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead); @@ -1076,6 +1109,7 @@ internal static List PopulateAdminConsentUrls( ["Agent 365 Tools"] = mcpResourceAppId, ["Messaging Bot API"] = ConfigConstants.MessagingBotApiAppId, ["Observability API"] = ConfigConstants.ObservabilityApiAppId, + ["Defender API"] = ConfigConstants.DefenderApiAppId, ["Power Platform API"] = PowerPlatformConstants.PowerPlatformApiResourceAppId, }; @@ -1177,6 +1211,8 @@ internal static string GetResourceIdentifierUri(string resourceAppId, bool isMcp return ConfigConstants.MessagingBotApiIdentifierUri; if (string.Equals(resourceAppId, ConfigConstants.ObservabilityApiAppId, StringComparison.OrdinalIgnoreCase)) return ConfigConstants.ObservabilityApiIdentifierUri; + if (string.Equals(resourceAppId, ConfigConstants.DefenderApiAppId, StringComparison.OrdinalIgnoreCase)) + return ConfigConstants.DefenderApiIdentifierUri; if (string.Equals(resourceAppId, PowerPlatformConstants.PowerPlatformApiResourceAppId, StringComparison.OrdinalIgnoreCase)) return PowerPlatformConstants.PowerPlatformApiIdentifierUri; // WorkIQ Tools shared (issue #429): match by appId, not display name. V2 per-server @@ -1316,6 +1352,7 @@ static string Build(string tenant, string client, string resourceUri, IEnumerabl urls.Add(("Messaging Bot API", Build(tenantId, blueprintClientId, ConfigConstants.MessagingBotApiIdentifierUri, new[] { ConfigConstants.MessagingBotApiAdminConsentScope }))); urls.Add(("Observability API", Build(tenantId, blueprintClientId, ConfigConstants.ObservabilityApiIdentifierUri, new[] { ConfigConstants.ObservabilityApiOtelWriteScope }))); + urls.Add(("Defender API", Build(tenantId, blueprintClientId, ConfigConstants.DefenderApiIdentifierUri, new[] { ConfigConstants.DefenderApiToolInvocationScope }))); urls.Add(("Power Platform API", Build(tenantId, blueprintClientId, PowerPlatformConstants.PowerPlatformApiIdentifierUri, new[] { PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead }))); return urls; @@ -1371,6 +1408,7 @@ internal static string BuildCombinedConsentUrl( if (isM365) allScopes.Add($"{ConfigConstants.MessagingBotApiIdentifierUri}/{ConfigConstants.MessagingBotApiAdminConsentScope}"); allScopes.Add($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); + allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiToolInvocationScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes); } @@ -1454,7 +1492,7 @@ internal static void PrintDwSetupAllDryRunPlan( } // 4. Inheritable Permissions - logger.LogInformation(DryRunRow(4, "Inheritable Permissions") + "configure for Microsoft Graph, Agent 365 Tools, Messaging Bot API, Observability API, Power Platform API"); + logger.LogInformation(DryRunRow(4, "Inheritable Permissions") + "configure for Microsoft Graph, Agent 365 Tools, Messaging Bot API, Observability API, Defender API, Power Platform API"); // 5. Blueprint Permission Grants logger.LogInformation(DryRunRow(5, "Blueprint Permission Grants") + "admin approval required — see 'Action Required' in setup output"); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index bf2fe665..a6a808c4 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -80,6 +80,18 @@ public static class ConfigConstants /// public const string ObservabilityApiIdentifierUri = "api://9b975845-388f-4429-889e-eab1ef63949c"; + /// + /// Defender API App ID. Hosts the security inspection endpoint used by the Defender integration. + /// + public const string DefenderApiAppId = "86a21212-634e-4553-b3d6-e477e4c9d9ec"; + + /// + /// Defender API identifier URI. Unlike the Observability API this resource + /// publishes an https identifier URI only — api://{appId} is not in its + /// servicePrincipalNames and consent fails with AADSTS500011. + /// + public const string DefenderApiIdentifierUri = "https://rtp-a365.ai.defender.microsoft.com"; + /// /// Single source of truth for the Messaging Bot API delegated scope. /// The resource SP (appId 5a807f24-c9de-44ee-a3a7-329e88a00ffc) exposes exactly @@ -97,6 +109,13 @@ public static class ConfigConstants /// public const string ObservabilityApiOtelWriteScope = "Agent365.Observability.OtelWrite"; + /// + /// Defender API scope for tool invocation inspection, enabling the Defender + /// security integration. Published on the resource as both a delegated scope (OBO) and an + /// application app role (S2S), so it is granted through both paths like OtelWrite. + /// + public const string DefenderApiToolInvocationScope = "AIAgentsRTP.ToolInvocation"; + /// /// Delegated scope value exposed on the blueprint app registration to enable /// OBO (On-Behalf-Of) callers to acquire tokens scoped to the agent. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs index cf678914..b3d09442 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs @@ -59,6 +59,7 @@ public sealed class LogRedactionService : ILogRedactionService "00000003-0000-0000-c000-000000000000", // Microsoft Graph "5a807f24-c9de-44ee-a3a7-329e88a00ffc", // Agent 365 Messaging Bot API "9b975845-388f-4429-889e-eab1ef63949c", // Agent 365 Observability API + "86a21212-634e-4553-b3d6-e477e4c9d9ec", // Agent 365 Defender API "8578e004-a5c6-46e7-913e-12f58912df43", // Power Platform API (Connectivity) "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1", // Agent 365 Tools (MCP audience, production) }; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs index 674d6a52..38d4d71b 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs @@ -89,9 +89,10 @@ public async Task DwPath_NoManifest_NoCustom_ProducesBaselineSpecSet() AuthenticationConstants.MicrosoftGraphResourceAppId, ConfigConstants.MessagingBotApiAppId, ConfigConstants.ObservabilityApiAppId, + ConfigConstants.DefenderApiAppId, PowerPlatformConstants.PowerPlatformApiResourceAppId, McpConstants.WorkIQToolsProdAppId, - }, because: "the DW baseline spec set is the four fixed platform APIs plus the ATG AppId (seeded with McpServersMetadata.Read.All for V1 compatibility)"); + }, because: "the DW baseline spec set is the five fixed platform APIs (including the Defender API required by the security integration) plus the ATG AppId (seeded with McpServersMetadata.Read.All for V1 compatibility)"); // Assert: ATG entry carries only the seeded V1-compat scope when no manifest is present. SpecFor(specs, McpConstants.WorkIQToolsProdAppId).Scopes.Should().BeEquivalentTo(new[] { McpServersMetadataReadAll }, @@ -260,6 +261,7 @@ public async Task Unified_WithManifest_IsM365_StampsFullSet() AuthenticationConstants.MicrosoftGraphResourceAppId, ConfigConstants.MessagingBotApiAppId, ConfigConstants.ObservabilityApiAppId, + ConfigConstants.DefenderApiAppId, PowerPlatformConstants.PowerPlatformApiResourceAppId, McpConstants.WorkIQToolsProdAppId, }, because: "with a manifest present and isM365 true, blueprint agents must receive the same spec set as DW agents — this is the unified-pipeline contract"); @@ -286,6 +288,25 @@ public async Task ObservabilityApi_CarriesBothDelegatedScopeAndAppRole() because: "Observability API app role grants OtelWrite for the s2s path — losing either side breaks one auth mode"); } + [Fact] + public async Task DefenderApi_CarriesBothDelegatedScopeAndAppRole() + { + // Arrange: smallest config that produces the Defender spec on either path. + var config = new Agent365Config { DeploymentProjectPath = _tempDir }; + + // Act + var specs = await SetupHelpers.BuildConfiguredPermissionSpecsAsync(config, setInheritable: true, isM365: true); + + // Assert + var defender = SpecFor(specs, ConfigConstants.DefenderApiAppId); + defender.Scopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiToolInvocationScope }, + because: "the Defender API delegated scope grants ToolInvocation for the OBO path"); + defender.AppRoleScopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiToolInvocationScope }, + because: "the Defender API app role grants ToolInvocation for the s2s path — the Defender webhook rejects tokens without the roles claim, so losing either side breaks one auth mode"); + defender.SetInheritable.Should().BeTrue( + because: "agent identities minted from the blueprint must inherit the Defender permission, exactly as they do for OtelWrite"); + } + [Fact] public async Task MessagingBotApi_UsesScopeConstantSoSpecAndConsentUrlAgree() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs index 4e8d61e1..c5b14b36 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs @@ -61,6 +61,22 @@ public void LogNonDwAdminConsentInstructions_OptionA_ShowsDelegatedPermissionsIn because: "only delegated grants are needed for OBO — no Application permissions"); } + [Fact] + public void LogNonDwAdminConsentInstructions_OptionA_ShowsDefenderDelegatedPermission() + { + var logger = new CapturingLogger(); + + SetupHelpers.LogNonDwAdminConsentInstructions(logger, BlueprintId); + + var defenderLines = logger.Messages + .Where(m => m.Contains("Defender API") && m.Contains(ConfigConstants.DefenderApiToolInvocationScope)) + .ToList(); + defenderLines.Should().HaveCount(1, + because: "the Defender API delegated scope must appear exactly once so the admin grants it alongside the other platform APIs"); + defenderLines[0].Should().Contain("Delegated", + because: "only delegated grants are needed for OBO — no Application permissions"); + } + [Fact] public void LogNonDwAdminConsentInstructions_DoesNotEmitOptionBPowerShell() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index 9607bbb4..13adc210 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs @@ -26,13 +26,14 @@ public void BuildAdminConsentUrls_WithGraphAndMcpScopes_ReturnsUrlForEachResourc var urls = SetupHelpers.BuildAdminConsentUrls(TenantId, BlueprintClientId, graphScopes, mcpScopes); - urls.Should().HaveCount(5); + urls.Should().HaveCount(6); urls.Select(u => u.ResourceName).Should().Contain(new[] { "Microsoft Graph", "Agent 365 Tools", "Messaging Bot API", "Observability API", + "Defender API", "Power Platform API" }); } @@ -72,6 +73,18 @@ public void BuildAdminConsentUrls_ObservabilityApi_UsesCorrectScopeConstant() because: "OtelWrite is the published delegated scope on the Observability API used for admin consent"); } + [Fact] + public void BuildAdminConsentUrls_DefenderApi_UsesHttpsIdentifierUriNotApiScheme() + { + var urls = SetupHelpers.BuildAdminConsentUrls(TenantId, BlueprintClientId, new[] { "Mail.Send" }, new[] { "scope" }); + var defenderUrl = urls.First(u => u.ResourceName == "Defender API").ConsentUrl; + + defenderUrl.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiToolInvocationScope}"), + because: "the Defender resource publishes only the https identifier URI — api://{appId} is not in its servicePrincipalNames and consent fails with AADSTS500011"); + defenderUrl.Should().NotContain(Uri.EscapeDataString($"api://{ConfigConstants.DefenderApiAppId}"), + because: "the api:// form of the Defender resource is not a registered servicePrincipalName"); + } + [Fact] public void BuildAdminConsentUrls_PowerPlatformApi_UsesCorrectScopeConstant() { @@ -211,9 +224,9 @@ public void BuildCombinedConsentUrl_IncludesAllMcpScopes() } [Fact] - public void BuildCombinedConsentUrl_AlwaysIncludesAllThreeFixedResources() + public void BuildCombinedConsentUrl_AlwaysIncludesAllFixedResources() { - // Even with empty graph and MCP scopes, the three fixed resources must be present + // Even with empty graph and MCP scopes, the fixed resources must be present var url = SetupHelpers.BuildCombinedConsentUrl( TenantId, BlueprintClientId, Array.Empty(), Array.Empty()); @@ -222,6 +235,8 @@ public void BuildCombinedConsentUrl_AlwaysIncludesAllThreeFixedResources() because: "scope URIs are Uri.EscapeDataString-encoded in the query string — required by AAD for adminconsent"); url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"), because: "OtelWrite is the published delegated scope on the Observability API used for admin consent"); + url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiToolInvocationScope}"), + because: "ToolInvocation is the published delegated scope on the Defender API — without it the agent cannot call the Defender security webhook"); url.Should().Contain(Uri.EscapeDataString($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}")); } @@ -264,8 +279,9 @@ public void BuildAdminConsentUrls_NonM365_ExcludesMessagingBotButKeepsAllOthers( "Microsoft Graph", "Agent 365 Tools", "Observability API", + "Defender API", "Power Platform API", - }, because: "non-M365 tenants lack the Messaging Bot resource SP — Bot would cause AADSTS650053 if included"); + }, because: "non-M365 tenants lack the Messaging Bot resource SP — Bot would cause AADSTS650053 if included; the Defender API is required for the security integration on every agent regardless of M365 surface"); } [Fact] From a345450c653c069e0063b1604148047665da1e61 Mon Sep 17 00:00:00 2001 From: slreznit Date: Tue, 11 Aug 2026 16:24:18 +0300 Subject: [PATCH 02/14] Rename Defender app role to RealtimeProtection.Process Updates the Defender API app role and delegated scope value from AIAgentsRTP.ToolInvocation to RealtimeProtection.Process, and renames the constant accordingly since the old name no longer described the role. NOT YET VERIFIED AGAINST A LIVE RESOURCE. The resource SP still publishes AIAgentsRTP.ToolInvocation in the agent365003 tenant, and the resource is not provisioned in the corp tenant at all, so the new value could not be confirmed anywhere. Until the Defender-side rename ships, the combined /v2.0/adminconsent URL will fail with AADSTS650053 for every resource in the request - Graph, MCP, Bot, Observability and Power Platform - not just Defender, and the S2S app role lookup will find no matching appRoles entry. Confirm the resource publishes the new value before merging. Adds DefenderApi_ScopeValue_MatchesValuePublishedOnResource pinning the literal string so future drift fails a test that explains the blast radius. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- .../SetupSubcommands/PermissionsSubcommand.cs | 2 +- .../Commands/SetupSubcommands/SetupHelpers.cs | 16 ++++++++-------- .../Constants/ConfigConstants.cs | 9 +++++---- .../SetupSubcommands/PermissionSpecsTests.cs | 18 ++++++++++++++---- ...etupHelpersAdminConsentInstructionsTests.cs | 2 +- .../Helpers/SetupHelpersConsentUrlTests.cs | 6 +++--- 7 files changed, 34 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e097c4e3..4ee34c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,10 +24,10 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g #### Existing agents: grant Defender API permissions -Agents provisioned before this release need `AIAgentsRTP.ToolInvocation` granted as both a **delegated** and an **application** permission on the blueprint app for the Defender security integration. Requires Global Administrator. Follow the steps above, searching for `86a21212-634e-4553-b3d6-e477e4c9d9ec` in step 2 and selecting `AIAgentsRTP.ToolInvocation` in steps 3 and 4. Re-running `a365 setup all` grants it automatically. +Agents provisioned before this release need `RealtimeProtection.Process` granted as both a **delegated** and an **application** permission on the blueprint app for the Defender security integration. Requires Global Administrator. Follow the steps above, searching for `86a21212-634e-4553-b3d6-e477e4c9d9ec` in step 2 and selecting `RealtimeProtection.Process` in steps 3 and 4. Re-running `a365 setup all` grants it automatically. ### Added -- `AIAgentsRTP.ToolInvocation` on the Defender API is now granted automatically during `a365 setup` as both a delegated and an application permission, enabling the Microsoft Defender security integration without manual Entra steps. +- `RealtimeProtection.Process` on the Defender API is now granted automatically during `a365 setup` as both a delegated and an application permission, enabling the Microsoft Defender security integration without manual Entra steps. - Log separator written at the start of each CLI invocation now redacts values for secret-bearing options (e.g. `--idp-client-secret`) so they are not written to the log file in plain text. - Authentication context (tenant and user) is now logged at the `Information` level whenever the resolved sign-in identity changes, giving operators a clear audit trail in the log file of who the CLI is acting as, without exposing credentials. - `a365 develop-mcp evaluate` command for evaluating MCP server tool schema quality — runs deterministic and semantic checks (via GitHub Copilot or Claude Code CLIs), computes maturity scoring, and generates an interactive HTML report diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index e9d12b08..0f32b988 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -310,7 +310,7 @@ private static Command CreateBotSubcommand( logger.LogInformation(" - Blueprint: {BlueprintId}", dryRunConfig.AgentBlueprintId); logger.LogInformation(" - Messaging Bot API: {Scope}", ConfigConstants.MessagingBotApiAdminConsentScope); logger.LogInformation(" - Observability API: {OtelScope} (delegated + application)", ConfigConstants.ObservabilityApiOtelWriteScope); - logger.LogInformation(" - Defender API: {DefenderScope} (delegated + application)", ConfigConstants.DefenderApiToolInvocationScope); + logger.LogInformation(" - Defender API: {DefenderScope} (delegated + application)", ConfigConstants.DefenderApiRealtimeProtectionScope); logger.LogInformation(" - Power Platform API: Connectivity.Connections.Read"); logger.LogInformation("No changes made. Run without --dry-run to execute."); return; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index a24bef0b..0f0e69a8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -82,9 +82,9 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInhe specs.Add(new ResourcePermissionSpec( ConfigConstants.DefenderApiAppId, "Defender API", - new[] { ConfigConstants.DefenderApiToolInvocationScope }, + new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, setInheritable, - AppRoleScopes: new[] { ConfigConstants.DefenderApiToolInvocationScope })); + AppRoleScopes: new[] { ConfigConstants.DefenderApiRealtimeProtectionScope })); specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -377,8 +377,8 @@ internal static async Task> BuildConfiguredPermissi [ ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Application"), ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Delegated"), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiToolInvocationScope, "Application"), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiToolInvocationScope, "Delegated"), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application"), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated"), ("Power Platform API", PowerPlatformConstants.PowerPlatformApiResourceAppId, PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, "Delegated"), ]; @@ -389,7 +389,7 @@ internal static async Task> BuildConfiguredPermissi internal static readonly IReadOnlyList<(string ResourceName, string ResourceAppId, string Role)> FixedApiAppRoleHandoffSpecs = [ ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiToolInvocationScope), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope), ]; /// @@ -913,7 +913,7 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) logger.LogInformation(""); logger.LogInformation(" # Defender API"); logger.LogInformation(" $defenderSp = Get-MgServicePrincipal -Filter \"appId eq '{DefenderAppId}'\"", ConfigConstants.DefenderApiAppId); - logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $defenderSp.Id; scope = '{DefenderScope}' }} | ConvertTo-Json", ConfigConstants.DefenderApiToolInvocationScope); + logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $defenderSp.Id; scope = '{DefenderScope}' }} | ConvertTo-Json", ConfigConstants.DefenderApiRealtimeProtectionScope); logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri 'https://graph.microsoft.com/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'"); logger.LogInformation(""); logger.LogInformation(" # Power Platform API"); @@ -1352,7 +1352,7 @@ static string Build(string tenant, string client, string resourceUri, IEnumerabl urls.Add(("Messaging Bot API", Build(tenantId, blueprintClientId, ConfigConstants.MessagingBotApiIdentifierUri, new[] { ConfigConstants.MessagingBotApiAdminConsentScope }))); urls.Add(("Observability API", Build(tenantId, blueprintClientId, ConfigConstants.ObservabilityApiIdentifierUri, new[] { ConfigConstants.ObservabilityApiOtelWriteScope }))); - urls.Add(("Defender API", Build(tenantId, blueprintClientId, ConfigConstants.DefenderApiIdentifierUri, new[] { ConfigConstants.DefenderApiToolInvocationScope }))); + urls.Add(("Defender API", Build(tenantId, blueprintClientId, ConfigConstants.DefenderApiIdentifierUri, new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }))); urls.Add(("Power Platform API", Build(tenantId, blueprintClientId, PowerPlatformConstants.PowerPlatformApiIdentifierUri, new[] { PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead }))); return urls; @@ -1408,7 +1408,7 @@ internal static string BuildCombinedConsentUrl( if (isM365) allScopes.Add($"{ConfigConstants.MessagingBotApiIdentifierUri}/{ConfigConstants.MessagingBotApiAdminConsentScope}"); allScopes.Add($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); - allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiToolInvocationScope}"); + allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes); } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index a6a808c4..703a8a9c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -110,11 +110,12 @@ public static class ConfigConstants public const string ObservabilityApiOtelWriteScope = "Agent365.Observability.OtelWrite"; /// - /// Defender API scope for tool invocation inspection, enabling the Defender - /// security integration. Published on the resource as both a delegated scope (OBO) and an - /// application app role (S2S), so it is granted through both paths like OtelWrite. + /// Defender API app role and delegated scope enabling the Defender security integration. + /// Published on the resource as both a delegated scope (OBO) and an application app role + /// (S2S), so it is granted through both paths like OtelWrite. Must match the value published + /// on the resource SP — a mismatch fails the combined consent URL with AADSTS650053. /// - public const string DefenderApiToolInvocationScope = "AIAgentsRTP.ToolInvocation"; + public const string DefenderApiRealtimeProtectionScope = "RealtimeProtection.Process"; /// /// Delegated scope value exposed on the blueprint app registration to enable diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs index 38d4d71b..967f6475 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs @@ -299,14 +299,24 @@ public async Task DefenderApi_CarriesBothDelegatedScopeAndAppRole() // Assert var defender = SpecFor(specs, ConfigConstants.DefenderApiAppId); - defender.Scopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiToolInvocationScope }, - because: "the Defender API delegated scope grants ToolInvocation for the OBO path"); - defender.AppRoleScopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiToolInvocationScope }, - because: "the Defender API app role grants ToolInvocation for the s2s path — the Defender webhook rejects tokens without the roles claim, so losing either side breaks one auth mode"); + defender.Scopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, + because: "the Defender API delegated scope grants RealtimeProtection.Process for the OBO path"); + defender.AppRoleScopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, + because: "the Defender API app role grants RealtimeProtection.Process for the s2s path — the Defender webhook rejects tokens without the roles claim, so losing either side breaks one auth mode"); defender.SetInheritable.Should().BeTrue( because: "agent identities minted from the blueprint must inherit the Defender permission, exactly as they do for OtelWrite"); } + [Fact] + public void DefenderApi_ScopeValue_MatchesValuePublishedOnResource() + { + // The combined /v2.0/adminconsent URL validates every scope against the resource SP and + // rejects the ENTIRE url with AADSTS650053 on any unknown value — so a drift here breaks + // consent for Graph, MCP, Bot, Observability and Power Platform too, not just Defender. + ConfigConstants.DefenderApiRealtimeProtectionScope.Should().Be("RealtimeProtection.Process", + because: "this is the app role and delegated scope value published on the Defender resource SP; changing it without a matching resource-side change fails admin consent tenant-wide"); + } + [Fact] public async Task MessagingBotApi_UsesScopeConstantSoSpecAndConsentUrlAgree() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs index c5b14b36..7bbd0a75 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs @@ -69,7 +69,7 @@ public void LogNonDwAdminConsentInstructions_OptionA_ShowsDefenderDelegatedPermi SetupHelpers.LogNonDwAdminConsentInstructions(logger, BlueprintId); var defenderLines = logger.Messages - .Where(m => m.Contains("Defender API") && m.Contains(ConfigConstants.DefenderApiToolInvocationScope)) + .Where(m => m.Contains("Defender API") && m.Contains(ConfigConstants.DefenderApiRealtimeProtectionScope)) .ToList(); defenderLines.Should().HaveCount(1, because: "the Defender API delegated scope must appear exactly once so the admin grants it alongside the other platform APIs"); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index 13adc210..023c71d7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs @@ -79,7 +79,7 @@ public void BuildAdminConsentUrls_DefenderApi_UsesHttpsIdentifierUriNotApiScheme var urls = SetupHelpers.BuildAdminConsentUrls(TenantId, BlueprintClientId, new[] { "Mail.Send" }, new[] { "scope" }); var defenderUrl = urls.First(u => u.ResourceName == "Defender API").ConsentUrl; - defenderUrl.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiToolInvocationScope}"), + defenderUrl.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), because: "the Defender resource publishes only the https identifier URI — api://{appId} is not in its servicePrincipalNames and consent fails with AADSTS500011"); defenderUrl.Should().NotContain(Uri.EscapeDataString($"api://{ConfigConstants.DefenderApiAppId}"), because: "the api:// form of the Defender resource is not a registered servicePrincipalName"); @@ -235,8 +235,8 @@ public void BuildCombinedConsentUrl_AlwaysIncludesAllFixedResources() because: "scope URIs are Uri.EscapeDataString-encoded in the query string — required by AAD for adminconsent"); url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"), because: "OtelWrite is the published delegated scope on the Observability API used for admin consent"); - url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiToolInvocationScope}"), - because: "ToolInvocation is the published delegated scope on the Defender API — without it the agent cannot call the Defender security webhook"); + url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), + because: "RealtimeProtection.Process is the published delegated scope on the Defender API — without it the agent cannot call the Defender security webhook"); url.Should().Contain(Uri.EscapeDataString($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}")); } From 42cf82e054a7bccb06fb960b396437e86179f35b Mon Sep 17 00:00:00 2001 From: slreznit Date: Tue, 11 Aug 2026 17:12:58 +0300 Subject: [PATCH 03/14] Trim verbose doc comments on Defender constants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Constants/ConfigConstants.cs | 12 ++++-------- .../SetupSubcommands/PermissionSpecsTests.cs | 5 ++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index 703a8a9c..6ea1deab 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -81,14 +81,12 @@ public static class ConfigConstants public const string ObservabilityApiIdentifierUri = "api://9b975845-388f-4429-889e-eab1ef63949c"; /// - /// Defender API App ID. Hosts the security inspection endpoint used by the Defender integration. + /// Defender API App ID /// public const string DefenderApiAppId = "86a21212-634e-4553-b3d6-e477e4c9d9ec"; /// - /// Defender API identifier URI. Unlike the Observability API this resource - /// publishes an https identifier URI only — api://{appId} is not in its - /// servicePrincipalNames and consent fails with AADSTS500011. + /// Defender API identifier URI. /// public const string DefenderApiIdentifierUri = "https://rtp-a365.ai.defender.microsoft.com"; @@ -110,10 +108,8 @@ public static class ConfigConstants public const string ObservabilityApiOtelWriteScope = "Agent365.Observability.OtelWrite"; /// - /// Defender API app role and delegated scope enabling the Defender security integration. - /// Published on the resource as both a delegated scope (OBO) and an application app role - /// (S2S), so it is granted through both paths like OtelWrite. Must match the value published - /// on the resource SP — a mismatch fails the combined consent URL with AADSTS650053. + /// Defender API app role and delegated scope for the Defender security integration. + /// Must match the value published on the resource SP. /// public const string DefenderApiRealtimeProtectionScope = "RealtimeProtection.Process"; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs index 967f6475..9f5727fc 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs @@ -310,9 +310,8 @@ public async Task DefenderApi_CarriesBothDelegatedScopeAndAppRole() [Fact] public void DefenderApi_ScopeValue_MatchesValuePublishedOnResource() { - // The combined /v2.0/adminconsent URL validates every scope against the resource SP and - // rejects the ENTIRE url with AADSTS650053 on any unknown value — so a drift here breaks - // consent for Graph, MCP, Bot, Observability and Power Platform too, not just Defender. + // A value the resource SP does not publish fails the combined consent URL for every + // resource in it (AADSTS650053), not just Defender. ConfigConstants.DefenderApiRealtimeProtectionScope.Should().Be("RealtimeProtection.Process", because: "this is the app role and delegated scope value published on the Defender resource SP; changing it without a matching resource-side change fails admin consent tenant-wide"); } From e4aa96976145ddebd93e767f5f93e9cb15303df6 Mon Sep 17 00:00:00 2001 From: slreznit Date: Mon, 5 Oct 2026 23:11:49 +0300 Subject: [PATCH 04/14] Update Defender permission and audience Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- .../Constants/ConfigConstants.cs | 4 ++-- .../Commands/SetupSubcommands/PermissionSpecsTests.cs | 6 +++--- .../Helpers/SetupHelpersConsentUrlTests.cs | 8 +++----- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eaf59aa..e7f497d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,10 +30,10 @@ Blueprint agents that export telemetry through the app-only S2S endpoint don't n #### Existing agents: grant Defender API permissions -Agents provisioned before this release need `RealtimeProtection.Process` granted as both a **delegated** and an **application** permission on the blueprint app for the Defender security integration. Requires Global Administrator. Follow the steps above, searching for `86a21212-634e-4553-b3d6-e477e4c9d9ec` in step 2 and selecting `RealtimeProtection.Process` in steps 3 and 4. Re-running `a365 setup all` grants it automatically. +Agents provisioned before this release need `RealtimeProtection.Evaluate.All` granted as both a **delegated** and an **application** permission on the blueprint app for the Defender security integration. Requires Global Administrator. Follow the steps above, searching for `86a21212-634e-4553-b3d6-e477e4c9d9ec` in step 2 and selecting `RealtimeProtection.Evaluate.All` in steps 3 and 4. Re-running `a365 setup all` grants it automatically. ### Added -- `RealtimeProtection.Process` on the Defender API is now granted automatically during `a365 setup` as both a delegated and an application permission, enabling the Microsoft Defender security integration without manual Entra steps. +- `RealtimeProtection.Evaluate.All` on the Defender API is now granted automatically during `a365 setup` as both a delegated and an application permission, enabling the Microsoft Defender security integration without manual Entra steps. - `a365 develop-mcp grant-agents-access --agent-blueprint-id --mcp-server-name ` reports which agent instances of a blueprint are missing the permission to call a BYO MCP server, and prompts you to select which ones to grant it to (#500). - When more than one Entra application shares the MCP server's name, `a365 develop-mcp grant-agents-access` now lists them all and asks which one to use instead of failing (#500). - `a365 develop-mcp grant-agents-access --help` now lists Microsoft's first-party agent blueprint names and IDs, and the same list is printed when `--agent-blueprint-id` is missing or not a GUID, so you can find the ID without looking it up elsewhere (#500). diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index e1064de7..1a7618e8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -116,7 +116,7 @@ public static class ConfigConstants /// /// Defender API identifier URI. /// - public const string DefenderApiIdentifierUri = "https://rtp-a365.ai.defender.microsoft.com"; + public const string DefenderApiIdentifierUri = "api://86a21212-634e-4553-b3d6-e477e4c9d9ec"; /// /// Single source of truth for the Messaging Bot API delegated scope. @@ -139,7 +139,7 @@ public static class ConfigConstants /// Defender API app role and delegated scope for the Defender security integration. /// Must match the value published on the resource SP. /// - public const string DefenderApiRealtimeProtectionScope = "RealtimeProtection.Process"; + public const string DefenderApiRealtimeProtectionScope = "RealtimeProtection.Evaluate.All"; /// /// Delegated scope value exposed on the blueprint app registration to enable diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs index 48d4dbba..8ca94b16 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs @@ -320,9 +320,9 @@ public async Task DefenderApi_CarriesBothDelegatedScopeAndAppRole() // Assert var defender = SpecFor(specs, ConfigConstants.DefenderApiAppId); defender.Scopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, - because: "the Defender API delegated scope grants RealtimeProtection.Process for the OBO path"); + because: "the Defender API delegated scope grants RealtimeProtection.Evaluate.All for the OBO path"); defender.AppRoleScopes.Should().BeEquivalentTo(new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, - because: "the Defender API app role grants RealtimeProtection.Process for the s2s path — the Defender webhook rejects tokens without the roles claim, so losing either side breaks one auth mode"); + because: "the Defender API app role grants RealtimeProtection.Evaluate.All for the s2s path - the Defender webhook rejects tokens without the roles claim, so losing either side breaks one auth mode"); defender.SetInheritable.Should().BeTrue( because: "agent identities minted from the blueprint must inherit the Defender permission, exactly as they do for OtelWrite"); } @@ -332,7 +332,7 @@ public void DefenderApi_ScopeValue_MatchesValuePublishedOnResource() { // A value the resource SP does not publish fails the combined consent URL for every // resource in it (AADSTS650053), not just Defender. - ConfigConstants.DefenderApiRealtimeProtectionScope.Should().Be("RealtimeProtection.Process", + ConfigConstants.DefenderApiRealtimeProtectionScope.Should().Be("RealtimeProtection.Evaluate.All", because: "this is the app role and delegated scope value published on the Defender resource SP; changing it without a matching resource-side change fails admin consent tenant-wide"); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index 78227561..7afca5c1 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs @@ -74,15 +74,13 @@ public void BuildAdminConsentUrls_ObservabilityApi_UsesCorrectScopeConstant() } [Fact] - public void BuildAdminConsentUrls_DefenderApi_UsesHttpsIdentifierUriNotApiScheme() + public void BuildAdminConsentUrls_DefenderApi_UsesAppIdIdentifierUri() { var urls = SetupHelpers.BuildAdminConsentUrls(TenantId, BlueprintClientId, new[] { "Mail.Send" }, new[] { "scope" }); var defenderUrl = urls.First(u => u.ResourceName == "Defender API").ConsentUrl; defenderUrl.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), - because: "the Defender resource publishes only the https identifier URI — api://{appId} is not in its servicePrincipalNames and consent fails with AADSTS500011"); - defenderUrl.Should().NotContain(Uri.EscapeDataString($"api://{ConfigConstants.DefenderApiAppId}"), - because: "the api:// form of the Defender resource is not a registered servicePrincipalName"); + because: "the Defender resource publishes its delegated permission under the api://{appId} audience"); } [Fact] @@ -240,7 +238,7 @@ public void BuildCombinedConsentUrl_AlwaysIncludesAllFixedResources() url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"), because: "OtelWrite is the published delegated scope on the Observability API used for admin consent"); url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), - because: "RealtimeProtection.Process is the published delegated scope on the Defender API — without it the agent cannot call the Defender security webhook"); + because: "RealtimeProtection.Evaluate.All is the published delegated scope on the Defender API - without it the agent cannot call the Defender security webhook"); url.Should().Contain(Uri.EscapeDataString($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}")); } From 50cd7bd97a4adc566a87840ae4c72f1913d900bc Mon Sep 17 00:00:00 2001 From: slreznit Date: Tue, 6 Oct 2026 00:13:30 +0300 Subject: [PATCH 05/14] Align Defender permissions with auth mode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + .../SetupSubcommands/AllSubcommand.cs | 9 +- .../BatchPermissionsOrchestrator.cs | 22 ++-- .../NonDwBlueprintSetupOrchestrator.cs | 26 ++-- .../ResourcePermissionSpec.cs | 7 ++ .../Commands/SetupSubcommands/SetupHelpers.cs | 111 ++++++++++++------ .../Commands/SetupSubcommands/SetupResults.cs | 2 +- .../Commands/AllSubcommandTests.cs | 88 +++++++++++++- .../BatchPermissionsOrchestratorTests.cs | 15 ++- .../Commands/SetupCommandTests.cs | 12 +- ...tupHelpersAdminConsentInstructionsTests.cs | 20 ++++ .../Helpers/SetupHelpersConsentUrlTests.cs | 26 ++++ 12 files changed, 267 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f497d4..79d1d735 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,6 +139,7 @@ Agents provisioned before this release need `RealtimeProtection.Evaluate.All` gr ### Changed +- Defender permissions now follow the blueprint agent authentication mode: delegated for `obo`, application for `s2s`, and both for `both`; AI Teammate setup continues to request both permission types (#485). - `a365 setup all` no longer requests Observability API permissions for blueprint agents; registered agents that export telemetry through the app-only S2S endpoint need no admin consent (#501). - Hardened token storage: the CLI no longer writes access tokens to a plaintext file — they live only in the OS-protected MSAL cache (DPAPI/Keychain/owner-only file). Any legacy plaintext cache is removed automatically; sign-in prompts are unchanged. - `develop-mcp register-external-mcp-server` now sets `exit code 1` on failure paths (validation errors, tenant detection failure, Graph unavailable, Entra app creation failure, MCP-Platform AddMcpServer failure). Previously these paths logged an error and exited `0`, which made the command's success/failure status undetectable from scripts and CI. Successful dry-run and user-initiated cancellation at the y/N prompt continue to exit `0`. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs index 49ace519..65987112 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -1020,7 +1020,14 @@ await PermissionsSubcommand.RemoveStaleCustomPermissionsAsync( // names so V2 audiences read as e.g. "mcp_MailTools" rather than "Agent 365 Tools". var specs = await SetupHelpers.BuildConfiguredPermissionSpecsAsync( ctx.Config, setInheritable: true, isM365: ctx.IsM365, scopesByAudience, serverNamesByAudience, - includeObservability: !ctx.SkipObservabilityPermissions); + includeObservability: !ctx.SkipObservabilityPermissions, + defenderPermissionMode: ctx.Results.IsNonDwBlueprintFlow + ? ctx.IsS2sMode + ? DefenderPermissionMode.Application + : ctx.IsBothMode + ? DefenderPermissionMode.Both + : DefenderPermissionMode.Delegated + : DefenderPermissionMode.Both); // Return the full scopesByAudience map alongside the V1-compat mcpScopes so V2 // callers (ApplyConsentUrlsIfNeeded) can route per-server audiences to the bare diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index c4a23c67..4b417648 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -84,9 +84,11 @@ internal static class BatchPermissionsOrchestrator return (true, true, true, null); } - // Filter out specs with no scopes — they would produce empty OAuth2 grants (HTTP 400). - // This can happen when the MCP manifest is missing or contains no required scopes. - var effectiveSpecs = specs.Where(s => s.Scopes.Length > 0).ToList(); + // Drop specs that carry neither delegated scopes nor application roles. Application-only + // specs must remain so S2S mode can assign app roles without creating an OAuth2 grant. + var effectiveSpecs = specs + .Where(s => s.Scopes.Length > 0 || s.AppRoleScopes is { Length: > 0 }) + .ToList(); if (setupResults is not null) { setupResults.ObservabilityResourceAppId = effectiveSpecs @@ -97,12 +99,12 @@ internal static class BatchPermissionsOrchestrator if (effectiveSpecs.Count < specs.Count) { var skipped = specs.Count - effectiveSpecs.Count; - logger.LogDebug("Skipping {Count} resource spec(s) with no scopes (manifest missing or empty).", skipped); + logger.LogDebug("Skipping {Count} resource spec(s) with no delegated scopes or application roles.", skipped); } if (effectiveSpecs.Count == 0) { - logger.LogInformation("All permission specs have empty scope lists — skipping batch permissions configuration."); + logger.LogInformation("All permission specs have empty delegated scope and application role lists — skipping batch permissions configuration."); return (true, true, true, null); } @@ -415,12 +417,16 @@ private static async Task UpdateBlueprintPermissions continue; } + var permissionValues = spec.Scopes + .Concat(spec.AppRoleScopes ?? Array.Empty()) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); logger.LogDebug( - " - Configuring inheritable permissions: {ResourceName} [{Scopes}]", - spec.ResourceName, string.Join(' ', spec.Scopes)); + " - Configuring inheritable permissions: {ResourceName} [{Permissions}]", + spec.ResourceName, string.Join(' ', permissionValues)); var (ok, alreadyExists, err) = await blueprintService.SetInheritablePermissionsAsync( - tenantId, blueprintAppId, spec.ResourceAppId, spec.Scopes, + tenantId, blueprintAppId, spec.ResourceAppId, permissionValues, requiredScopes: permScopes, ct); if (alreadyExists || ok) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index 0b8809f9..e8d54ea3 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -36,6 +36,16 @@ internal static class NonDwBlueprintSetupOrchestrator public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool isBootstrap = false, string[]? rawArgs = null, bool skipRequirements = false, bool isM365 = false, bool agentRegistrationOnly = false, string? authMode = null, string? messagingEndpointOverride = null, bool skipObservabilityPermissions = false) { var sub = new string(' ', SetupHelpers.DryRunValCol); + var selectedAuthMode = authMode ?? config.AuthMode; + var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) + ? "obo" + : selectedAuthMode.Trim().ToLowerInvariant(); + var defenderPermissionMode = effectiveMode switch + { + "s2s" => DefenderPermissionMode.Application, + "both" => DefenderPermissionMode.Both, + _ => DefenderPermissionMode.Delegated, + }; var observabilityPermissionsEffectivelySkipped = skipObservabilityPermissions && !SetupHelpers.CustomPermissionsRequestObservability(config); // Dry-run S2S work comes only from fixed specs today; MCP and custom specs carry delegated scopes. @@ -43,7 +53,8 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i setInheritable: true, isM365, config.Environment, - includeObservability: !skipObservabilityPermissions) + includeObservability: !skipObservabilityPermissions, + defenderPermissionMode) .Any(s => s.AppRoleScopes is { Length: > 0 }); // --messaging-endpoint flag (if supplied) wins over the init-only config value for the plan. var plannedEndpoint = !string.IsNullOrWhiteSpace(messagingEndpointOverride) @@ -128,10 +139,6 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i // 3. Inheritable Permissions — the non-DW spec set is stamped on the blueprint so MAC and // dependent systems can see it. The same set is applied to the agent identity SP in step 5. - var selectedAuthMode = authMode ?? config.AuthMode; - var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) - ? "obo" - : selectedAuthMode.Trim().ToLowerInvariant(); logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for {Resources} (Global Administrator required; consent URL printed if absent)", skipObservabilityPermissions ? "Defender API, Power Platform API, and custom permissions" @@ -283,6 +290,11 @@ await ctx.ClientAppValidator.GrantConsentForPermissionsAsync( public static async Task ExecuteAsync(SetupContext ctx) { ctx.Results.IsNonDwBlueprintFlow = true; + ctx.Results.EffectiveAuthMode = ctx.IsBothMode + ? Models.AuthMode.Both + : ctx.IsS2sMode + ? Models.AuthMode.S2s + : Models.AuthMode.Obo; ctx.Results.ObservabilityPermissionsSkipped = ctx.ObservabilityPermissionsEffectivelySkipped; ctx.Results.TenantId = ctx.Config.TenantId; // Bootstrap already printed the "Running..." banner before auth steps; skip here to avoid duplication. @@ -723,8 +735,8 @@ internal static async Task GrantAgentIdentityS2SPermissionsAsync( List specs) { var hasS2sSpecs = specs.Any(s => s.AppRoleScopes is { Length: > 0 }); - // Blueprint agents no longer request OtelWrite, the only app role setup requested, so this - // step usually has nothing to grant. Record that so the summary does not report a delegated grant. + // Record whether the selected auth mode produced any application permissions so the + // summary can distinguish "no S2S work" from a failed grant. ctx.Results.NoS2SAppRolesToGrant = !hasS2sSpecs; if (hasS2sSpecs && AgentIdentityInheritsBlueprintAppRoles(ctx.Results)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs index 5ca5d19e..1c363c4c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs @@ -3,6 +3,13 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; +internal enum DefenderPermissionMode +{ + Delegated, + Application, + Both, +} + /// /// Describes a single resource whose permissions should be configured on the agent blueprint. /// Used as input to . diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 33eef967..16076585 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -59,7 +59,8 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( bool setInheritable, bool isM365, string? environment = null, - bool includeObservability = true) + bool includeObservability = true, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { var specs = new List(); if (isM365) @@ -87,12 +88,18 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( setInheritable, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); } + var defenderDelegatedScopes = defenderPermissionMode is DefenderPermissionMode.Delegated or DefenderPermissionMode.Both + ? new[] { ConfigConstants.DefenderApiRealtimeProtectionScope } + : Array.Empty(); + var defenderAppRoleScopes = defenderPermissionMode is DefenderPermissionMode.Application or DefenderPermissionMode.Both + ? new[] { ConfigConstants.DefenderApiRealtimeProtectionScope } + : null; specs.Add(new ResourcePermissionSpec( ConfigConstants.DefenderApiAppId, "Defender API", - new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, + defenderDelegatedScopes, setInheritable, - AppRoleScopes: new[] { ConfigConstants.DefenderApiRealtimeProtectionScope })); + AppRoleScopes: defenderAppRoleScopes)); specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -147,7 +154,8 @@ internal static async Task> BuildConfiguredPermissi bool isM365 = true, Dictionary? scopesByAudience = null, Dictionary>? serverNamesByAudience = null, - bool includeObservability = true) + bool includeObservability = true, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { // Manifest read at most once, and only when scopesByAudience is not pre-supplied. // Callers that already have the manifest loaded (e.g. AllSubcommand.BuildPermissionSpecsAsync) @@ -186,7 +194,8 @@ internal static async Task> BuildConfiguredPermissi : "Agent 365 Tools", kvp.Value, SetInheritable: setInheritable))); - specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365, config.Environment, includeObservability)); + specs.AddRange(GetFixedApiPermissionSpecs( + setInheritable, isM365, config.Environment, includeObservability, defenderPermissionMode)); foreach (var customPerm in config.CustomBlueprintPermissions ?? new List()) { @@ -475,8 +484,8 @@ internal static async Task ResolveBootstrapEnvironmentAsync( /// /// Fixed permission specs for the non-DW admin consent flow. - /// Observability API and Defender API require both Application (app role for S2S) - /// and Delegated (oauth2 grant for OBO). Power Platform API requires Delegated only. + /// Observability API includes both permission types when requested. Defender follows the + /// selected auth mode. Power Platform API requires Delegated only. /// Extend this list or pass an override to /// when additional APIs are required (e.g. dynamic MCP scopes, custom permissions). /// @@ -484,20 +493,28 @@ internal static async Task ResolveBootstrapEnvironmentAsync( GetNonDwAdminConsentSpecs("prod"); internal static IReadOnlyList<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> GetNonDwAdminConsentSpecs( - string? environment) - => BuildNonDwAdminConsentSpecs(ConfigConstants.GetObservabilityApiAppId(environment)); + string? environment, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) + => BuildNonDwAdminConsentSpecs(ConfigConstants.GetObservabilityApiAppId(environment), defenderPermissionMode); private static IReadOnlyList<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> BuildNonDwAdminConsentSpecs( - string observabilityAppId) + string observabilityAppId, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { - return - [ + var specs = new List<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> + { ("Observability API", observabilityAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Application"), ("Observability API", observabilityAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Delegated"), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application"), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated"), ("Power Platform API", PowerPlatformConstants.PowerPlatformApiResourceAppId, PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, "Delegated"), - ]; + }; + + if (defenderPermissionMode is DefenderPermissionMode.Application or DefenderPermissionMode.Both) + specs.Insert(2, ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application")); + if (defenderPermissionMode is DefenderPermissionMode.Delegated or DefenderPermissionMode.Both) + specs.Insert(defenderPermissionMode == DefenderPermissionMode.Both ? 3 : 2, + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated")); + + return specs; } /// @@ -675,9 +692,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str // (e.g. admin already granted tenant consent but the per-principal call still failed). var pendingDelegatedAction = agentIdDelegatedFailed && !pendingAdminAction; var pendingS2SAction = permissionGrantsPending && isS2SFlow; - // Blueprint agents no longer request OtelWrite, the only app role setup requested, so an - // s2s/both run usually has no S2S grant at all. Say so explicitly: otherwise the row falls - // through to the delegated wording, which for s2s-only shows a PENDING with no action item. + // Some configurations can have no application-role specs. Say so explicitly; otherwise + // an s2s-only row can fall through to delegated wording with no matching action item. var noS2SAppRolesToGrant = isNonDw && (isS2sOnlyMode || isBothMode) && results.NoS2SAppRolesToGrant && !isS2SFlow; if (results.PermissionGrantsSkipped && isNonDw) @@ -967,7 +983,13 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str if (isNonDw && string.IsNullOrWhiteSpace(consentUrl)) { logger.LogInformation(" {N}. Permission Grants — must be granted by {Roles} in the Entra portal:", actionCount, AuthenticationConstants.DelegatedGrantRequiredRoles); - var consentSpecs = BuildNonDwAdminConsentSpecs(observabilityResourceAppId); + var defenderPermissionMode = results.EffectiveAuthMode switch + { + Models.AuthMode.S2s => DefenderPermissionMode.Application, + Models.AuthMode.Both => DefenderPermissionMode.Both, + _ => DefenderPermissionMode.Delegated, + }; + var consentSpecs = BuildNonDwAdminConsentSpecs(observabilityResourceAppId, defenderPermissionMode); if (results.ObservabilityPermissionsSkipped) consentSpecs = consentSpecs.Where(s => !ConfigConstants.IsObservabilityApiAppId(s.ResourceAppId)).ToList(); LogNonDwAdminConsentInstructions( @@ -1259,10 +1281,11 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str /// resources. Called when the current user lacks the Global Administrator role so that the URLs /// can be saved to a365.generated.config.json and shared with a tenant administrator. /// - /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated; the Observability - /// API URL is generated unless is false. Messaging Bot API is included only - /// when is true — non-M365 tenants typically lack the Messaging Bot - /// resource SP and the consent endpoint returns AADSTS650053 otherwise. + /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated. Defender is + /// generated when delegated Defender consent is enabled, and Observability is generated unless + /// is false. Messaging Bot API is included only when + /// is true — non-M365 tenants typically lack the Messaging Bot resource + /// SP and the consent endpoint returns AADSTS650053 otherwise. /// /// /// Display names of the resources for which URLs were saved. @@ -1273,7 +1296,8 @@ internal static List PopulateAdminConsentUrls( bool isM365 = true, IReadOnlyDictionary? mcpScopesByAudience = null, IReadOnlyDictionary>? mcpAudienceDisplayNames = null, - bool includeObservability = true) + bool includeObservability = true, + bool includeDefenderDelegated = true) { var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(config.Environment, config.GraphBaseUrl); var graphResourceUri = graphBaseUrl; @@ -1283,7 +1307,7 @@ internal static List PopulateAdminConsentUrls( var urls = BuildAdminConsentUrls( config.TenantId, config.AgentBlueprintId!, config.AgentApplicationScopes, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId, includeObservability); + mcpResourceAppId, observabilityResourceAppId, includeObservability, includeDefenderDelegated); // Clear an Observability consent URL saved by an earlier run so the admin is not asked for permissions this run skipped. if (!includeObservability) @@ -1469,7 +1493,8 @@ internal static string BuildFullyQualifiedScope( /// (mirrors ): Microsoft Graph (when /// non-empty), Agent 365 Tools (when /// non-empty), Messaging Bot API (when is true), Observability API - /// (unless is false), and Power Platform API. + /// (unless is false), Defender API (when delegated + /// Defender consent is enabled), and Power Platform API. /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, in which case the /v2.0/adminconsent endpoint returns @@ -1489,7 +1514,8 @@ internal static string BuildFullyQualifiedScope( string? authorityHost = null, string? sharedMcpResourceAppId = null, string? observabilityResourceAppId = null, - bool includeObservability = true) + bool includeObservability = true, + bool includeDefenderDelegated = true) { var urls = new List<(string, string)>(); @@ -1558,7 +1584,8 @@ string Build(string tenant, string client, string resourceUri, IEnumerable /// Builds a single combined /v2.0/adminconsent URL covering every resource stamped on the /// blueprint: Graph, Agent 365 Tools (MCP), Observability API (unless - /// is false), Power Platform API, and - /// Messaging Bot API (only when is true). + /// is false), Defender API when delegated Defender + /// consent is enabled, Power Platform API, and Messaging Bot API (only when + /// is true). /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, which would cause the entire combined consent grant @@ -1587,7 +1615,8 @@ internal static string BuildCombinedConsentUrl( string? authorityHost = null, string? sharedMcpResourceAppId = null, string? observabilityResourceAppId = null, - bool includeObservability = true) + bool includeObservability = true, + bool includeDefenderDelegated = true) { var allScopes = new List(); foreach (var s in graphScopes) @@ -1623,7 +1652,8 @@ internal static string BuildCombinedConsentUrl( if (includeObservability) allScopes.Add( $"{ConfigConstants.BuildObservabilityApiIdentifierUri(observabilityResourceAppId ?? ConfigConstants.ObservabilityApiAppId)}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); - allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); + if (includeDefenderDelegated) + allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes, authorityHost); } @@ -1633,11 +1663,11 @@ internal static string BuildCombinedConsentUrl( /// when the running account is not a Global Administrator. Called by both DW and non-DW setup paths /// after the batch permissions step. /// - /// Messaging Bot API URLs are included only when is true, and - /// Observability API URLs only when the context requests Observability permissions; the other - /// resources (Graph, MCP, Power Platform) are always included so a tenant admin - /// can complete the hand-off with a single URL. When Observability permissions are skipped, a - /// consent URL saved for them by an earlier run is cleared on every run, admin runs included. + /// Messaging Bot API URLs are included only when is true. + /// Observability and delegated Defender URLs follow the selected setup permissions; Graph, + /// MCP, and Power Platform are always included so a tenant admin can complete the hand-off + /// with a single URL. When Observability permissions are skipped, a consent URL saved for them + /// by an earlier run is cleared on every run, admin runs included. /// Otherwise this is a no-op if admin consent was already granted or the blueprint ID is absent. /// /// @@ -1658,7 +1688,10 @@ internal static void ApplyConsentUrlsIfNeeded( return; var includeObservability = !ctx.SkipObservabilityPermissions; - var consentResourceNames = PopulateAdminConsentUrls(ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, includeObservability); + var includeDefenderDelegated = !ctx.Results.IsNonDwBlueprintFlow || !ctx.IsS2sMode; + var consentResourceNames = PopulateAdminConsentUrls( + ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, + mcpAudienceDisplayNames, includeObservability, includeDefenderDelegated); ctx.Results.ConsentUrlsSavedToPath = ctx.GeneratedConfigPath; ctx.Results.ConsentResourceNames.AddRange(consentResourceNames); var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(ctx.Config.Environment, ctx.Config.GraphBaseUrl); @@ -1668,7 +1701,7 @@ internal static void ApplyConsentUrlsIfNeeded( ctx.Results.CombinedConsentUrl = BuildCombinedConsentUrl( ctx.Config.TenantId!, ctx.Config.AgentBlueprintId!, graphScopes, mcpScopes, isM365, mcpScopesByAudience, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId, includeObservability); + mcpResourceAppId, observabilityResourceAppId, includeObservability, includeDefenderDelegated); } /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs index 088761c2..2e2375e0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -65,7 +65,7 @@ public class SetupResults /// /// Outcome of S2S app role assignments targeting the blueprint service principal. Written by /// in the DW path and in the non-DW path when the - /// blueprint carries app-role scopes (e.g. Observability API). + /// blueprint carries app-role scopes (for example Defender API in s2s or both mode). /// public GrantOutcome BlueprintS2SOutcome { get; set; } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index 432272d6..e644b615 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs @@ -358,11 +358,16 @@ public async Task ExecuteMessagingEndpointStepAsync_WhenOverrideProvidedAndConfi // Observability API permission wiring // ----------------------------------------------------------------------- - private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, List? customPermissions = null) + private SetupContext BuildPermissionsContext( + bool skipObservabilityPermissions, + List? customPermissions = null, + string? authMode = null, + bool isNonDwBlueprintFlow = true) { var executor = Substitute.For(Substitute.For>()); var graph = Substitute.For(); var blueprintService = Substitute.For(Substitute.For>(), graph); + var results = new SetupResults { IsNonDwBlueprintFlow = isNonDwBlueprintFlow }; // The blueprint has no inheritable permissions yet, so stale-permission cleanup has nothing to remove. blueprintService.ListInheritablePermissionsAsync( Arg.Any(), Arg.Any(), Arg.Any?>(), Arg.Any()) @@ -378,7 +383,7 @@ private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, DeploymentProjectPath = _tempDir, CustomBlueprintPermissions = customPermissions, }, - results: new SetupResults(), + results: results, logger: NullLogger.Instance, configFile: new FileInfo(Path.Combine(_tempDir, "a365.config.json")), generatedConfigPath: Path.Combine(_tempDir, "a365.generated.config.json"), @@ -398,6 +403,7 @@ private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, federatedCredentialService: Substitute.ForPartsOf( Substitute.For>(), graph), clientAppValidator: Substitute.For(), + authMode: authMode, skipObservabilityPermissions: skipObservabilityPermissions); } @@ -412,12 +418,63 @@ public async Task BuildPermissionSpecsAsync_StampsObservabilityApiUnlessSkipped( specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId).Should().Be(!skipObservabilityPermissions, because: "the spec list drives inheritable permissions, app role grants, and admin consent, so skipping Observability permissions must remove Observability API from it"); - specs.Any(s => s.AppRoleScopes is { Length: > 0 }).Should().Be(!skipObservabilityPermissions, - because: "OtelWrite is the only app role setup requests, so skipping it must leave no app role grant that needs a Global Administrator"); + specs.Should().Contain(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId, + because: "skipping Observability permissions must not remove the Defender permission required by the agent auth mode"); specs.Should().Contain(s => s.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, because: "skipping Observability API must not drop the other required resources"); } + [Theory] + [InlineData(null, true, false)] + [InlineData("obo", true, false)] + [InlineData("s2s", false, true)] + [InlineData("both", true, true)] + public async Task BuildPermissionSpecsAsync_NonDw_DefenderPermissionsMatchAuthMode( + string? authMode, + bool expectDelegated, + bool expectApplication) + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: true, + authMode: authMode, + isNonDwBlueprintFlow: true); + + var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); + + var defender = specs.Single(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId); + defender.Scopes.Should().BeEquivalentTo( + expectDelegated ? [ConfigConstants.DefenderApiRealtimeProtectionScope] : [], + because: $"the '{authMode ?? "obo"}' auth mode must request delegated Defender consent only when OBO is enabled"); + if (expectApplication) + { + defender.AppRoleScopes.Should().BeEquivalentTo( + [ConfigConstants.DefenderApiRealtimeProtectionScope], + because: $"the '{authMode ?? "obo"}' auth mode enables S2S Defender evaluation"); + } + else + { + defender.AppRoleScopes.Should().BeNull( + because: $"the '{authMode ?? "obo"}' auth mode must not request a Defender application role"); + } + } + + [Fact] + public async Task BuildPermissionSpecsAsync_Dw_PreservesBothDefenderPermissionTypes() + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: false, + authMode: "obo", + isNonDwBlueprintFlow: false); + + var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); + + var defender = specs.Single(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId); + defender.Scopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], + because: "DW setup does not use blueprint-agent auth modes and must preserve its delegated Defender permission"); + defender.AppRoleScopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], + because: "DW setup must preserve the existing Defender application permission"); + } + [Fact] public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRemainingResources() { @@ -426,14 +483,33 @@ public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRem SetupHelpers.ApplyConsentUrlsIfNeeded( ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, new[] { "McpServers.Mail.All" }, isM365: false); - ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Power Platform API" }, - because: "a non-admin run must hand every stamped resource to an administrator, and Observability API is no longer stamped"); + ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Defender API", "Power Platform API" }, + because: "default OBO setup must hand off every delegated resource, including Defender, while omitting Observability"); ctx.Config.ResourceConsents.Should().NotContain(rc => rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId, because: "no Observability API consent URL may be persisted when its permissions were skipped"); ctx.Results.CombinedConsentUrl.Should().NotContain(ConfigConstants.ObservabilityApiAppId, because: "the single hand-off URL must not request Observability API scopes that setup skipped"); } + [Fact] + public void ApplyConsentUrlsIfNeeded_S2s_OmitsDelegatedDefenderConsent() + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: true, + authMode: "s2s", + isNonDwBlueprintFlow: true); + + SetupHelpers.ApplyConsentUrlsIfNeeded( + ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, + new[] { "McpServers.Mail.All" }, isM365: false); + + ctx.Results.ConsentResourceNames.Should().NotContain("Defender API", + because: "S2S-only mode requests the Defender application role, not its delegated scope"); + ctx.Results.CombinedConsentUrl.Should().NotContain( + Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), + because: "the S2S hand-off URL must not request delegated Defender admin consent"); + } + [Fact] public void ApplyConsentUrlsIfNeeded_AdminRun_ClearsObservabilityConsentUrlSavedByAnEarlierRun() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs index fa985611..184ef64c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs @@ -312,12 +312,12 @@ private void ArrangeS2SPhase1AndAdminCheck() .Returns(Task.FromResult((ok: false, alreadyExists: false, error: (string?)"Insufficient privileges"))); } - private static ResourcePermissionSpec[] S2SSpec() => + private static ResourcePermissionSpec[] S2SSpec(bool includeDelegatedScope = true) => [ new ResourcePermissionSpec( ConfigConstants.ObservabilityApiAppId, "Observability API", - new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, + includeDelegatedScope ? new[] { ConfigConstants.ObservabilityApiOtelWriteScope } : [], SetInheritable: false, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope }) ]; @@ -501,8 +501,11 @@ private void ArrangeAzRestS2SCalls(bool blueprintAlreadyAssigned, int postExitCo /// DisplaySetupSummary surfaces the S2S hand-off block in the Action Required section — /// just like it does for a GA whose Graph API call returns 403. /// - [Fact] - public async Task ConfigureAllPermissions_NonAdmin_WithS2SSpecs_SetsBlueprintS2SOutcomeFailed() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ConfigureAllPermissions_NonAdmin_WithS2SSpecs_SetsBlueprintS2SOutcomeFailed( + bool includeDelegatedScope) { // Arrange _graph.GraphGetAsync( @@ -534,13 +537,13 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( _graph, _blueprintService, new Agent365Config { TenantId = S2STenantId, AgentBlueprintId = S2SBlueprintAppId }, blueprintAppId: S2SBlueprintAppId, tenantId: S2STenantId, - specs: S2SSpec(), _logger, setupResults, ct: default); + specs: S2SSpec(includeDelegatedScope), _logger, setupResults, ct: default); // Assert setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Failed, because: "a non-admin user cannot complete S2S app role assignment directly — the outcome must be marked Failed so DisplaySetupSummary surfaces the hand-off block"); setupResults.PendingBlueprintAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId, - because: "a non-admin run leaves every requested app role for the summary's hand-off"); + because: "a non-admin run leaves every requested app role for the summary's hand-off, including application-only specs"); } // ────────────────────────────────────────────────────────────────────────────────────── diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs index 2af4e503..2844adb3 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs @@ -731,14 +731,16 @@ public async Task SetupAll_BlueprintAgent_DefaultPlan_OmitsObservabilityApi() } /// - /// The S2S endpoint authorizes registered agents without OtelWrite whatever the auth mode (validated live), so - /// s2s and both — from the flag or from a365.config.json — must not request Observability API permissions either. + /// The S2S endpoint authorizes registered agents without OtelWrite, while Defender still requires + /// its application role for s2s and both modes. /// [Theory] [InlineData("--authmode s2s", null)] [InlineData("--authmode both", null)] [InlineData("", "both")] - public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityApi(string args, string? configAuthMode) + public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityAndKeepDefenderAppRole( + string args, + string? configAuthMode) { var config = new Agent365Config { @@ -771,7 +773,9 @@ public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityApi( _mockLogger.Received().Log( LogLevel.Information, Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && o.ToString()!.Contains("no S2S app roles to grant")), + Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && + o.ToString()!.Contains("S2S app roles") && + !o.ToString()!.Contains("no S2S app roles to grant")), Arg.Any(), Arg.Any>()); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs index 4835986f..57b2f85b 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs @@ -172,4 +172,24 @@ public void NonDwAdminConsentSpecs_PowerPlatformApi_IsDelegatedOnly() specs.Should().Contain(s => s.ResourceName == "Power Platform API" && s.PermissionType == "Delegated", because: "Power Platform API ConnectivityConnections.Read is a delegated scope"); } + + [Theory] + [InlineData("Delegated", true, false)] + [InlineData("Application", false, true)] + [InlineData("Both", true, true)] + public void GetNonDwAdminConsentSpecs_DefenderMatchesAuthMode( + string modeName, + bool expectDelegated, + bool expectApplication) + { + var mode = Enum.Parse(modeName); + var specs = SetupHelpers.GetNonDwAdminConsentSpecs("prod", mode); + + specs.Any(s => s.ResourceName == "Defender API" && s.PermissionType == "Delegated") + .Should().Be(expectDelegated, + because: $"{mode} mode must include the delegated Defender permission only when OBO is enabled"); + specs.Any(s => s.ResourceName == "Defender API" && s.PermissionType == "Application") + .Should().Be(expectApplication, + because: $"{mode} mode must include the Defender app role only when S2S is enabled"); + } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index 7afca5c1..b7d13c0e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs @@ -83,6 +83,17 @@ public void BuildAdminConsentUrls_DefenderApi_UsesAppIdIdentifierUri() because: "the Defender resource publishes its delegated permission under the api://{appId} audience"); } + [Fact] + public void BuildAdminConsentUrls_WhenDefenderDelegatedExcluded_OmitsDefenderApi() + { + var urls = SetupHelpers.BuildAdminConsentUrls( + TenantId, BlueprintClientId, new[] { "Mail.Send" }, new[] { "scope" }, + includeDefenderDelegated: false); + + urls.Should().NotContain(url => url.ResourceName == "Defender API", + because: "S2S-only blueprint agents must not request delegated Defender admin consent"); + } + [Fact] public void BuildAdminConsentUrls_PowerPlatformApi_UsesCorrectScopeConstant() { @@ -262,6 +273,21 @@ public void BuildCombinedConsentUrl_WithGccObservabilityResource_UsesGccAudience because: "a GCC consent URL must not request the commercial Observability audience"); } + [Fact] + public void BuildCombinedConsentUrl_WhenDefenderDelegatedExcluded_OmitsDefenderScope() + { + var url = SetupHelpers.BuildCombinedConsentUrl( + TenantId, + BlueprintClientId, + Array.Empty(), + Array.Empty(), + includeDefenderDelegated: false); + + url.Should().NotContain( + Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), + because: "S2S-only setup must not include a delegated Defender scope in the combined admin-consent URL"); + } + [Fact] public void BuildCombinedConsentUrl_ScopesJoinedWithEncodedSpaceNotAmpersand() { From d9e4e67ede5386cd7c52ce454651659a368d47e8 Mon Sep 17 00:00:00 2001 From: slreznit Date: Tue, 6 Oct 2026 00:20:02 +0300 Subject: [PATCH 06/14] Revert "Align Defender permissions with auth mode" This reverts commit 50cd7bd97a4adc566a87840ae4c72f1913d900bc. --- CHANGELOG.md | 1 - .../SetupSubcommands/AllSubcommand.cs | 9 +- .../BatchPermissionsOrchestrator.cs | 22 ++-- .../NonDwBlueprintSetupOrchestrator.cs | 26 ++-- .../ResourcePermissionSpec.cs | 7 -- .../Commands/SetupSubcommands/SetupHelpers.cs | 111 ++++++------------ .../Commands/SetupSubcommands/SetupResults.cs | 2 +- .../Commands/AllSubcommandTests.cs | 88 +------------- .../BatchPermissionsOrchestratorTests.cs | 15 +-- .../Commands/SetupCommandTests.cs | 12 +- ...tupHelpersAdminConsentInstructionsTests.cs | 20 ---- .../Helpers/SetupHelpersConsentUrlTests.cs | 26 ---- 12 files changed, 72 insertions(+), 267 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79d1d735..e7f497d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,7 +139,6 @@ Agents provisioned before this release need `RealtimeProtection.Evaluate.All` gr ### Changed -- Defender permissions now follow the blueprint agent authentication mode: delegated for `obo`, application for `s2s`, and both for `both`; AI Teammate setup continues to request both permission types (#485). - `a365 setup all` no longer requests Observability API permissions for blueprint agents; registered agents that export telemetry through the app-only S2S endpoint need no admin consent (#501). - Hardened token storage: the CLI no longer writes access tokens to a plaintext file — they live only in the OS-protected MSAL cache (DPAPI/Keychain/owner-only file). Any legacy plaintext cache is removed automatically; sign-in prompts are unchanged. - `develop-mcp register-external-mcp-server` now sets `exit code 1` on failure paths (validation errors, tenant detection failure, Graph unavailable, Entra app creation failure, MCP-Platform AddMcpServer failure). Previously these paths logged an error and exited `0`, which made the command's success/failure status undetectable from scripts and CI. Successful dry-run and user-initiated cancellation at the y/N prompt continue to exit `0`. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs index 65987112..49ace519 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -1020,14 +1020,7 @@ await PermissionsSubcommand.RemoveStaleCustomPermissionsAsync( // names so V2 audiences read as e.g. "mcp_MailTools" rather than "Agent 365 Tools". var specs = await SetupHelpers.BuildConfiguredPermissionSpecsAsync( ctx.Config, setInheritable: true, isM365: ctx.IsM365, scopesByAudience, serverNamesByAudience, - includeObservability: !ctx.SkipObservabilityPermissions, - defenderPermissionMode: ctx.Results.IsNonDwBlueprintFlow - ? ctx.IsS2sMode - ? DefenderPermissionMode.Application - : ctx.IsBothMode - ? DefenderPermissionMode.Both - : DefenderPermissionMode.Delegated - : DefenderPermissionMode.Both); + includeObservability: !ctx.SkipObservabilityPermissions); // Return the full scopesByAudience map alongside the V1-compat mcpScopes so V2 // callers (ApplyConsentUrlsIfNeeded) can route per-server audiences to the bare diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index 4b417648..c4a23c67 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -84,11 +84,9 @@ internal static class BatchPermissionsOrchestrator return (true, true, true, null); } - // Drop specs that carry neither delegated scopes nor application roles. Application-only - // specs must remain so S2S mode can assign app roles without creating an OAuth2 grant. - var effectiveSpecs = specs - .Where(s => s.Scopes.Length > 0 || s.AppRoleScopes is { Length: > 0 }) - .ToList(); + // Filter out specs with no scopes — they would produce empty OAuth2 grants (HTTP 400). + // This can happen when the MCP manifest is missing or contains no required scopes. + var effectiveSpecs = specs.Where(s => s.Scopes.Length > 0).ToList(); if (setupResults is not null) { setupResults.ObservabilityResourceAppId = effectiveSpecs @@ -99,12 +97,12 @@ internal static class BatchPermissionsOrchestrator if (effectiveSpecs.Count < specs.Count) { var skipped = specs.Count - effectiveSpecs.Count; - logger.LogDebug("Skipping {Count} resource spec(s) with no delegated scopes or application roles.", skipped); + logger.LogDebug("Skipping {Count} resource spec(s) with no scopes (manifest missing or empty).", skipped); } if (effectiveSpecs.Count == 0) { - logger.LogInformation("All permission specs have empty delegated scope and application role lists — skipping batch permissions configuration."); + logger.LogInformation("All permission specs have empty scope lists — skipping batch permissions configuration."); return (true, true, true, null); } @@ -417,16 +415,12 @@ private static async Task UpdateBlueprintPermissions continue; } - var permissionValues = spec.Scopes - .Concat(spec.AppRoleScopes ?? Array.Empty()) - .Distinct(StringComparer.OrdinalIgnoreCase) - .ToArray(); logger.LogDebug( - " - Configuring inheritable permissions: {ResourceName} [{Permissions}]", - spec.ResourceName, string.Join(' ', permissionValues)); + " - Configuring inheritable permissions: {ResourceName} [{Scopes}]", + spec.ResourceName, string.Join(' ', spec.Scopes)); var (ok, alreadyExists, err) = await blueprintService.SetInheritablePermissionsAsync( - tenantId, blueprintAppId, spec.ResourceAppId, permissionValues, + tenantId, blueprintAppId, spec.ResourceAppId, spec.Scopes, requiredScopes: permScopes, ct); if (alreadyExists || ok) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index e8d54ea3..0b8809f9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -36,16 +36,6 @@ internal static class NonDwBlueprintSetupOrchestrator public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool isBootstrap = false, string[]? rawArgs = null, bool skipRequirements = false, bool isM365 = false, bool agentRegistrationOnly = false, string? authMode = null, string? messagingEndpointOverride = null, bool skipObservabilityPermissions = false) { var sub = new string(' ', SetupHelpers.DryRunValCol); - var selectedAuthMode = authMode ?? config.AuthMode; - var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) - ? "obo" - : selectedAuthMode.Trim().ToLowerInvariant(); - var defenderPermissionMode = effectiveMode switch - { - "s2s" => DefenderPermissionMode.Application, - "both" => DefenderPermissionMode.Both, - _ => DefenderPermissionMode.Delegated, - }; var observabilityPermissionsEffectivelySkipped = skipObservabilityPermissions && !SetupHelpers.CustomPermissionsRequestObservability(config); // Dry-run S2S work comes only from fixed specs today; MCP and custom specs carry delegated scopes. @@ -53,8 +43,7 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i setInheritable: true, isM365, config.Environment, - includeObservability: !skipObservabilityPermissions, - defenderPermissionMode) + includeObservability: !skipObservabilityPermissions) .Any(s => s.AppRoleScopes is { Length: > 0 }); // --messaging-endpoint flag (if supplied) wins over the init-only config value for the plan. var plannedEndpoint = !string.IsNullOrWhiteSpace(messagingEndpointOverride) @@ -139,6 +128,10 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i // 3. Inheritable Permissions — the non-DW spec set is stamped on the blueprint so MAC and // dependent systems can see it. The same set is applied to the agent identity SP in step 5. + var selectedAuthMode = authMode ?? config.AuthMode; + var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) + ? "obo" + : selectedAuthMode.Trim().ToLowerInvariant(); logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for {Resources} (Global Administrator required; consent URL printed if absent)", skipObservabilityPermissions ? "Defender API, Power Platform API, and custom permissions" @@ -290,11 +283,6 @@ await ctx.ClientAppValidator.GrantConsentForPermissionsAsync( public static async Task ExecuteAsync(SetupContext ctx) { ctx.Results.IsNonDwBlueprintFlow = true; - ctx.Results.EffectiveAuthMode = ctx.IsBothMode - ? Models.AuthMode.Both - : ctx.IsS2sMode - ? Models.AuthMode.S2s - : Models.AuthMode.Obo; ctx.Results.ObservabilityPermissionsSkipped = ctx.ObservabilityPermissionsEffectivelySkipped; ctx.Results.TenantId = ctx.Config.TenantId; // Bootstrap already printed the "Running..." banner before auth steps; skip here to avoid duplication. @@ -735,8 +723,8 @@ internal static async Task GrantAgentIdentityS2SPermissionsAsync( List specs) { var hasS2sSpecs = specs.Any(s => s.AppRoleScopes is { Length: > 0 }); - // Record whether the selected auth mode produced any application permissions so the - // summary can distinguish "no S2S work" from a failed grant. + // Blueprint agents no longer request OtelWrite, the only app role setup requested, so this + // step usually has nothing to grant. Record that so the summary does not report a delegated grant. ctx.Results.NoS2SAppRolesToGrant = !hasS2sSpecs; if (hasS2sSpecs && AgentIdentityInheritsBlueprintAppRoles(ctx.Results)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs index 1c363c4c..5ca5d19e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs @@ -3,13 +3,6 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; -internal enum DefenderPermissionMode -{ - Delegated, - Application, - Both, -} - /// /// Describes a single resource whose permissions should be configured on the agent blueprint. /// Used as input to . diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 16076585..33eef967 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -59,8 +59,7 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( bool setInheritable, bool isM365, string? environment = null, - bool includeObservability = true, - DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) + bool includeObservability = true) { var specs = new List(); if (isM365) @@ -88,18 +87,12 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( setInheritable, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); } - var defenderDelegatedScopes = defenderPermissionMode is DefenderPermissionMode.Delegated or DefenderPermissionMode.Both - ? new[] { ConfigConstants.DefenderApiRealtimeProtectionScope } - : Array.Empty(); - var defenderAppRoleScopes = defenderPermissionMode is DefenderPermissionMode.Application or DefenderPermissionMode.Both - ? new[] { ConfigConstants.DefenderApiRealtimeProtectionScope } - : null; specs.Add(new ResourcePermissionSpec( ConfigConstants.DefenderApiAppId, "Defender API", - defenderDelegatedScopes, + new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, setInheritable, - AppRoleScopes: defenderAppRoleScopes)); + AppRoleScopes: new[] { ConfigConstants.DefenderApiRealtimeProtectionScope })); specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -154,8 +147,7 @@ internal static async Task> BuildConfiguredPermissi bool isM365 = true, Dictionary? scopesByAudience = null, Dictionary>? serverNamesByAudience = null, - bool includeObservability = true, - DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) + bool includeObservability = true) { // Manifest read at most once, and only when scopesByAudience is not pre-supplied. // Callers that already have the manifest loaded (e.g. AllSubcommand.BuildPermissionSpecsAsync) @@ -194,8 +186,7 @@ internal static async Task> BuildConfiguredPermissi : "Agent 365 Tools", kvp.Value, SetInheritable: setInheritable))); - specs.AddRange(GetFixedApiPermissionSpecs( - setInheritable, isM365, config.Environment, includeObservability, defenderPermissionMode)); + specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365, config.Environment, includeObservability)); foreach (var customPerm in config.CustomBlueprintPermissions ?? new List()) { @@ -484,8 +475,8 @@ internal static async Task ResolveBootstrapEnvironmentAsync( /// /// Fixed permission specs for the non-DW admin consent flow. - /// Observability API includes both permission types when requested. Defender follows the - /// selected auth mode. Power Platform API requires Delegated only. + /// Observability API and Defender API require both Application (app role for S2S) + /// and Delegated (oauth2 grant for OBO). Power Platform API requires Delegated only. /// Extend this list or pass an override to /// when additional APIs are required (e.g. dynamic MCP scopes, custom permissions). /// @@ -493,28 +484,20 @@ internal static async Task ResolveBootstrapEnvironmentAsync( GetNonDwAdminConsentSpecs("prod"); internal static IReadOnlyList<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> GetNonDwAdminConsentSpecs( - string? environment, - DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) - => BuildNonDwAdminConsentSpecs(ConfigConstants.GetObservabilityApiAppId(environment), defenderPermissionMode); + string? environment) + => BuildNonDwAdminConsentSpecs(ConfigConstants.GetObservabilityApiAppId(environment)); private static IReadOnlyList<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> BuildNonDwAdminConsentSpecs( - string observabilityAppId, - DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) + string observabilityAppId) { - var specs = new List<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> - { + return + [ ("Observability API", observabilityAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Application"), ("Observability API", observabilityAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Delegated"), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application"), + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated"), ("Power Platform API", PowerPlatformConstants.PowerPlatformApiResourceAppId, PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, "Delegated"), - }; - - if (defenderPermissionMode is DefenderPermissionMode.Application or DefenderPermissionMode.Both) - specs.Insert(2, ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application")); - if (defenderPermissionMode is DefenderPermissionMode.Delegated or DefenderPermissionMode.Both) - specs.Insert(defenderPermissionMode == DefenderPermissionMode.Both ? 3 : 2, - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated")); - - return specs; + ]; } /// @@ -692,8 +675,9 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str // (e.g. admin already granted tenant consent but the per-principal call still failed). var pendingDelegatedAction = agentIdDelegatedFailed && !pendingAdminAction; var pendingS2SAction = permissionGrantsPending && isS2SFlow; - // Some configurations can have no application-role specs. Say so explicitly; otherwise - // an s2s-only row can fall through to delegated wording with no matching action item. + // Blueprint agents no longer request OtelWrite, the only app role setup requested, so an + // s2s/both run usually has no S2S grant at all. Say so explicitly: otherwise the row falls + // through to the delegated wording, which for s2s-only shows a PENDING with no action item. var noS2SAppRolesToGrant = isNonDw && (isS2sOnlyMode || isBothMode) && results.NoS2SAppRolesToGrant && !isS2SFlow; if (results.PermissionGrantsSkipped && isNonDw) @@ -983,13 +967,7 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str if (isNonDw && string.IsNullOrWhiteSpace(consentUrl)) { logger.LogInformation(" {N}. Permission Grants — must be granted by {Roles} in the Entra portal:", actionCount, AuthenticationConstants.DelegatedGrantRequiredRoles); - var defenderPermissionMode = results.EffectiveAuthMode switch - { - Models.AuthMode.S2s => DefenderPermissionMode.Application, - Models.AuthMode.Both => DefenderPermissionMode.Both, - _ => DefenderPermissionMode.Delegated, - }; - var consentSpecs = BuildNonDwAdminConsentSpecs(observabilityResourceAppId, defenderPermissionMode); + var consentSpecs = BuildNonDwAdminConsentSpecs(observabilityResourceAppId); if (results.ObservabilityPermissionsSkipped) consentSpecs = consentSpecs.Where(s => !ConfigConstants.IsObservabilityApiAppId(s.ResourceAppId)).ToList(); LogNonDwAdminConsentInstructions( @@ -1281,11 +1259,10 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str /// resources. Called when the current user lacks the Global Administrator role so that the URLs /// can be saved to a365.generated.config.json and shared with a tenant administrator. /// - /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated. Defender is - /// generated when delegated Defender consent is enabled, and Observability is generated unless - /// is false. Messaging Bot API is included only when - /// is true — non-M365 tenants typically lack the Messaging Bot resource - /// SP and the consent endpoint returns AADSTS650053 otherwise. + /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated; the Observability + /// API URL is generated unless is false. Messaging Bot API is included only + /// when is true — non-M365 tenants typically lack the Messaging Bot + /// resource SP and the consent endpoint returns AADSTS650053 otherwise. /// /// /// Display names of the resources for which URLs were saved. @@ -1296,8 +1273,7 @@ internal static List PopulateAdminConsentUrls( bool isM365 = true, IReadOnlyDictionary? mcpScopesByAudience = null, IReadOnlyDictionary>? mcpAudienceDisplayNames = null, - bool includeObservability = true, - bool includeDefenderDelegated = true) + bool includeObservability = true) { var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(config.Environment, config.GraphBaseUrl); var graphResourceUri = graphBaseUrl; @@ -1307,7 +1283,7 @@ internal static List PopulateAdminConsentUrls( var urls = BuildAdminConsentUrls( config.TenantId, config.AgentBlueprintId!, config.AgentApplicationScopes, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId, includeObservability, includeDefenderDelegated); + mcpResourceAppId, observabilityResourceAppId, includeObservability); // Clear an Observability consent URL saved by an earlier run so the admin is not asked for permissions this run skipped. if (!includeObservability) @@ -1493,8 +1469,7 @@ internal static string BuildFullyQualifiedScope( /// (mirrors ): Microsoft Graph (when /// non-empty), Agent 365 Tools (when /// non-empty), Messaging Bot API (when is true), Observability API - /// (unless is false), Defender API (when delegated - /// Defender consent is enabled), and Power Platform API. + /// (unless is false), and Power Platform API. /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, in which case the /v2.0/adminconsent endpoint returns @@ -1514,8 +1489,7 @@ internal static string BuildFullyQualifiedScope( string? authorityHost = null, string? sharedMcpResourceAppId = null, string? observabilityResourceAppId = null, - bool includeObservability = true, - bool includeDefenderDelegated = true) + bool includeObservability = true) { var urls = new List<(string, string)>(); @@ -1584,8 +1558,7 @@ string Build(string tenant, string client, string resourceUri, IEnumerable /// Builds a single combined /v2.0/adminconsent URL covering every resource stamped on the /// blueprint: Graph, Agent 365 Tools (MCP), Observability API (unless - /// is false), Defender API when delegated Defender - /// consent is enabled, Power Platform API, and Messaging Bot API (only when - /// is true). + /// is false), Power Platform API, and + /// Messaging Bot API (only when is true). /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, which would cause the entire combined consent grant @@ -1615,8 +1587,7 @@ internal static string BuildCombinedConsentUrl( string? authorityHost = null, string? sharedMcpResourceAppId = null, string? observabilityResourceAppId = null, - bool includeObservability = true, - bool includeDefenderDelegated = true) + bool includeObservability = true) { var allScopes = new List(); foreach (var s in graphScopes) @@ -1652,8 +1623,7 @@ internal static string BuildCombinedConsentUrl( if (includeObservability) allScopes.Add( $"{ConfigConstants.BuildObservabilityApiIdentifierUri(observabilityResourceAppId ?? ConfigConstants.ObservabilityApiAppId)}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); - if (includeDefenderDelegated) - allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); + allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes, authorityHost); } @@ -1663,11 +1633,11 @@ internal static string BuildCombinedConsentUrl( /// when the running account is not a Global Administrator. Called by both DW and non-DW setup paths /// after the batch permissions step. /// - /// Messaging Bot API URLs are included only when is true. - /// Observability and delegated Defender URLs follow the selected setup permissions; Graph, - /// MCP, and Power Platform are always included so a tenant admin can complete the hand-off - /// with a single URL. When Observability permissions are skipped, a consent URL saved for them - /// by an earlier run is cleared on every run, admin runs included. + /// Messaging Bot API URLs are included only when is true, and + /// Observability API URLs only when the context requests Observability permissions; the other + /// resources (Graph, MCP, Power Platform) are always included so a tenant admin + /// can complete the hand-off with a single URL. When Observability permissions are skipped, a + /// consent URL saved for them by an earlier run is cleared on every run, admin runs included. /// Otherwise this is a no-op if admin consent was already granted or the blueprint ID is absent. /// /// @@ -1688,10 +1658,7 @@ internal static void ApplyConsentUrlsIfNeeded( return; var includeObservability = !ctx.SkipObservabilityPermissions; - var includeDefenderDelegated = !ctx.Results.IsNonDwBlueprintFlow || !ctx.IsS2sMode; - var consentResourceNames = PopulateAdminConsentUrls( - ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, - mcpAudienceDisplayNames, includeObservability, includeDefenderDelegated); + var consentResourceNames = PopulateAdminConsentUrls(ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, includeObservability); ctx.Results.ConsentUrlsSavedToPath = ctx.GeneratedConfigPath; ctx.Results.ConsentResourceNames.AddRange(consentResourceNames); var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(ctx.Config.Environment, ctx.Config.GraphBaseUrl); @@ -1701,7 +1668,7 @@ internal static void ApplyConsentUrlsIfNeeded( ctx.Results.CombinedConsentUrl = BuildCombinedConsentUrl( ctx.Config.TenantId!, ctx.Config.AgentBlueprintId!, graphScopes, mcpScopes, isM365, mcpScopesByAudience, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId, includeObservability, includeDefenderDelegated); + mcpResourceAppId, observabilityResourceAppId, includeObservability); } /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs index 2e2375e0..088761c2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -65,7 +65,7 @@ public class SetupResults /// /// Outcome of S2S app role assignments targeting the blueprint service principal. Written by /// in the DW path and in the non-DW path when the - /// blueprint carries app-role scopes (for example Defender API in s2s or both mode). + /// blueprint carries app-role scopes (e.g. Observability API). /// public GrantOutcome BlueprintS2SOutcome { get; set; } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index e644b615..432272d6 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs @@ -358,16 +358,11 @@ public async Task ExecuteMessagingEndpointStepAsync_WhenOverrideProvidedAndConfi // Observability API permission wiring // ----------------------------------------------------------------------- - private SetupContext BuildPermissionsContext( - bool skipObservabilityPermissions, - List? customPermissions = null, - string? authMode = null, - bool isNonDwBlueprintFlow = true) + private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, List? customPermissions = null) { var executor = Substitute.For(Substitute.For>()); var graph = Substitute.For(); var blueprintService = Substitute.For(Substitute.For>(), graph); - var results = new SetupResults { IsNonDwBlueprintFlow = isNonDwBlueprintFlow }; // The blueprint has no inheritable permissions yet, so stale-permission cleanup has nothing to remove. blueprintService.ListInheritablePermissionsAsync( Arg.Any(), Arg.Any(), Arg.Any?>(), Arg.Any()) @@ -383,7 +378,7 @@ private SetupContext BuildPermissionsContext( DeploymentProjectPath = _tempDir, CustomBlueprintPermissions = customPermissions, }, - results: results, + results: new SetupResults(), logger: NullLogger.Instance, configFile: new FileInfo(Path.Combine(_tempDir, "a365.config.json")), generatedConfigPath: Path.Combine(_tempDir, "a365.generated.config.json"), @@ -403,7 +398,6 @@ private SetupContext BuildPermissionsContext( federatedCredentialService: Substitute.ForPartsOf( Substitute.For>(), graph), clientAppValidator: Substitute.For(), - authMode: authMode, skipObservabilityPermissions: skipObservabilityPermissions); } @@ -418,63 +412,12 @@ public async Task BuildPermissionSpecsAsync_StampsObservabilityApiUnlessSkipped( specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId).Should().Be(!skipObservabilityPermissions, because: "the spec list drives inheritable permissions, app role grants, and admin consent, so skipping Observability permissions must remove Observability API from it"); - specs.Should().Contain(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId, - because: "skipping Observability permissions must not remove the Defender permission required by the agent auth mode"); + specs.Any(s => s.AppRoleScopes is { Length: > 0 }).Should().Be(!skipObservabilityPermissions, + because: "OtelWrite is the only app role setup requests, so skipping it must leave no app role grant that needs a Global Administrator"); specs.Should().Contain(s => s.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, because: "skipping Observability API must not drop the other required resources"); } - [Theory] - [InlineData(null, true, false)] - [InlineData("obo", true, false)] - [InlineData("s2s", false, true)] - [InlineData("both", true, true)] - public async Task BuildPermissionSpecsAsync_NonDw_DefenderPermissionsMatchAuthMode( - string? authMode, - bool expectDelegated, - bool expectApplication) - { - var ctx = BuildPermissionsContext( - skipObservabilityPermissions: true, - authMode: authMode, - isNonDwBlueprintFlow: true); - - var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); - - var defender = specs.Single(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId); - defender.Scopes.Should().BeEquivalentTo( - expectDelegated ? [ConfigConstants.DefenderApiRealtimeProtectionScope] : [], - because: $"the '{authMode ?? "obo"}' auth mode must request delegated Defender consent only when OBO is enabled"); - if (expectApplication) - { - defender.AppRoleScopes.Should().BeEquivalentTo( - [ConfigConstants.DefenderApiRealtimeProtectionScope], - because: $"the '{authMode ?? "obo"}' auth mode enables S2S Defender evaluation"); - } - else - { - defender.AppRoleScopes.Should().BeNull( - because: $"the '{authMode ?? "obo"}' auth mode must not request a Defender application role"); - } - } - - [Fact] - public async Task BuildPermissionSpecsAsync_Dw_PreservesBothDefenderPermissionTypes() - { - var ctx = BuildPermissionsContext( - skipObservabilityPermissions: false, - authMode: "obo", - isNonDwBlueprintFlow: false); - - var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); - - var defender = specs.Single(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId); - defender.Scopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], - because: "DW setup does not use blueprint-agent auth modes and must preserve its delegated Defender permission"); - defender.AppRoleScopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], - because: "DW setup must preserve the existing Defender application permission"); - } - [Fact] public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRemainingResources() { @@ -483,33 +426,14 @@ public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRem SetupHelpers.ApplyConsentUrlsIfNeeded( ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, new[] { "McpServers.Mail.All" }, isM365: false); - ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Defender API", "Power Platform API" }, - because: "default OBO setup must hand off every delegated resource, including Defender, while omitting Observability"); + ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Power Platform API" }, + because: "a non-admin run must hand every stamped resource to an administrator, and Observability API is no longer stamped"); ctx.Config.ResourceConsents.Should().NotContain(rc => rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId, because: "no Observability API consent URL may be persisted when its permissions were skipped"); ctx.Results.CombinedConsentUrl.Should().NotContain(ConfigConstants.ObservabilityApiAppId, because: "the single hand-off URL must not request Observability API scopes that setup skipped"); } - [Fact] - public void ApplyConsentUrlsIfNeeded_S2s_OmitsDelegatedDefenderConsent() - { - var ctx = BuildPermissionsContext( - skipObservabilityPermissions: true, - authMode: "s2s", - isNonDwBlueprintFlow: true); - - SetupHelpers.ApplyConsentUrlsIfNeeded( - ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, - new[] { "McpServers.Mail.All" }, isM365: false); - - ctx.Results.ConsentResourceNames.Should().NotContain("Defender API", - because: "S2S-only mode requests the Defender application role, not its delegated scope"); - ctx.Results.CombinedConsentUrl.Should().NotContain( - Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), - because: "the S2S hand-off URL must not request delegated Defender admin consent"); - } - [Fact] public void ApplyConsentUrlsIfNeeded_AdminRun_ClearsObservabilityConsentUrlSavedByAnEarlierRun() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs index 184ef64c..fa985611 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs @@ -312,12 +312,12 @@ private void ArrangeS2SPhase1AndAdminCheck() .Returns(Task.FromResult((ok: false, alreadyExists: false, error: (string?)"Insufficient privileges"))); } - private static ResourcePermissionSpec[] S2SSpec(bool includeDelegatedScope = true) => + private static ResourcePermissionSpec[] S2SSpec() => [ new ResourcePermissionSpec( ConfigConstants.ObservabilityApiAppId, "Observability API", - includeDelegatedScope ? new[] { ConfigConstants.ObservabilityApiOtelWriteScope } : [], + new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, SetInheritable: false, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope }) ]; @@ -501,11 +501,8 @@ private void ArrangeAzRestS2SCalls(bool blueprintAlreadyAssigned, int postExitCo /// DisplaySetupSummary surfaces the S2S hand-off block in the Action Required section — /// just like it does for a GA whose Graph API call returns 403. /// - [Theory] - [InlineData(true)] - [InlineData(false)] - public async Task ConfigureAllPermissions_NonAdmin_WithS2SSpecs_SetsBlueprintS2SOutcomeFailed( - bool includeDelegatedScope) + [Fact] + public async Task ConfigureAllPermissions_NonAdmin_WithS2SSpecs_SetsBlueprintS2SOutcomeFailed() { // Arrange _graph.GraphGetAsync( @@ -537,13 +534,13 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( _graph, _blueprintService, new Agent365Config { TenantId = S2STenantId, AgentBlueprintId = S2SBlueprintAppId }, blueprintAppId: S2SBlueprintAppId, tenantId: S2STenantId, - specs: S2SSpec(includeDelegatedScope), _logger, setupResults, ct: default); + specs: S2SSpec(), _logger, setupResults, ct: default); // Assert setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Failed, because: "a non-admin user cannot complete S2S app role assignment directly — the outcome must be marked Failed so DisplaySetupSummary surfaces the hand-off block"); setupResults.PendingBlueprintAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId, - because: "a non-admin run leaves every requested app role for the summary's hand-off, including application-only specs"); + because: "a non-admin run leaves every requested app role for the summary's hand-off"); } // ────────────────────────────────────────────────────────────────────────────────────── diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs index 2844adb3..2af4e503 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs @@ -731,16 +731,14 @@ public async Task SetupAll_BlueprintAgent_DefaultPlan_OmitsObservabilityApi() } /// - /// The S2S endpoint authorizes registered agents without OtelWrite, while Defender still requires - /// its application role for s2s and both modes. + /// The S2S endpoint authorizes registered agents without OtelWrite whatever the auth mode (validated live), so + /// s2s and both — from the flag or from a365.config.json — must not request Observability API permissions either. /// [Theory] [InlineData("--authmode s2s", null)] [InlineData("--authmode both", null)] [InlineData("", "both")] - public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityAndKeepDefenderAppRole( - string args, - string? configAuthMode) + public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityApi(string args, string? configAuthMode) { var config = new Agent365Config { @@ -773,9 +771,7 @@ public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityAndK _mockLogger.Received().Log( LogLevel.Information, Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && - o.ToString()!.Contains("S2S app roles") && - !o.ToString()!.Contains("no S2S app roles to grant")), + Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && o.ToString()!.Contains("no S2S app roles to grant")), Arg.Any(), Arg.Any>()); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs index 57b2f85b..4835986f 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs @@ -172,24 +172,4 @@ public void NonDwAdminConsentSpecs_PowerPlatformApi_IsDelegatedOnly() specs.Should().Contain(s => s.ResourceName == "Power Platform API" && s.PermissionType == "Delegated", because: "Power Platform API ConnectivityConnections.Read is a delegated scope"); } - - [Theory] - [InlineData("Delegated", true, false)] - [InlineData("Application", false, true)] - [InlineData("Both", true, true)] - public void GetNonDwAdminConsentSpecs_DefenderMatchesAuthMode( - string modeName, - bool expectDelegated, - bool expectApplication) - { - var mode = Enum.Parse(modeName); - var specs = SetupHelpers.GetNonDwAdminConsentSpecs("prod", mode); - - specs.Any(s => s.ResourceName == "Defender API" && s.PermissionType == "Delegated") - .Should().Be(expectDelegated, - because: $"{mode} mode must include the delegated Defender permission only when OBO is enabled"); - specs.Any(s => s.ResourceName == "Defender API" && s.PermissionType == "Application") - .Should().Be(expectApplication, - because: $"{mode} mode must include the Defender app role only when S2S is enabled"); - } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index b7d13c0e..7afca5c1 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs @@ -83,17 +83,6 @@ public void BuildAdminConsentUrls_DefenderApi_UsesAppIdIdentifierUri() because: "the Defender resource publishes its delegated permission under the api://{appId} audience"); } - [Fact] - public void BuildAdminConsentUrls_WhenDefenderDelegatedExcluded_OmitsDefenderApi() - { - var urls = SetupHelpers.BuildAdminConsentUrls( - TenantId, BlueprintClientId, new[] { "Mail.Send" }, new[] { "scope" }, - includeDefenderDelegated: false); - - urls.Should().NotContain(url => url.ResourceName == "Defender API", - because: "S2S-only blueprint agents must not request delegated Defender admin consent"); - } - [Fact] public void BuildAdminConsentUrls_PowerPlatformApi_UsesCorrectScopeConstant() { @@ -273,21 +262,6 @@ public void BuildCombinedConsentUrl_WithGccObservabilityResource_UsesGccAudience because: "a GCC consent URL must not request the commercial Observability audience"); } - [Fact] - public void BuildCombinedConsentUrl_WhenDefenderDelegatedExcluded_OmitsDefenderScope() - { - var url = SetupHelpers.BuildCombinedConsentUrl( - TenantId, - BlueprintClientId, - Array.Empty(), - Array.Empty(), - includeDefenderDelegated: false); - - url.Should().NotContain( - Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), - because: "S2S-only setup must not include a delegated Defender scope in the combined admin-consent URL"); - } - [Fact] public void BuildCombinedConsentUrl_ScopesJoinedWithEncodedSpaceNotAmpersand() { From 5b62a86fc536cc3e10f87b710c1e6b8cc51a3845 Mon Sep 17 00:00:00 2001 From: slreznit Date: Tue, 6 Oct 2026 00:24:16 +0300 Subject: [PATCH 07/14] Update Defender permission expectations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Commands/AllSubcommandTests.cs | 13 +++++++++---- .../Commands/SetupCommandTests.cs | 12 ++++++++---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index 432272d6..c7450b9c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs @@ -412,8 +412,13 @@ public async Task BuildPermissionSpecsAsync_StampsObservabilityApiUnlessSkipped( specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId).Should().Be(!skipObservabilityPermissions, because: "the spec list drives inheritable permissions, app role grants, and admin consent, so skipping Observability permissions must remove Observability API from it"); - specs.Any(s => s.AppRoleScopes is { Length: > 0 }).Should().Be(!skipObservabilityPermissions, - because: "OtelWrite is the only app role setup requests, so skipping it must leave no app role grant that needs a Global Administrator"); + specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId && + s.AppRoleScopes is { Length: > 0 }).Should().Be(!skipObservabilityPermissions, + because: "skipping Observability permissions must remove the OtelWrite app role without affecting application roles required by other resources"); + specs.Should().Contain(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId && + s.AppRoleScopes != null && + s.AppRoleScopes.Contains(ConfigConstants.DefenderApiRealtimeProtectionScope), + because: "Defender independently requires RealtimeProtection.Evaluate.All as an application permission"); specs.Should().Contain(s => s.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, because: "skipping Observability API must not drop the other required resources"); } @@ -426,8 +431,8 @@ public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRem SetupHelpers.ApplyConsentUrlsIfNeeded( ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, new[] { "McpServers.Mail.All" }, isM365: false); - ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Power Platform API" }, - because: "a non-admin run must hand every stamped resource to an administrator, and Observability API is no longer stamped"); + ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Defender API", "Power Platform API" }, + because: "a non-admin run must hand every stamped resource to an administrator, including Defender, while Observability is no longer stamped"); ctx.Config.ResourceConsents.Should().NotContain(rc => rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId, because: "no Observability API consent URL may be persisted when its permissions were skipped"); ctx.Results.CombinedConsentUrl.Should().NotContain(ConfigConstants.ObservabilityApiAppId, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs index 2af4e503..6c95747d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs @@ -731,14 +731,16 @@ public async Task SetupAll_BlueprintAgent_DefaultPlan_OmitsObservabilityApi() } /// - /// The S2S endpoint authorizes registered agents without OtelWrite whatever the auth mode (validated live), so - /// s2s and both — from the flag or from a365.config.json — must not request Observability API permissions either. + /// The S2S endpoint authorizes registered agents without OtelWrite, while Defender still + /// contributes an application role for s2s and both modes. /// [Theory] [InlineData("--authmode s2s", null)] [InlineData("--authmode both", null)] [InlineData("", "both")] - public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityApi(string args, string? configAuthMode) + public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityAndKeepDefenderAppRole( + string args, + string? configAuthMode) { var config = new Agent365Config { @@ -771,7 +773,9 @@ public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityApi( _mockLogger.Received().Log( LogLevel.Information, Arg.Any(), - Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && o.ToString()!.Contains("no S2S app roles to grant")), + Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && + o.ToString()!.Contains("S2S app roles") && + !o.ToString()!.Contains("no S2S app roles to grant")), Arg.Any(), Arg.Any>()); } From a2ea7d9142b8698cd78e1a42db880ce0aa4aefdc Mon Sep 17 00:00:00 2001 From: slreznit Date: Wed, 7 Oct 2026 19:17:32 +0300 Subject: [PATCH 08/14] Fix OBO dry-run permission summary Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- .../NonDwBlueprintSetupOrchestrator.cs | 8 ++++--- ...DwBlueprintSetupOrchestratorDryRunTests.cs | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index 0b8809f9..f23634af 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -139,12 +139,14 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i if (observabilityPermissionsEffectivelySkipped) logger.LogInformation(sub + "Observability API not requested (registered agents export telemetry with an app-only token)"); - // 4. Blueprint Permission Grants — per authMode. The consent URL targets the blueprint - // app, and S2S app-role assignments are persisted as grants flowing from the blueprint; + // 4. Blueprint Permission Grants. authMode controls delegated grants, while fixed S2S + // app-role assignments are persisted on the blueprint whenever the specs require them; // grouping here keeps all blueprint-side rows (2 Blueprint, 3 Inheritable Permissions, // 4 Blueprint Permission Grants) contiguous. if (effectiveMode is "obo") - logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + "delegated grants — attempted programmatically for the signed-in principal (403 may indicate additional delegated consent or permissions are required)"); + logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + (!fixedSpecsHaveAppRoles + ? "delegated grants — attempted programmatically for the signed-in principal (403 may indicate additional delegated consent or permissions are required)" + : $"delegated grants for the signed-in principal + S2S app roles — attempted programmatically; {AuthenticationConstants.S2SGrantRequiredRoles} required for S2S if 403")); else if (effectiveMode is "s2s") logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + (!fixedSpecsHaveAppRoles ? "not required (no S2S app roles to grant)" diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs index 56020e61..8c21e63e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs @@ -157,6 +157,27 @@ public void PrintDryRunPlan_AuthModeObo_ShowsDelegatedGrantsOnAgentIdentity() AnyLogContains("delegated").Should().BeTrue(because: "OBO mode applies principal-scoped delegated grants to the agent identity SP"); } + /// + /// OBO mode still grants fixed application roles on the blueprint independently of the + /// principal-scoped delegated grants applied to the agent identity. + /// + [Theory] + [InlineData("obo")] + [InlineData(null)] + public void PrintDryRunPlan_AuthModeObo_ShowsDefenderApplicationRoleGrantOnBlueprint(string? authMode) + { + NonDwBlueprintSetupOrchestrator.PrintDryRunPlan( + BuildConfig(), + _logger, + authMode: authMode, + skipObservabilityPermissions: true); + + AnyLogContains("delegated grants for the signed-in principal + S2S app roles").Should().BeTrue( + because: "the blueprint receives the Defender application role in OBO mode even though the agent identity uses delegated grants"); + AnyLogContains("Global Administrator required for S2S if 403").Should().BeTrue( + because: "a non-admin dry run must surface the administrative handoff required for the Defender application role"); + } + /// /// S2S mode must show application permissions on the agent identity SP and must not show /// delegated grants — there is no user context in S2S so delegated scopes don't apply. From bf02b99ceda068e2828fe1068bc8ac50cd375101 Mon Sep 17 00:00:00 2001 From: slreznit Date: Wed, 7 Oct 2026 19:36:11 +0300 Subject: [PATCH 09/14] Align Defender grants with auth mode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- CHANGELOG.md | 5 +- .../SetupSubcommands/AllSubcommand.cs | 9 +- .../BatchPermissionsOrchestrator.cs | 22 +-- .../NonDwBlueprintSetupOrchestrator.cs | 41 ++++-- .../ResourcePermissionSpec.cs | 7 + .../Commands/SetupSubcommands/SetupHelpers.cs | 136 +++++++++++------- .../Commands/SetupSubcommands/SetupResults.cs | 4 +- .../Services/LogRedactionService.cs | 2 +- .../Commands/AllSubcommandTests.cs | 114 +++++++++++++-- .../BatchPermissionsOrchestratorTests.cs | 15 +- ...DwBlueprintSetupOrchestratorDryRunTests.cs | 21 ++- ...tupHelpersAdminConsentInstructionsTests.cs | 20 +++ .../SetupHelpersDisplaySetupSummaryTests.cs | 63 ++++++++ .../Services/LogRedactionServiceTests.cs | 11 ++ 14 files changed, 368 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f497d4..dd59fabb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,10 +30,11 @@ Blueprint agents that export telemetry through the app-only S2S endpoint don't n #### Existing agents: grant Defender API permissions -Agents provisioned before this release need `RealtimeProtection.Evaluate.All` granted as both a **delegated** and an **application** permission on the blueprint app for the Defender security integration. Requires Global Administrator. Follow the steps above, searching for `86a21212-634e-4553-b3d6-e477e4c9d9ec` in step 2 and selecting `RealtimeProtection.Evaluate.All` in steps 3 and 4. Re-running `a365 setup all` grants it automatically. +Agents provisioned before this release need `RealtimeProtection.Evaluate.All` on the blueprint app for the Defender security integration: **delegated** for `--authmode obo`, **application** for `--authmode s2s`, or both for `--authmode both`. Re-run `a365 setup all --authmode ` as a Global Administrator; this stamps the Defender inheritable-permission entry and grants the permission required by that auth mode. Adding only the API permission in the Entra portal is insufficient for older blueprints that do not yet have the Defender inheritable-permission entry. ### Added -- `RealtimeProtection.Evaluate.All` on the Defender API is now granted automatically during `a365 setup` as both a delegated and an application permission, enabling the Microsoft Defender security integration without manual Entra steps. + +- `a365 setup all` now grants the Defender API `RealtimeProtection.Evaluate.All` permission according to `--authmode`: delegated for `obo`, application for `s2s`, and both for `both`. - `a365 develop-mcp grant-agents-access --agent-blueprint-id --mcp-server-name ` reports which agent instances of a blueprint are missing the permission to call a BYO MCP server, and prompts you to select which ones to grant it to (#500). - When more than one Entra application shares the MCP server's name, `a365 develop-mcp grant-agents-access` now lists them all and asks which one to use instead of failing (#500). - `a365 develop-mcp grant-agents-access --help` now lists Microsoft's first-party agent blueprint names and IDs, and the same list is printed when `--agent-blueprint-id` is missing or not a GUID, so you can find the ID without looking it up elsewhere (#500). diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs index 49ace519..65987112 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -1020,7 +1020,14 @@ await PermissionsSubcommand.RemoveStaleCustomPermissionsAsync( // names so V2 audiences read as e.g. "mcp_MailTools" rather than "Agent 365 Tools". var specs = await SetupHelpers.BuildConfiguredPermissionSpecsAsync( ctx.Config, setInheritable: true, isM365: ctx.IsM365, scopesByAudience, serverNamesByAudience, - includeObservability: !ctx.SkipObservabilityPermissions); + includeObservability: !ctx.SkipObservabilityPermissions, + defenderPermissionMode: ctx.Results.IsNonDwBlueprintFlow + ? ctx.IsS2sMode + ? DefenderPermissionMode.Application + : ctx.IsBothMode + ? DefenderPermissionMode.Both + : DefenderPermissionMode.Delegated + : DefenderPermissionMode.Both); // Return the full scopesByAudience map alongside the V1-compat mcpScopes so V2 // callers (ApplyConsentUrlsIfNeeded) can route per-server audiences to the bare diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index c4a23c67..cc2e0ee4 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -84,9 +84,11 @@ internal static class BatchPermissionsOrchestrator return (true, true, true, null); } - // Filter out specs with no scopes — they would produce empty OAuth2 grants (HTTP 400). - // This can happen when the MCP manifest is missing or contains no required scopes. - var effectiveSpecs = specs.Where(s => s.Scopes.Length > 0).ToList(); + // Application-only specs must remain so S2S mode can assign app roles without creating + // an empty OAuth2 grant. + var effectiveSpecs = specs + .Where(s => s.Scopes.Length > 0 || s.AppRoleScopes is { Length: > 0 }) + .ToList(); if (setupResults is not null) { setupResults.ObservabilityResourceAppId = effectiveSpecs @@ -97,12 +99,12 @@ internal static class BatchPermissionsOrchestrator if (effectiveSpecs.Count < specs.Count) { var skipped = specs.Count - effectiveSpecs.Count; - logger.LogDebug("Skipping {Count} resource spec(s) with no scopes (manifest missing or empty).", skipped); + logger.LogDebug("Skipping {Count} resource spec(s) with no delegated scopes or application roles.", skipped); } if (effectiveSpecs.Count == 0) { - logger.LogInformation("All permission specs have empty scope lists — skipping batch permissions configuration."); + logger.LogInformation("All permission specs have empty delegated scope and application role lists — skipping batch permissions configuration."); return (true, true, true, null); } @@ -415,12 +417,16 @@ private static async Task UpdateBlueprintPermissions continue; } + var permissionValues = spec.Scopes + .Concat(spec.AppRoleScopes ?? Array.Empty()) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); logger.LogDebug( - " - Configuring inheritable permissions: {ResourceName} [{Scopes}]", - spec.ResourceName, string.Join(' ', spec.Scopes)); + " - Configuring inheritable permissions: {ResourceName} [{Permissions}]", + spec.ResourceName, string.Join(' ', permissionValues)); var (ok, alreadyExists, err) = await blueprintService.SetInheritablePermissionsAsync( - tenantId, blueprintAppId, spec.ResourceAppId, spec.Scopes, + tenantId, blueprintAppId, spec.ResourceAppId, permissionValues, requiredScopes: permScopes, ct); if (alreadyExists || ok) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index f23634af..0235ccb6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -36,6 +36,16 @@ internal static class NonDwBlueprintSetupOrchestrator public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool isBootstrap = false, string[]? rawArgs = null, bool skipRequirements = false, bool isM365 = false, bool agentRegistrationOnly = false, string? authMode = null, string? messagingEndpointOverride = null, bool skipObservabilityPermissions = false) { var sub = new string(' ', SetupHelpers.DryRunValCol); + var selectedAuthMode = authMode ?? config.AuthMode; + var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) + ? "obo" + : selectedAuthMode.Trim().ToLowerInvariant(); + var defenderPermissionMode = effectiveMode switch + { + "s2s" => DefenderPermissionMode.Application, + "both" => DefenderPermissionMode.Both, + _ => DefenderPermissionMode.Delegated, + }; var observabilityPermissionsEffectivelySkipped = skipObservabilityPermissions && !SetupHelpers.CustomPermissionsRequestObservability(config); // Dry-run S2S work comes only from fixed specs today; MCP and custom specs carry delegated scopes. @@ -43,7 +53,8 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i setInheritable: true, isM365, config.Environment, - includeObservability: !skipObservabilityPermissions) + includeObservability: !skipObservabilityPermissions, + defenderPermissionMode) .Any(s => s.AppRoleScopes is { Length: > 0 }); // --messaging-endpoint flag (if supplied) wins over the init-only config value for the plan. var plannedEndpoint = !string.IsNullOrWhiteSpace(messagingEndpointOverride) @@ -128,10 +139,6 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i // 3. Inheritable Permissions — the non-DW spec set is stamped on the blueprint so MAC and // dependent systems can see it. The same set is applied to the agent identity SP in step 5. - var selectedAuthMode = authMode ?? config.AuthMode; - var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) - ? "obo" - : selectedAuthMode.Trim().ToLowerInvariant(); logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for {Resources} (Global Administrator required; consent URL printed if absent)", skipObservabilityPermissions ? "Defender API, Power Platform API, and custom permissions" @@ -139,14 +146,12 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i if (observabilityPermissionsEffectivelySkipped) logger.LogInformation(sub + "Observability API not requested (registered agents export telemetry with an app-only token)"); - // 4. Blueprint Permission Grants. authMode controls delegated grants, while fixed S2S - // app-role assignments are persisted on the blueprint whenever the specs require them; + // 4. Blueprint Permission Grants — Defender follows authMode; other permission specs retain + // their configured grant types. // grouping here keeps all blueprint-side rows (2 Blueprint, 3 Inheritable Permissions, // 4 Blueprint Permission Grants) contiguous. if (effectiveMode is "obo") - logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + (!fixedSpecsHaveAppRoles - ? "delegated grants — attempted programmatically for the signed-in principal (403 may indicate additional delegated consent or permissions are required)" - : $"delegated grants for the signed-in principal + S2S app roles — attempted programmatically; {AuthenticationConstants.S2SGrantRequiredRoles} required for S2S if 403")); + logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + "delegated grants — attempted programmatically for the signed-in principal (403 may indicate additional delegated consent or permissions are required)"); else if (effectiveMode is "s2s") logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + (!fixedSpecsHaveAppRoles ? "not required (no S2S app roles to grant)" @@ -285,6 +290,11 @@ await ctx.ClientAppValidator.GrantConsentForPermissionsAsync( public static async Task ExecuteAsync(SetupContext ctx) { ctx.Results.IsNonDwBlueprintFlow = true; + ctx.Results.EffectiveAuthMode = ctx.IsBothMode + ? Models.AuthMode.Both + : ctx.IsS2sMode + ? Models.AuthMode.S2s + : Models.AuthMode.Obo; ctx.Results.ObservabilityPermissionsSkipped = ctx.ObservabilityPermissionsEffectivelySkipped; ctx.Results.TenantId = ctx.Config.TenantId; // Bootstrap already printed the "Running..." banner before auth steps; skip here to avoid duplication. @@ -468,9 +478,14 @@ internal static async Task ExecuteAgentIdentityAndRegistrationAsync( // Skipped when --agent-registration-only: identity result flags are pre-set by the caller. if (!skipIdentityAndPermissions) { - // Record the auth mode and whether any S2S app role is requested before identity creation, - // so the summary stays accurate when the identity step fails. - ctx.Results.EffectiveAuthMode = ctx.IsBothMode ? Models.AuthMode.Both : ctx.IsS2sMode ? Models.AuthMode.S2s : Models.AuthMode.Obo; + // Keep direct callers of this phase aligned with the top-level orchestrator. + ctx.Results.EffectiveAuthMode = ctx.IsBothMode + ? Models.AuthMode.Both + : ctx.IsS2sMode + ? Models.AuthMode.S2s + : Models.AuthMode.Obo; + // Record whether the selected auth mode produced application permissions before + // identity creation so the summary stays accurate when that step fails. if (ctx.IsS2sMode || ctx.IsBothMode) ctx.Results.NoS2SAppRolesToGrant = !specs.Any(s => s.AppRoleScopes is { Length: > 0 }); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs index 5ca5d19e..1c363c4c 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs @@ -3,6 +3,13 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; +internal enum DefenderPermissionMode +{ + Delegated, + Application, + Both, +} + /// /// Describes a single resource whose permissions should be configured on the agent blueprint. /// Used as input to . diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 33eef967..c37530b8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -59,7 +59,8 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( bool setInheritable, bool isM365, string? environment = null, - bool includeObservability = true) + bool includeObservability = true, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { var specs = new List(); if (isM365) @@ -87,12 +88,18 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( setInheritable, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); } + var defenderDelegatedScopes = defenderPermissionMode is DefenderPermissionMode.Delegated or DefenderPermissionMode.Both + ? new[] { ConfigConstants.DefenderApiRealtimeProtectionScope } + : Array.Empty(); + var defenderAppRoleScopes = defenderPermissionMode is DefenderPermissionMode.Application or DefenderPermissionMode.Both + ? new[] { ConfigConstants.DefenderApiRealtimeProtectionScope } + : null; specs.Add(new ResourcePermissionSpec( ConfigConstants.DefenderApiAppId, "Defender API", - new[] { ConfigConstants.DefenderApiRealtimeProtectionScope }, + defenderDelegatedScopes, setInheritable, - AppRoleScopes: new[] { ConfigConstants.DefenderApiRealtimeProtectionScope })); + AppRoleScopes: defenderAppRoleScopes)); specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -147,7 +154,8 @@ internal static async Task> BuildConfiguredPermissi bool isM365 = true, Dictionary? scopesByAudience = null, Dictionary>? serverNamesByAudience = null, - bool includeObservability = true) + bool includeObservability = true, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { // Manifest read at most once, and only when scopesByAudience is not pre-supplied. // Callers that already have the manifest loaded (e.g. AllSubcommand.BuildPermissionSpecsAsync) @@ -186,7 +194,8 @@ internal static async Task> BuildConfiguredPermissi : "Agent 365 Tools", kvp.Value, SetInheritable: setInheritable))); - specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365, config.Environment, includeObservability)); + specs.AddRange(GetFixedApiPermissionSpecs( + setInheritable, isM365, config.Environment, includeObservability, defenderPermissionMode)); foreach (var customPerm in config.CustomBlueprintPermissions ?? new List()) { @@ -475,8 +484,8 @@ internal static async Task ResolveBootstrapEnvironmentAsync( /// /// Fixed permission specs for the non-DW admin consent flow. - /// Observability API and Defender API require both Application (app role for S2S) - /// and Delegated (oauth2 grant for OBO). Power Platform API requires Delegated only. + /// Observability API includes both permission types when requested. Defender follows the + /// selected auth mode. Power Platform API requires Delegated only. /// Extend this list or pass an override to /// when additional APIs are required (e.g. dynamic MCP scopes, custom permissions). /// @@ -484,31 +493,31 @@ internal static async Task ResolveBootstrapEnvironmentAsync( GetNonDwAdminConsentSpecs("prod"); internal static IReadOnlyList<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> GetNonDwAdminConsentSpecs( - string? environment) - => BuildNonDwAdminConsentSpecs(ConfigConstants.GetObservabilityApiAppId(environment)); + string? environment, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) + => BuildNonDwAdminConsentSpecs( + ConfigConstants.GetObservabilityApiAppId(environment), + defenderPermissionMode); private static IReadOnlyList<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> BuildNonDwAdminConsentSpecs( - string observabilityAppId) + string observabilityAppId, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { - return - [ + var specs = new List<(string ResourceName, string ResourceAppId, string Scope, string PermissionType)> + { ("Observability API", observabilityAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Application"), ("Observability API", observabilityAppId, ConfigConstants.ObservabilityApiOtelWriteScope, "Delegated"), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application"), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated"), ("Power Platform API", PowerPlatformConstants.PowerPlatformApiResourceAppId, PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, "Delegated"), - ]; - } + }; - /// - /// Fixed platform APIs that expose an application (S2S) app role, used to render the manual - /// PowerShell hand-off when the programmatic assignment could not complete. - /// - internal static readonly IReadOnlyList<(string ResourceName, string ResourceAppId, string Role)> FixedApiAppRoleHandoffSpecs = - [ - ("Observability API", ConfigConstants.ObservabilityApiAppId, ConfigConstants.ObservabilityApiOtelWriteScope), - ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope), - ]; + if (defenderPermissionMode is DefenderPermissionMode.Application or DefenderPermissionMode.Both) + specs.Insert(2, ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Application")); + if (defenderPermissionMode is DefenderPermissionMode.Delegated or DefenderPermissionMode.Both) + specs.Insert(defenderPermissionMode == DefenderPermissionMode.Both ? 3 : 2, + ("Defender API", ConfigConstants.DefenderApiAppId, ConfigConstants.DefenderApiRealtimeProtectionScope, "Delegated")); + + return specs; + } /// /// Logs step-by-step instructions for a Global Administrator to grant admin consent @@ -944,7 +953,7 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str if (hasActionRequired) { var blueprintAppId = results.BlueprintId ?? ""; - var consentUrl = results.CombinedConsentUrl ?? results.AdminConsentUrl; + var consentUrl = results.AdminConsentUrl ?? results.CombinedConsentUrl; logger.LogInformation(""); logger.LogInformation("Action Required:"); @@ -967,7 +976,15 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str if (isNonDw && string.IsNullOrWhiteSpace(consentUrl)) { logger.LogInformation(" {N}. Permission Grants — must be granted by {Roles} in the Entra portal:", actionCount, AuthenticationConstants.DelegatedGrantRequiredRoles); - var consentSpecs = BuildNonDwAdminConsentSpecs(observabilityResourceAppId); + var defenderPermissionMode = results.EffectiveAuthMode switch + { + Models.AuthMode.S2s => DefenderPermissionMode.Application, + Models.AuthMode.Both => DefenderPermissionMode.Both, + _ => DefenderPermissionMode.Delegated, + }; + var consentSpecs = BuildNonDwAdminConsentSpecs( + observabilityResourceAppId, + defenderPermissionMode); if (results.ObservabilityPermissionsSkipped) consentSpecs = consentSpecs.Where(s => !ConfigConstants.IsObservabilityApiAppId(s.ResourceAppId)).ToList(); LogNonDwAdminConsentInstructions( @@ -1259,10 +1276,9 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str /// resources. Called when the current user lacks the Global Administrator role so that the URLs /// can be saved to a365.generated.config.json and shared with a tenant administrator. /// - /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated; the Observability - /// API URL is generated unless is false. Messaging Bot API is included only - /// when is true — non-M365 tenants typically lack the Messaging Bot - /// resource SP and the consent endpoint returns AADSTS650053 otherwise. + /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated. Observability + /// and delegated Defender URLs are generated only when their corresponding include flags are true. + /// Messaging Bot API is included only when is true. /// /// /// Display names of the resources for which URLs were saved. @@ -1273,7 +1289,8 @@ internal static List PopulateAdminConsentUrls( bool isM365 = true, IReadOnlyDictionary? mcpScopesByAudience = null, IReadOnlyDictionary>? mcpAudienceDisplayNames = null, - bool includeObservability = true) + bool includeObservability = true, + bool includeDefenderDelegated = true) { var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(config.Environment, config.GraphBaseUrl); var graphResourceUri = graphBaseUrl; @@ -1283,11 +1300,13 @@ internal static List PopulateAdminConsentUrls( var urls = BuildAdminConsentUrls( config.TenantId, config.AgentBlueprintId!, config.AgentApplicationScopes, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId, includeObservability); + mcpResourceAppId, observabilityResourceAppId, includeObservability, includeDefenderDelegated); // Clear an Observability consent URL saved by an earlier run so the admin is not asked for permissions this run skipped. if (!includeObservability) ClearSkippedObservabilityConsentUrl(config); + if (!includeDefenderDelegated) + ClearSkippedDefenderConsentUrl(config); // Map resource names to App IDs for upsert into ResourceConsents. The fixed-name // entries cover Graph + Bot + Obs + PP + the WorkIQ shared MCP audience. V2 @@ -1469,7 +1488,8 @@ internal static string BuildFullyQualifiedScope( /// (mirrors ): Microsoft Graph (when /// non-empty), Agent 365 Tools (when /// non-empty), Messaging Bot API (when is true), Observability API - /// (unless is false), and Power Platform API. + /// (unless is false), Defender API (when delegated + /// Defender consent is enabled), and Power Platform API. /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, in which case the /v2.0/adminconsent endpoint returns @@ -1489,7 +1509,8 @@ internal static string BuildFullyQualifiedScope( string? authorityHost = null, string? sharedMcpResourceAppId = null, string? observabilityResourceAppId = null, - bool includeObservability = true) + bool includeObservability = true, + bool includeDefenderDelegated = true) { var urls = new List<(string, string)>(); @@ -1558,7 +1579,8 @@ string Build(string tenant, string client, string resourceUri, IEnumerable /// Builds a single combined /v2.0/adminconsent URL covering every resource stamped on the /// blueprint: Graph, Agent 365 Tools (MCP), Observability API (unless - /// is false), Power Platform API, and - /// Messaging Bot API (only when is true). + /// is false), Defender API when delegated Defender + /// consent is enabled, Power Platform API, and Messaging Bot API (only when + /// is true). /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, which would cause the entire combined consent grant @@ -1587,7 +1610,8 @@ internal static string BuildCombinedConsentUrl( string? authorityHost = null, string? sharedMcpResourceAppId = null, string? observabilityResourceAppId = null, - bool includeObservability = true) + bool includeObservability = true, + bool includeDefenderDelegated = true) { var allScopes = new List(); foreach (var s in graphScopes) @@ -1623,7 +1647,8 @@ internal static string BuildCombinedConsentUrl( if (includeObservability) allScopes.Add( $"{ConfigConstants.BuildObservabilityApiIdentifierUri(observabilityResourceAppId ?? ConfigConstants.ObservabilityApiAppId)}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); - allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); + if (includeDefenderDelegated) + allScopes.Add($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes, authorityHost); } @@ -1633,11 +1658,9 @@ internal static string BuildCombinedConsentUrl( /// when the running account is not a Global Administrator. Called by both DW and non-DW setup paths /// after the batch permissions step. /// - /// Messaging Bot API URLs are included only when is true, and - /// Observability API URLs only when the context requests Observability permissions; the other - /// resources (Graph, MCP, Power Platform) are always included so a tenant admin - /// can complete the hand-off with a single URL. When Observability permissions are skipped, a - /// consent URL saved for them by an earlier run is cleared on every run, admin runs included. + /// Messaging Bot API URLs are included only when is true. + /// Observability and delegated Defender URLs follow the current setup request; stale URLs from + /// earlier runs are cleared when either permission is no longer requested. /// Otherwise this is a no-op if admin consent was already granted or the blueprint ID is absent. /// /// @@ -1650,25 +1673,31 @@ internal static void ApplyConsentUrlsIfNeeded( IReadOnlyDictionary? mcpScopesByAudience = null, IReadOnlyDictionary>? mcpAudienceDisplayNames = null) { - // Before the early return, so an admin run also clears a URL saved by an earlier non-admin run. + var includeDefenderDelegated = !ctx.Results.IsNonDwBlueprintFlow || !ctx.IsS2sMode; + + // Before the early return, so an admin run also clears URLs saved by an earlier non-admin run. if (ctx.ObservabilityPermissionsEffectivelySkipped) ClearSkippedObservabilityConsentUrl(ctx.Config); + if (!includeDefenderDelegated) + ClearSkippedDefenderConsentUrl(ctx.Config); if (ctx.Results.TenantWideConsentOutcome == Models.GrantOutcome.Granted || string.IsNullOrWhiteSpace(ctx.Config.AgentBlueprintId)) return; var includeObservability = !ctx.SkipObservabilityPermissions; - var consentResourceNames = PopulateAdminConsentUrls(ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, includeObservability); + var consentResourceNames = PopulateAdminConsentUrls( + ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, + mcpAudienceDisplayNames, includeObservability, includeDefenderDelegated); ctx.Results.ConsentUrlsSavedToPath = ctx.GeneratedConfigPath; ctx.Results.ConsentResourceNames.AddRange(consentResourceNames); var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(ctx.Config.Environment, ctx.Config.GraphBaseUrl); var graphResourceUri = graphBaseUrl; var authorityHost = ConfigConstants.GetAuthorityHost(ctx.Config.Environment, ctx.Config.AuthorityHost); var observabilityResourceAppId = ConfigConstants.GetObservabilityApiAppId(ctx.Config.Environment); - ctx.Results.CombinedConsentUrl = BuildCombinedConsentUrl( + ctx.Results.CombinedConsentUrl = ctx.Results.AdminConsentUrl ?? BuildCombinedConsentUrl( ctx.Config.TenantId!, ctx.Config.AgentBlueprintId!, graphScopes, mcpScopes, isM365, mcpScopesByAudience, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId, includeObservability); + mcpResourceAppId, observabilityResourceAppId, includeObservability, includeDefenderDelegated); } /// @@ -1682,6 +1711,15 @@ internal static void ClearSkippedObservabilityConsentUrl(Agent365Config config) consent.ConsentUrl = null; } + internal static void ClearSkippedDefenderConsentUrl(Agent365Config config) + { + foreach (var consent in config.ResourceConsents.Where( + rc => string.Equals(rc.ResourceAppId, ConfigConstants.DefenderApiAppId, StringComparison.OrdinalIgnoreCase))) + { + consent.ConsentUrl = null; + } + } + /// /// Prints the dry-run plan for the AI Teammate agent (--aiteammate true) path of setup all. /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs index 088761c2..b8b34191 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -201,8 +201,8 @@ public class SetupResults /// /// The effective --authmode value used during the non-DW grant step. - /// Null when the non-DW grant step was not reached (e.g. agent identity creation failed) or - /// when the run is a DW (AI Teammate) flow — DW does not use --authmode. + /// Set at the start of a non-DW run so permission construction and failure summaries share the + /// same mode. Null for DW (AI Teammate) flows, which do not use --authmode. /// Used by DisplaySetupSummary to compute per-grant-type completion for the "both" mode and to /// derive which Action Required items apply. /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs index eba21e42..b101f1d1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs @@ -60,7 +60,7 @@ public sealed class LogRedactionService : ILogRedactionService "00000003-0000-0000-c000-000000000000", // Microsoft Graph "5a807f24-c9de-44ee-a3a7-329e88a00ffc", // Agent 365 Messaging Bot API "9b975845-388f-4429-889e-eab1ef63949c", // Agent 365 Observability API - "86a21212-634e-4553-b3d6-e477e4c9d9ec", // Agent 365 Defender API + ConfigConstants.DefenderApiAppId, ConfigConstants.GccObservabilityApiAppId, ConfigConstants.GccHighObservabilityApiAppId, ConfigConstants.DodObservabilityApiAppId, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index c7450b9c..c709baed 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs @@ -358,7 +358,11 @@ public async Task ExecuteMessagingEndpointStepAsync_WhenOverrideProvidedAndConfi // Observability API permission wiring // ----------------------------------------------------------------------- - private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, List? customPermissions = null) + private SetupContext BuildPermissionsContext( + bool skipObservabilityPermissions, + List? customPermissions = null, + string? authMode = null, + bool isNonDwBlueprintFlow = true) { var executor = Substitute.For(Substitute.For>()); var graph = Substitute.For(); @@ -368,6 +372,8 @@ private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, Arg.Any(), Arg.Any(), Arg.Any?>(), Arg.Any()) .Returns(new List<(string ResourceAppId, bool ScopesAllAllowed, bool RolesAllAllowed)>()); + var results = new SetupResults { IsNonDwBlueprintFlow = isNonDwBlueprintFlow }; + return new SetupContext( config: new Agent365Config { @@ -378,7 +384,7 @@ private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, DeploymentProjectPath = _tempDir, CustomBlueprintPermissions = customPermissions, }, - results: new SetupResults(), + results: results, logger: NullLogger.Instance, configFile: new FileInfo(Path.Combine(_tempDir, "a365.config.json")), generatedConfigPath: Path.Combine(_tempDir, "a365.generated.config.json"), @@ -398,6 +404,7 @@ private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions, federatedCredentialService: Substitute.ForPartsOf( Substitute.For>(), graph), clientAppValidator: Substitute.For(), + authMode: authMode, skipObservabilityPermissions: skipObservabilityPermissions); } @@ -412,17 +419,63 @@ public async Task BuildPermissionSpecsAsync_StampsObservabilityApiUnlessSkipped( specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId).Should().Be(!skipObservabilityPermissions, because: "the spec list drives inheritable permissions, app role grants, and admin consent, so skipping Observability permissions must remove Observability API from it"); - specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId && - s.AppRoleScopes is { Length: > 0 }).Should().Be(!skipObservabilityPermissions, - because: "skipping Observability permissions must remove the OtelWrite app role without affecting application roles required by other resources"); - specs.Should().Contain(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId && - s.AppRoleScopes != null && - s.AppRoleScopes.Contains(ConfigConstants.DefenderApiRealtimeProtectionScope), - because: "Defender independently requires RealtimeProtection.Evaluate.All as an application permission"); + specs.Should().Contain(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId, + because: "skipping Observability permissions must not remove the Defender permission required by the selected auth mode"); specs.Should().Contain(s => s.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, because: "skipping Observability API must not drop the other required resources"); } + [Theory] + [InlineData(null, true, false)] + [InlineData("obo", true, false)] + [InlineData("s2s", false, true)] + [InlineData("both", true, true)] + public async Task BuildPermissionSpecsAsync_NonDw_DefenderPermissionsMatchAuthMode( + string? authMode, + bool expectDelegated, + bool expectApplication) + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: true, + authMode: authMode, + isNonDwBlueprintFlow: true); + + var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); + + var defender = specs.Single(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId); + defender.Scopes.Should().BeEquivalentTo( + expectDelegated ? [ConfigConstants.DefenderApiRealtimeProtectionScope] : [], + because: $"the '{authMode ?? "obo"}' auth mode must request delegated Defender consent only when OBO is enabled"); + if (expectApplication) + { + defender.AppRoleScopes.Should().BeEquivalentTo( + [ConfigConstants.DefenderApiRealtimeProtectionScope], + because: $"the '{authMode ?? "obo"}' auth mode enables S2S Defender evaluation"); + } + else + { + defender.AppRoleScopes.Should().BeNull( + because: $"the '{authMode ?? "obo"}' auth mode must not request a Defender application role"); + } + } + + [Fact] + public async Task BuildPermissionSpecsAsync_Dw_PreservesBothDefenderPermissionTypes() + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: false, + authMode: "obo", + isNonDwBlueprintFlow: false); + + var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); + + var defender = specs.Single(s => s.ResourceAppId == ConfigConstants.DefenderApiAppId); + defender.Scopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], + because: "DW setup does not use blueprint-agent auth modes and must preserve its delegated Defender permission"); + defender.AppRoleScopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], + because: "DW setup must preserve its existing Defender application permission"); + } + [Fact] public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRemainingResources() { @@ -439,6 +492,49 @@ public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRem because: "the single hand-off URL must not request Observability API scopes that setup skipped"); } + [Fact] + public void ApplyConsentUrlsIfNeeded_S2s_OmitsDelegatedDefenderConsentAndClearsStaleUrl() + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: true, + authMode: "s2s", + isNonDwBlueprintFlow: true); + ctx.Config.ResourceConsents.Add(new ResourceConsent + { + ResourceName = "Defender API", + ResourceAppId = ConfigConstants.DefenderApiAppId, + ConsentUrl = "https://login.microsoftonline.com/stale-defender-consent", + }); + + SetupHelpers.ApplyConsentUrlsIfNeeded( + ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, + new[] { "McpServers.Mail.All" }, isM365: false); + + ctx.Results.ConsentResourceNames.Should().NotContain("Defender API", + because: "S2S-only mode requests the Defender application role, not its delegated scope"); + ctx.Results.CombinedConsentUrl.Should().NotContain( + Uri.EscapeDataString($"{ConfigConstants.DefenderApiIdentifierUri}/{ConfigConstants.DefenderApiRealtimeProtectionScope}"), + because: "the S2S handoff URL must not request delegated Defender admin consent"); + ctx.Config.ResourceConsents.Single(rc => rc.ResourceAppId == ConfigConstants.DefenderApiAppId) + .ConsentUrl.Should().BeNull( + because: "switching from OBO to S2S must clear a stale delegated Defender consent URL"); + } + + [Fact] + public void ApplyConsentUrlsIfNeeded_WhenOrchestratorFilteredMissingResource_PreservesFilteredUrl() + { + const string filteredUrl = "https://login.microsoftonline.com/tenant/v2.0/adminconsent?client_id=blueprint&scope=https%3A%2F%2Fgraph.microsoft.com%2FUser.Read"; + var ctx = BuildPermissionsContext(skipObservabilityPermissions: true); + ctx.Results.AdminConsentUrl = filteredUrl; + + SetupHelpers.ApplyConsentUrlsIfNeeded( + ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, + new[] { "McpServers.Mail.All" }, isM365: false); + + ctx.Results.CombinedConsentUrl.Should().Be(filteredUrl, + because: "the orchestrator already excluded unresolved service principals, so rebuilding the URL would reintroduce a resource that poisons the entire consent request"); + } + [Fact] public void ApplyConsentUrlsIfNeeded_AdminRun_ClearsObservabilityConsentUrlSavedByAnEarlierRun() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs index fa985611..184ef64c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs @@ -312,12 +312,12 @@ private void ArrangeS2SPhase1AndAdminCheck() .Returns(Task.FromResult((ok: false, alreadyExists: false, error: (string?)"Insufficient privileges"))); } - private static ResourcePermissionSpec[] S2SSpec() => + private static ResourcePermissionSpec[] S2SSpec(bool includeDelegatedScope = true) => [ new ResourcePermissionSpec( ConfigConstants.ObservabilityApiAppId, "Observability API", - new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, + includeDelegatedScope ? new[] { ConfigConstants.ObservabilityApiOtelWriteScope } : [], SetInheritable: false, AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope }) ]; @@ -501,8 +501,11 @@ private void ArrangeAzRestS2SCalls(bool blueprintAlreadyAssigned, int postExitCo /// DisplaySetupSummary surfaces the S2S hand-off block in the Action Required section — /// just like it does for a GA whose Graph API call returns 403. /// - [Fact] - public async Task ConfigureAllPermissions_NonAdmin_WithS2SSpecs_SetsBlueprintS2SOutcomeFailed() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ConfigureAllPermissions_NonAdmin_WithS2SSpecs_SetsBlueprintS2SOutcomeFailed( + bool includeDelegatedScope) { // Arrange _graph.GraphGetAsync( @@ -534,13 +537,13 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( _graph, _blueprintService, new Agent365Config { TenantId = S2STenantId, AgentBlueprintId = S2SBlueprintAppId }, blueprintAppId: S2SBlueprintAppId, tenantId: S2STenantId, - specs: S2SSpec(), _logger, setupResults, ct: default); + specs: S2SSpec(includeDelegatedScope), _logger, setupResults, ct: default); // Assert setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Failed, because: "a non-admin user cannot complete S2S app role assignment directly — the outcome must be marked Failed so DisplaySetupSummary surfaces the hand-off block"); setupResults.PendingBlueprintAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId, - because: "a non-admin run leaves every requested app role for the summary's hand-off"); + because: "a non-admin run leaves every requested app role for the summary's hand-off, including application-only specs"); } // ────────────────────────────────────────────────────────────────────────────────────── diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs index 8c21e63e..f5341558 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs @@ -145,9 +145,7 @@ public void PrintDryRunPlan_IncludesAgentRegistrationStep() // ── authMode dry-run output ──────────────────────────────────────────────── /// - /// OBO mode must show delegated (principal-scoped) grants on the agent identity SP. - /// authMode controls only the agent-identity grant style; the blueprint step is independent - /// and always uses AllPrincipals grants on the blueprint (issue #417). + /// OBO mode must show delegated grants without a Defender application-role handoff. /// [Fact] public void PrintDryRunPlan_AuthModeObo_ShowsDelegatedGrantsOnAgentIdentity() @@ -158,13 +156,12 @@ public void PrintDryRunPlan_AuthModeObo_ShowsDelegatedGrantsOnAgentIdentity() } /// - /// OBO mode still grants fixed application roles on the blueprint independently of the - /// principal-scoped delegated grants applied to the agent identity. + /// OBO mode requests the Defender delegated permission only. /// [Theory] [InlineData("obo")] [InlineData(null)] - public void PrintDryRunPlan_AuthModeObo_ShowsDefenderApplicationRoleGrantOnBlueprint(string? authMode) + public void PrintDryRunPlan_AuthModeObo_OmitsDefenderApplicationRoleGrant(string? authMode) { NonDwBlueprintSetupOrchestrator.PrintDryRunPlan( BuildConfig(), @@ -172,16 +169,18 @@ public void PrintDryRunPlan_AuthModeObo_ShowsDefenderApplicationRoleGrantOnBluep authMode: authMode, skipObservabilityPermissions: true); - AnyLogContains("delegated grants for the signed-in principal + S2S app roles").Should().BeTrue( - because: "the blueprint receives the Defender application role in OBO mode even though the agent identity uses delegated grants"); - AnyLogContains("Global Administrator required for S2S if 403").Should().BeTrue( - because: "a non-admin dry run must surface the administrative handoff required for the Defender application role"); + AnyLogContains("delegated grants").Should().BeTrue( + because: "OBO mode requires the Defender delegated permission"); + AnyLogContains("S2S app roles").Should().BeFalse( + because: "OBO mode must not request the Defender application role"); + AnyLogContains("Global Administrator required for S2S if 403").Should().BeFalse( + because: "OBO mode has no Defender application-role handoff"); } /// /// S2S mode must show application permissions on the agent identity SP and must not show /// delegated grants — there is no user context in S2S so delegated scopes don't apply. - /// authMode only affects the agent-identity step; the blueprint step is independent. + /// Defender follows authMode on both the blueprint and agent identity. /// [Fact] public void PrintDryRunPlan_AuthModeS2s_ShowsAppPermsOnAgentIdentity_NoDelegatedGrants() diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs index 4835986f..fe5d1736 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersAdminConsentInstructionsTests.cs @@ -162,6 +162,26 @@ public void GetNonDwAdminConsentSpecs_ForGcc_UsesGccObservabilityResource() because: "manual GCC consent instructions must target the GCC Observability resource"); } + [Theory] + [InlineData("Delegated", true, false)] + [InlineData("Application", false, true)] + [InlineData("Both", true, true)] + public void GetNonDwAdminConsentSpecs_DefenderPermissionsMatchAuthMode( + string modeName, + bool expectDelegated, + bool expectApplication) + { + var mode = Enum.Parse(modeName); + var specs = SetupHelpers.GetNonDwAdminConsentSpecs("prod", mode) + .Where(spec => spec.ResourceName == "Defender API") + .ToList(); + + specs.Any(spec => spec.PermissionType == "Delegated").Should().Be(expectDelegated, + because: $"{mode} must include delegated Defender consent exactly when OBO is enabled"); + specs.Any(spec => spec.PermissionType == "Application").Should().Be(expectApplication, + because: $"{mode} must include the Defender app role exactly when S2S is enabled"); + } + [Fact] public void NonDwAdminConsentSpecs_PowerPlatformApi_IsDelegatedOnly() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs index b4f3f316..8d155c7e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs @@ -431,6 +431,69 @@ public void DisplaySetupSummary_NonDwAdminConsentPending_WithConsentUrl_ShowsUrl because: "the portal walkthrough is only a defensive fallback for the case where no consent URL was produced; when a URL is available the URL block must be used"); } + [Fact] + public void DisplaySetupSummary_NonDwAdminConsentPending_PrefersFilteredAdminConsentUrl() + { + var logger = new CapturingLogger(); + const string filteredUrl = "https://login.microsoftonline.com/tenant/v2.0/adminconsent?scope=filtered"; + const string reconstructedUrl = "https://login.microsoftonline.com/tenant/v2.0/adminconsent?scope=includes-missing-defender"; + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + TenantId = TenantId, + EffectiveAuthMode = Cli.Models.AuthMode.Obo, + TenantWideConsentOutcome = Cli.Models.GrantOutcome.Failed, + BatchPermissionsPhase1Completed = true, + BatchPermissionsPhase2Completed = true, + AdminConsentUrl = filteredUrl, + CombinedConsentUrl = reconstructedUrl, + }; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain(filteredUrl, + because: "the orchestrator URL excludes unresolved service principals and is the only safe primary handoff"); + logger.AllOutput.Should().NotContain(reconstructedUrl, + because: "a reconstructed URL can reintroduce a missing resource and fail the entire consent request"); + } + + [Fact] + public void DisplaySetupSummary_NonDwGccS2sPending_UsesPendingCloudSpecificSpec() + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + TenantId = TenantId, + EffectiveAuthMode = Cli.Models.AuthMode.S2s, + TenantWideConsentOutcome = Cli.Models.GrantOutcome.Granted, + BlueprintS2SOutcome = Cli.Models.GrantOutcome.Failed, + BatchPermissionsPhase1Completed = true, + BatchPermissionsPhase2Completed = true, + }; + results.PendingBlueprintAppRoleSpecs.Add(new ResourcePermissionSpec( + ConfigConstants.GccObservabilityApiAppId, + "Observability API", + [], + SetInheritable: true, + AppRoleScopes: [ConfigConstants.ObservabilityApiOtelWriteScope])); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain(ConfigConstants.GccObservabilityApiAppId, + because: "manual S2S recovery must use the cloud-aware resource from the failed permission spec"); + logger.AllOutput.Should().NotContain(ConfigConstants.ObservabilityApiAppId, + because: "GCC recovery must not target the commercial Observability application"); + } + [Fact] public void DisplaySetupSummary_NonDwAdminConsentPending_NoConsentUrl_FallsBackToPortalWalkthrough() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/LogRedactionServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/LogRedactionServiceTests.cs index 78a76a99..98a3c98a 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/LogRedactionServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/LogRedactionServiceTests.cs @@ -107,6 +107,17 @@ public void Redact_ObservabilityResourceAppId_IsPreserved(string appId) because: "well-known first-party resource IDs do not identify a tenant or user"); } + [Fact] + public void Redact_DefenderResourceAppId_IsPreserved() + { + var result = _sut.Redact($"[INF] Defender resource: {ConfigConstants.DefenderApiAppId}", Source); + + result.RedactedContent.Should().Contain(ConfigConstants.DefenderApiAppId, + because: "the public Defender resource ID must remain visible for support diagnostics"); + result.IdsRedacted.Should().Be(0, + because: "a well-known first-party resource ID does not identify a tenant or user"); + } + [Fact] public void Redact_SameGuidAppearsMultipleTimes_SameAliasUsed() { From 5e1d56ac6d4285d5766313fa04670eae005067ca Mon Sep 17 00:00:00 2001 From: slreznit Date: Wed, 7 Oct 2026 23:39:50 +0300 Subject: [PATCH 10/14] Fix Defender consent state tracking Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- CHANGELOG.md | 2 +- .../BatchPermissionsOrchestrator.cs | 7 ++++-- .../BatchPermissionsOrchestratorTests.cs | 24 +++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd59fabb..8ade9457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ Blueprint agents that export telemetry through the app-only S2S endpoint don't n #### Existing agents: grant Defender API permissions -Agents provisioned before this release need `RealtimeProtection.Evaluate.All` on the blueprint app for the Defender security integration: **delegated** for `--authmode obo`, **application** for `--authmode s2s`, or both for `--authmode both`. Re-run `a365 setup all --authmode ` as a Global Administrator; this stamps the Defender inheritable-permission entry and grants the permission required by that auth mode. Adding only the API permission in the Entra portal is insufficient for older blueprints that do not yet have the Defender inheritable-permission entry. +Agents provisioned before this release should have a Global Administrator re-run `a365 setup all --authmode ` to stamp Defender inheritance and grant `RealtimeProtection.Evaluate.All` as delegated for `obo`, application for `s2s`, or both for `both`, because portal-only grants do not add inheritance to older blueprints. ### Added diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index cc2e0ee4..ea9ff9a6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -960,7 +960,10 @@ internal static void UpdateResourceConsents( IReadOnlyList specs, Dictionary inheritedResults) { - var configuredObservabilityAppIds = specs + var delegatedSpecs = specs + .Where(spec => spec.Scopes.Length > 0) + .ToList(); + var configuredObservabilityAppIds = delegatedSpecs .Where(spec => ConfigConstants.IsObservabilityApiAppId(spec.ResourceAppId)) .Select(spec => spec.ResourceAppId) .ToHashSet(StringComparer.OrdinalIgnoreCase); @@ -972,7 +975,7 @@ internal static void UpdateResourceConsents( !configuredObservabilityAppIds.Contains(resourceConsent.ResourceAppId)); } - foreach (var spec in specs) + foreach (var spec in delegatedSpecs) { inheritedResults.TryGetValue(spec.ResourceAppId, out var inherited); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs index 184ef64c..c4df3dc2 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs @@ -906,6 +906,30 @@ public void UpdateResourceConsents_ReplacesCommercialObservabilityEntryForGcc() because: "a GCC rerun must replace stale commercial Observability state"); } + [Fact] + public void UpdateResourceConsents_ApplicationOnlySpec_DoesNotRecordDelegatedConsent() + { + var config = new Agent365Config(); + var specs = new[] + { + new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + [], + SetInheritable: true, + AppRoleScopes: [ConfigConstants.DefenderApiRealtimeProtectionScope]), + }; + var inheritedResults = new Dictionary + { + [ConfigConstants.DefenderApiAppId] = (true, false), + }; + + BatchPermissionsOrchestrator.UpdateResourceConsents(config, specs, inheritedResults); + + config.ResourceConsents.Should().BeEmpty( + because: "application-role outcomes are tracked separately and must not be persisted as verified delegated consent with an empty scope list"); + } + /// /// Non-admin path: GrantAdminConsentAsync builds the unified consent URL via the catch-all /// spec loop and returns it for hand-off. When a spec's appId is in knownMcpAudienceAppIds, From cf2e31e27662907e3fe5ea2b6e2ce582de6da131 Mon Sep 17 00:00:00 2001 From: slreznit Date: Fri, 9 Oct 2026 09:06:44 +0300 Subject: [PATCH 11/14] Handle missing application-only service principals Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- .../BatchPermissionsOrchestrator.cs | 30 +++++--- .../NonDwBlueprintSetupOrchestrator.cs | 4 +- .../Commands/SetupSubcommands/README.md | 2 +- .../Commands/SetupSubcommands/SetupContext.cs | 4 +- .../Commands/SetupSubcommands/SetupHelpers.cs | 23 +++--- .../Commands/SetupSubcommands/SetupResults.cs | 17 +++-- ...chPermissionsOrchestratorMissingSpTests.cs | 70 ++++++++++++++++++- .../SetupHelpersDisplaySetupSummaryTests.cs | 37 ++++++++++ 8 files changed, 153 insertions(+), 34 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index ea9ff9a6..f28812c9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -670,15 +670,13 @@ private static void SetPendingBlueprintAppRoleSpecs(SetupResults setupResults, I // first-party MCP audiences fail it with AADSTS65003 — token-to-self consent). // EnsureMissingResourceSpsAsync mutates the resolvedSpAppIds set on success and // records MissingSpActions for the rest so the Action Required block renders the - // recovery steps (the az command + a per-SP /v2.0/adminconsent URL keyed to the - // blueprint as client). Skips entirely when skipSpProvisioning is true (flag or + // az command and, for delegated specs, a per-SP consent URL keyed to the blueprint. + // Skips entirely when skipSpProvisioning is true (flag or // auto-detected from stdin) or when there is nothing missing. See helper for the // full state machine. if (resolvedSpAppIds.Count > 0) { - var missingSpecs = specs - .Where(s => s.Scopes is { Length: > 0 } && !resolvedSpAppIds.Contains(s.ResourceAppId)) - .ToList(); + var missingSpecs = FindMissingResourceSpSpecs(specs, resolvedSpAppIds); await EnsureMissingResourceSpsAsync( graph, tenantId, blueprintAppId, missingSpecs, resolvedSpAppIds, permScopes, skipSpProvisioning, logger, setupResults, ct, @@ -951,6 +949,15 @@ await EnsureMissingResourceSpsAsync( return (consentGranted, consentVerified ? null : consentUrl); } + internal static List FindMissingResourceSpSpecs( + IReadOnlyList specs, + IReadOnlySet resolvedSpAppIds) => + specs + .Where(spec => + (spec.Scopes is { Length: > 0 } || spec.AppRoleScopes is { Length: > 0 }) + && !resolvedSpAppIds.Contains(spec.ResourceAppId)) + .ToList(); + /// /// Updates config.ResourceConsents in-memory for each spec based on phase results. /// The caller is responsible for persisting the config via configService.SaveStateAsync. @@ -1279,11 +1286,11 @@ internal static string BuildAzAdSpCreateCommand(string resourceAppId) => /// /// Records a missing-SP action on so the /// setup summary's "Action Required" block renders it as a numbered item. Each entry - /// carries the two concrete artifacts the operator needs to complete provisioning + /// carries the concrete artifacts the operator needs to complete provisioning /// without re-running setup: /// /// az ad sp create --id {appId} — provisions the SP in the tenant. - /// Per-SP /v2.0/adminconsent URL keyed to the blueprint as + /// For delegated scopes only, a per-SP /v2.0/adminconsent URL keyed to the blueprint as /// client and this resource's scopes as the request. After step 1 succeeds, clicking /// this URL grants the blueprint consent for this one resource additively (does not /// wipe other resources' grants), avoiding any need to re-run a365 setup all. @@ -1308,14 +1315,19 @@ private static void RecordMissingSpAction( var azCommand = BuildAzAdSpCreateCommand(spec.ResourceAppId); var isMcpAudience = knownMcpAudienceAppIds?.Contains(spec.ResourceAppId) ?? false; - var perSpConsentUrl = BuildPerSpBlueprintConsentUrl(tenantId, blueprintAppId, spec, isMcpAudience, authorityHost); + var perSpConsentUrl = spec.Scopes is { Length: > 0 } + ? BuildPerSpBlueprintConsentUrl(tenantId, blueprintAppId, spec, isMcpAudience, authorityHost) + : null; setupResults?.MissingSpActions.Add(new MissingSpAction( ResourceName: spec.ResourceName, ResourceAppId: spec.ResourceAppId, Scopes: spec.Scopes?.ToArray() ?? Array.Empty(), AzCreateCommand: azCommand, - PerSpConsentUrl: perSpConsentUrl)); + PerSpConsentUrl: perSpConsentUrl) + { + AppRoleScopes = spec.AppRoleScopes?.ToArray() ?? Array.Empty(), + }); } /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index 0235ccb6..6eb8ac20 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -740,8 +740,8 @@ internal static async Task GrantAgentIdentityS2SPermissionsAsync( List specs) { var hasS2sSpecs = specs.Any(s => s.AppRoleScopes is { Length: > 0 }); - // Blueprint agents no longer request OtelWrite, the only app role setup requested, so this - // step usually has nothing to grant. Record that so the summary does not report a delegated grant. + // Record whether the selected auth mode produced any application permissions so the + // summary can distinguish "no S2S work" from a failed grant. ctx.Results.NoS2SAppRolesToGrant = !hasS2sSpecs; if (hasS2sSpecs && AgentIdentityInheritsBlueprintAppRoles(ctx.Results)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md index 2c4aaebc..e76b84ff 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md @@ -94,7 +94,7 @@ a365 setup all --authmode both ### Observability permissions -For blueprint agents, `setup all` does not request `Agent365.Observability.OtelWrite` in any auth mode. Registered agents export telemetry with an app-only token through the S2S endpoint, which authorizes them by their agent registration, so no Observability admin consent is needed. Registration is then the agent's only authorization, so a registration failure is reported as an error (exit code 1). OtelWrite was the only app role setup requested. Unless another permission adds an app role, `--authmode s2s` and `both` have nothing to assign for blueprint agents, and the setup summary reports the S2S grant as not required. +For blueprint agents, `setup all` does not request `Agent365.Observability.OtelWrite` in any auth mode. Registered agents export telemetry with an app-only token through the S2S endpoint, which authorizes them by their agent registration, so no Observability admin consent is needed. Registration is then the agent's only authorization, so a registration failure is reported as an error (exit code 1). Defender still contributes the `RealtimeProtection.Evaluate.All` application role in `--authmode s2s` and `both`, so those modes perform an S2S grant even when Observability permissions are omitted. Agents whose SDK still exports through the delegated (OBO) route need `OtelWrite`; grant it manually (see the CHANGELOG upgrade note). AI Teammate setup is unchanged. Re-running setup does not revoke permissions granted earlier. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs index 57632ed7..06c15e28 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs @@ -86,8 +86,8 @@ internal sealed class SetupContext /// (Global Administrator's directory role carries the required /// Application.ReadWrite.All). With this set, missing SPs are excluded from the /// unified admin-consent URL and recorded on - /// so the setup summary's Action Required block renders them as numbered items, each - /// with the az ad sp create command and a per-SP /v2.0/adminconsent URL. + /// so the setup summary's Action Required block renders them as numbered items with the + /// az ad sp create command and, for delegated specs, a per-SP consent URL. /// Set explicitly via --skip-sp-provisioning or implicitly when stdin is /// redirected (CI / coding-agent / pipe scenarios). /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index c37530b8..1112937f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -684,9 +684,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str // (e.g. admin already granted tenant consent but the per-principal call still failed). var pendingDelegatedAction = agentIdDelegatedFailed && !pendingAdminAction; var pendingS2SAction = permissionGrantsPending && isS2SFlow; - // Blueprint agents no longer request OtelWrite, the only app role setup requested, so an - // s2s/both run usually has no S2S grant at all. Say so explicitly: otherwise the row falls - // through to the delegated wording, which for s2s-only shows a PENDING with no action item. + // Some configurations can have no application-role specs. Say so explicitly; otherwise + // an s2s-only row can fall through to delegated wording with no matching action item. var noS2SAppRolesToGrant = isNonDw && (isS2sOnlyMode || isBothMode) && results.NoS2SAppRolesToGrant && !isS2SFlow; if (results.PermissionGrantsSkipped && isNonDw) @@ -1160,10 +1159,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str if (hasMissingSpActions) { // Issue #429: resources whose SP could not be provisioned in-line during - // setup. Each entry is a two-step recovery the operator can complete - // without re-running 'a365 setup all': (1) provision the SP via az, - // (2) click the per-SP unified-consent URL to grant the blueprint consent - // for this resource's scopes. Step 2 is keyed to the BLUEPRINT as client + // setup. Every entry provisions the SP via az; delegated specs also include + // a per-SP unified-consent URL. The URL is keyed to the BLUEPRINT as client // (not the resource as client — that pattern fails AADSTS65003 for // first-party token-to-self), so it is a normal cross-app consent and // additive to whatever the unified consent URL already granted. @@ -1171,11 +1168,17 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str { actionCount++; logger.LogInformation(" {N}. Missing service principal — '{Name}' ({AppId}) (Global Administrator required)", actionCount, action.ResourceName, action.ResourceAppId); - logger.LogInformation(" Scopes pending: {Scopes}", string.Join(", ", action.Scopes)); + if (action.Scopes.Length > 0) + logger.LogInformation(" Delegated scopes pending: {Scopes}", string.Join(", ", action.Scopes)); + if (action.AppRoleScopes.Length > 0) + logger.LogInformation(" Application roles pending: {Roles}", string.Join(", ", action.AppRoleScopes)); logger.LogInformation(" Step 1) Provision the SP:"); logger.LogInformation(" {AzCommand}", action.AzCreateCommand); - logger.LogInformation(" Step 2) Grant the blueprint consent for this resource (click Accept):"); - logger.LogInformation(" {Url}", action.PerSpConsentUrl); + if (!string.IsNullOrWhiteSpace(action.PerSpConsentUrl)) + { + logger.LogInformation(" Step 2) Grant the blueprint delegated consent for this resource (click Accept):"); + logger.LogInformation(" {Url}", action.PerSpConsentUrl); + } } } } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs index b8b34191..a4786952 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -326,8 +326,8 @@ public class SetupResults /// Resources whose service principal could not be provisioned in-line during setup /// (operator declined the per-SP prompt, az ad sp create failed, or /// --skip-sp-provisioning was set). Each entry is a fully-actionable pair: the - /// az ad sp create command to provision the SP plus the per-SP unified-consent - /// URL that grants the blueprint consent for this resource's scopes. The setup + /// az ad sp create command to provision the SP plus, when delegated scopes are + /// requested, the per-SP unified-consent URL that grants blueprint consent. The setup /// summary's "Action Required" block renders these as numbered items so the operator /// can complete provisioning without re-running setup. /// @@ -339,19 +339,22 @@ public class SetupResults /// /// One entry in . Resource identity plus the -/// two concrete commands/URLs the operator needs to complete provisioning manually: +/// concrete commands/URLs the operator needs to complete provisioning manually: /// (1) the az ad sp create command that creates the SP in the tenant, and -/// (2) the per-SP /v2.0/adminconsent URL that grants the blueprint consent for -/// this resource's delegated scopes once the SP exists. +/// (2) when delegated scopes are requested, the per-SP /v2.0/adminconsent URL. /// /// Human-readable display name (e.g. "Work IQ Teams MCP"). /// Application ID of the resource (the GUID). /// Delegated scopes the blueprint needs on this resource. /// Copy-paste-able az ad sp create --id .... -/// Per-SP unified-consent URL keyed to the blueprint as client and the resource scopes as the request. +/// Per-SP delegated-consent URL, or null for application-only specs. public sealed record MissingSpAction( string ResourceName, string ResourceAppId, string[] Scopes, string AzCreateCommand, - string PerSpConsentUrl); + string? PerSpConsentUrl) +{ + /// Application roles awaiting assignment after the resource SP is provisioned. + public string[] AppRoleScopes { get; init; } = []; +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs index 84e4f13f..18ed5c47 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs @@ -3,6 +3,7 @@ using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; +using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -23,9 +24,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; /// trusts az when an id is present (no Graph re-poll). When the operator declines, az /// fails, the GUID guard rejects, or --skip-sp-provisioning is set, the helper /// records a on so the setup -/// summary's Action Required block surfaces both the az command AND the per-SP -/// blueprint-as-client consent URL — together they are a complete recovery without -/// re-running a365 setup all. +/// summary's Action Required block surfaces the az command and, for delegated specs, +/// the per-SP blueprint-as-client consent URL. /// /// /// @@ -147,6 +147,70 @@ await _executor.DidNotReceive().ExecuteAsync( because: "the rework moved missing-SP messaging out of the noisy main-output Warnings block and into the focused Action Required block at the end"); } + [Fact] + public async Task SkipSpProvisioning_ApplicationOnlySpec_RecordsCreateActionWithoutConsentUrl() + { + using var bypass = TemporarilyDisableSpProvisioningBypass(); + + _graph + .LookupServicePrincipalByAppIdAsync( + TenantId, + ConfigConstants.DefenderApiAppId, + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult(null)); + + var resolvedSpAppIds = new HashSet(StringComparer.OrdinalIgnoreCase); + var setupResults = new SetupResults(); + var missing = new[] + { + new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + [], + SetInheritable: true, + AppRoleScopes: [ConfigConstants.DefenderApiRealtimeProtectionScope]), + }; + + await BatchPermissionsOrchestrator.EnsureMissingResourceSpsAsync( + _graph, TenantId, BlueprintAppId, missing, resolvedSpAppIds, + permScopes: Array.Empty(), + skipSpProvisioning: true, + _logger, + setupResults: setupResults, + ct: CancellationToken.None, + commandExecutor: _executor); + + var action = setupResults.MissingSpActions.Should().ContainSingle( + because: "an application-only Defender spec still needs an actionable service-principal provisioning handoff").Subject; + action.AzCreateCommand.Should().Be($"az ad sp create --id {ConfigConstants.DefenderApiAppId}", + because: "the operator must be able to provision the missing Defender service principal before assigning its app role"); + action.Scopes.Should().BeEmpty( + because: "S2S-only Defender requests no delegated consent"); + action.AppRoleScopes.Should().BeEquivalentTo([ConfigConstants.DefenderApiRealtimeProtectionScope], + because: "the recovery action must identify the application role that remains pending"); + action.PerSpConsentUrl.Should().BeNull( + because: "application-only recovery must not emit an empty delegated-consent URL"); + } + + [Fact] + public void FindMissingResourceSpSpecs_IncludesApplicationOnlyDefender() + { + var defenderSpec = new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + [], + SetInheritable: true, + AppRoleScopes: [ConfigConstants.DefenderApiRealtimeProtectionScope]); + + var missing = BatchPermissionsOrchestrator.FindMissingResourceSpSpecs( + [defenderSpec], + new HashSet(StringComparer.OrdinalIgnoreCase)); + + missing.Should().ContainSingle().Which.Should().Be(defenderSpec, + because: "S2S application-role assignment cannot succeed until the Defender service principal exists"); + } + [Fact] public async Task NullCommandExecutor_FallsBackToWarningPathAndDoesNotPrompt() { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs index 8d155c7e..983b5e14 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs @@ -494,6 +494,43 @@ public void DisplaySetupSummary_NonDwGccS2sPending_UsesPendingCloudSpecificSpec( because: "GCC recovery must not target the commercial Observability application"); } + [Fact] + public void DisplaySetupSummary_ApplicationOnlyMissingSp_ShowsProvisioningWithoutConsentUrl() + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + TenantId = TenantId, + EffectiveAuthMode = Cli.Models.AuthMode.S2s, + TenantWideConsentOutcome = Cli.Models.GrantOutcome.Granted, + BatchPermissionsPhase1Completed = true, + BatchPermissionsPhase2Completed = true, + }; + results.MissingSpActions.Add(new MissingSpAction( + "Defender API", + ConfigConstants.DefenderApiAppId, + [], + $"az ad sp create --id {ConfigConstants.DefenderApiAppId}", + null) + { + AppRoleScopes = [ConfigConstants.DefenderApiRealtimeProtectionScope], + }); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain($"az ad sp create --id {ConfigConstants.DefenderApiAppId}", + because: "application-only recovery must tell the administrator how to provision the missing Defender service principal"); + logger.AllOutput.Should().Contain($"Application roles pending: {ConfigConstants.DefenderApiRealtimeProtectionScope}", + because: "the handoff must identify the Defender app role that can be assigned after provisioning"); + logger.AllOutput.Should().NotContain("Step 2)", + because: "an application-only spec has no delegated scope and must not emit an empty admin-consent URL"); + } + [Fact] public void DisplaySetupSummary_NonDwAdminConsentPending_NoConsentUrl_FallsBackToPortalWalkthrough() { From d7bc66f1daf18aabe5521ab971de490ddbfac68c Mon Sep 17 00:00:00 2001 From: slreznit Date: Fri, 9 Oct 2026 09:14:24 +0300 Subject: [PATCH 12/14] Verify complete admin consent grants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- CHANGELOG.md | 4 +- .../BatchPermissionsOrchestrator.cs | 19 ++- .../SetupSubcommands/PermissionsSubcommand.cs | 2 +- .../Commands/SetupSubcommands/SetupHelpers.cs | 4 +- .../Services/Helpers/AdminConsentHelper.cs | 105 ++++++++++-- .../design.md | 19 ++- .../Commands/PermissionsSubcommandTests.cs | 25 ++- .../SetupHelpersDisplaySetupSummaryTests.cs | 2 + .../Services/AdminConsentHelperTests.cs | 158 ++++++++++++++++++ 9 files changed, 311 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ade9457..4163fe75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,11 +30,11 @@ Blueprint agents that export telemetry through the app-only S2S endpoint don't n #### Existing agents: grant Defender API permissions -Agents provisioned before this release should have a Global Administrator re-run `a365 setup all --authmode ` to stamp Defender inheritance and grant `RealtimeProtection.Evaluate.All` as delegated for `obo`, application for `s2s`, or both for `both`, because portal-only grants do not add inheritance to older blueprints. +Agents provisioned before this release should have a Global Administrator re-run `a365 setup all --authmode ` for blueprint agents (delegated for `obo`, application for `s2s`, or both for `both`) or `a365 setup all` without `--authmode` for AI Teammates (both permission types) to stamp Defender inheritance and grant `RealtimeProtection.Evaluate.All` (#485). ### Added -- `a365 setup all` now grants the Defender API `RealtimeProtection.Evaluate.All` permission according to `--authmode`: delegated for `obo`, application for `s2s`, and both for `both`. +- `a365 setup all` now grants the Defender API `RealtimeProtection.Evaluate.All` permission according to `--authmode` for blueprint agents (delegated for `obo`, application for `s2s`, both for `both`) and as both delegated and application for AI Teammates (#485). - `a365 develop-mcp grant-agents-access --agent-blueprint-id --mcp-server-name ` reports which agent instances of a blueprint are missing the permission to call a BYO MCP server, and prompts you to select which ones to grant it to (#500). - When more than one Entra application shares the MCP server's name, `a365 develop-mcp grant-agents-access` now lists them all and asks which one to use instead of failing (#500). - `a365 develop-mcp grant-agents-access --help` now lists Microsoft's first-party agent blueprint names and IDs, and the same list is printed when `--agent-blueprint-id` is missing or not a GUID, so you can find the ID without looking it up elsewhere (#500). diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index f28812c9..4175f9a8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -692,6 +692,19 @@ await EnsureMissingResourceSpsAsync( var specsForUrl = resolvedSpAppIds.Count > 0 ? specs.Where(s => resolvedSpAppIds.Contains(s.ResourceAppId)).ToList() : specs.ToList(); + var consentRequirements = specsForUrl + .Where(spec => spec.Scopes is { Length: > 0 }) + .Select(spec => + { + string? resourceSpId = null; + phase1Result?.ResourceSpObjectIds.TryGetValue(spec.ResourceAppId, out resourceSpId); + return new AdminConsentRequirement( + spec.ResourceName, + spec.ResourceAppId, + spec.Scopes, + resourceSpId); + }) + .ToList(); var sharedMcpResourceAppId = ConfigConstants.GetAgent365ToolsResourceAppId(config.Environment); var allScopes = specsForUrl @@ -844,7 +857,8 @@ await EnsureMissingResourceSpsAsync( var found = await AdminConsentHelper.PollAdminConsentAsync( commandExecutor, logger, blueprintAppId, "All permissions", timeoutSeconds: 180, intervalSeconds: 5, ct, - graphBaseUrl: graph.GraphBaseUrl); + graphBaseUrl: graph.GraphBaseUrl, + requiredGrants: consentRequirements); consentVerified = found; // Browser was opened regardless — either the grant was directly observed (Verified) // or the timeout elapsed without observing it (AssumedComplete). Either way, setup @@ -859,7 +873,8 @@ await EnsureMissingResourceSpsAsync( var pollResult = await AdminConsentHelper.PollAdminConsentAsync( graph, logger, tenantId, phase1Result.BlueprintSpObjectId, "All permissions", timeoutSeconds: 180, intervalSeconds: 5, ct, - permScopes: AuthenticationConstants.BlueprintOperationScopes); + permScopes: AuthenticationConstants.BlueprintOperationScopes, + requiredGrants: consentRequirements); consentVerified = pollResult == ConsentPollResult.Verified; consentGranted = pollResult != ConsentPollResult.NotDetected; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index 84cba386..a4ff7b60 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -296,7 +296,7 @@ private static Command CreateBotSubcommand( if (dryRunConfig is null) { logger.LogInformation("Dry run: a365 setup permissions bot --dry-run"); - logger.LogInformation(" Would configure Messaging Bot API OAuth2 grants and inheritable permissions."); + logger.LogInformation(" Would configure Messaging Bot, Observability, Defender, and Power Platform API grants and inheritable permissions."); logger.LogInformation("No changes made. Run without --dry-run to execute."); return; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 1112937f..5083d22b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -1260,8 +1260,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str { nextStepLines.Add(() => logger.LogInformation(" 1. Run 'a365 setup permissions mcp' to configure MCP permissions")); nextStepLines.Add(() => logger.LogInformation(results.IsM365 - ? " 2. Run 'a365 setup permissions bot' to configure Bot API, Observability, and Power Platform permissions" - : " 2. Run 'a365 setup permissions bot' to configure Observability and Power Platform permissions")); + ? " 2. Run 'a365 setup permissions bot' to configure Bot API, Observability, Defender, and Power Platform permissions" + : " 2. Run 'a365 setup permissions bot' to configure Observability, Defender, and Power Platform permissions")); } if (nextStepLines.Count > 0) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs index d827c5cc..9a14a01e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs @@ -2,6 +2,8 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; +using System.Linq; using System.Text.Json; using System.Threading; using System.Threading.Tasks; @@ -18,8 +20,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; public enum ConsentPollResult { /// - /// An oauth2PermissionGrant for the client SP was observed in Graph. Safe to mark - /// consent as granted in persisted state. + /// Every requested AllPrincipals resource grant and scope was observed in Graph. Safe + /// to mark consent as granted in persisted state. /// Verified, @@ -38,6 +40,15 @@ public enum ConsentPollResult NotDetected } +/// +/// One resource grant that must be observed before admin-consent polling can report success. +/// +public sealed record AdminConsentRequirement( + string ResourceName, + string ResourceAppId, + IReadOnlyCollection Scopes, + string? ResourceSpObjectId = null); + /// /// Helper methods for admin consent flows that use az cli to poll Graph resources. /// Kept intentionally small and focused so it can be reused across commands/runners. @@ -68,7 +79,7 @@ internal static bool BypassConsentChecksForTests private static readonly AsyncLocal _bypassConsentChecks = new(); /// - /// Polls Azure AD/Graph (via az rest) to detect an oauth2 permission grant for the provided appId. + /// Polls Azure AD/Graph (via az rest) until every requested resource grant is present. /// Mirrors the behavior previously implemented in A365SetupRunner.PollAdminConsentAsync. /// public static async Task PollAdminConsentAsync( @@ -79,7 +90,8 @@ public static async Task PollAdminConsentAsync( int timeoutSeconds, int intervalSeconds, CancellationToken ct, - string? graphBaseUrl = null) + string? graphBaseUrl = null, + IReadOnlyCollection? requiredGrants = null) { if (BypassConsentChecksForTests) return true; @@ -87,6 +99,7 @@ public static async Task PollAdminConsentAsync( var start = DateTime.UtcNow; var baseUrl = ConfigConstants.NormalizeGraphBaseUrl(graphBaseUrl); string? spId = null; + var resolvedRequirements = requiredGrants?.ToList(); int lastProgressReportSeconds = 0; logger.LogInformation( @@ -129,6 +142,21 @@ public static async Task PollAdminConsentAsync( if (spId != null) { + if (resolvedRequirements is { Count: > 0 }) + { + for (var i = 0; i < resolvedRequirements.Count; i++) + { + var requirement = resolvedRequirements[i]; + if (!string.IsNullOrWhiteSpace(requirement.ResourceSpObjectId)) + continue; + + var resourceSpId = await LookupSpObjectIdByAppIdAsync( + executor, requirement.ResourceAppId, baseUrl, ct); + if (!string.IsNullOrWhiteSpace(resourceSpId)) + resolvedRequirements[i] = requirement with { ResourceSpObjectId = resourceSpId }; + } + } + var grants = await executor.ExecuteAsync("az", $"rest --method GET --url \"{baseUrl}/v1.0/oauth2PermissionGrants?$filter=clientId eq '{spId}'\"", captureOutput: true, suppressErrorLogging: true, cancellationToken: ct); @@ -139,7 +167,10 @@ public static async Task PollAdminConsentAsync( { using var gdoc = JsonDocument.Parse(grants.StandardOutput); var arr = gdoc.RootElement.GetProperty("value"); - if (arr.GetArrayLength() > 0) + var consentComplete = resolvedRequirements is { Count: > 0 } + ? GrantsSatisfyRequirements(arr, resolvedRequirements) + : arr.GetArrayLength() > 0; + if (consentComplete) { logger.LogInformation("Consent granted ({ScopeDescriptor}).", scopeDescriptor); return true; @@ -194,7 +225,8 @@ public static async Task PollAdminConsentAsync( int timeoutSeconds, int intervalSeconds, CancellationToken ct, - IEnumerable? permScopes = null) + IEnumerable? permScopes = null, + IReadOnlyCollection? requiredGrants = null) { if (BypassConsentChecksForTests) { @@ -237,11 +269,16 @@ public static async Task PollAdminConsentAsync( permScopes); if (grantsDoc != null && - grantsDoc.RootElement.TryGetProperty("value", out var arr) && - arr.GetArrayLength() > 0) + grantsDoc.RootElement.TryGetProperty("value", out var arr)) { - logger.LogInformation("Consent granted ({ScopeDescriptor}).", scopeDescriptor); - return ConsentPollResult.Verified; + var consentComplete = requiredGrants is { Count: > 0 } + ? GrantsSatisfyRequirements(arr, requiredGrants) + : arr.GetArrayLength() > 0; + if (consentComplete) + { + logger.LogInformation("Consent granted ({ScopeDescriptor}).", scopeDescriptor); + return ConsentPollResult.Verified; + } } logger.LogDebug("No consent grants found for blueprint SP {ClientSpId} yet.", clientSpId); @@ -261,6 +298,54 @@ public static async Task PollAdminConsentAsync( } } + internal static bool GrantsSatisfyRequirements( + JsonElement grants, + IReadOnlyCollection requirements) + { + if (requirements.Count == 0) + return grants.GetArrayLength() > 0; + + var scopesByResourceSp = new Dictionary>(StringComparer.OrdinalIgnoreCase); + foreach (var grant in grants.EnumerateArray()) + { + if (!grant.TryGetProperty("consentType", out var consentType) + || !string.Equals(consentType.GetString(), "AllPrincipals", StringComparison.OrdinalIgnoreCase) + || !grant.TryGetProperty("resourceId", out var resourceIdElement) + || string.IsNullOrWhiteSpace(resourceIdElement.GetString())) + { + continue; + } + + var resourceId = resourceIdElement.GetString()!; + if (!scopesByResourceSp.TryGetValue(resourceId, out var grantedScopes)) + { + grantedScopes = new HashSet(StringComparer.OrdinalIgnoreCase); + scopesByResourceSp[resourceId] = grantedScopes; + } + + if (!grant.TryGetProperty("scope", out var scopeElement)) + continue; + + foreach (var scope in (scopeElement.GetString() ?? string.Empty) + .Split(' ', StringSplitOptions.RemoveEmptyEntries)) + { + grantedScopes.Add(scope); + } + } + + foreach (var requirement in requirements) + { + if (string.IsNullOrWhiteSpace(requirement.ResourceSpObjectId) + || !scopesByResourceSp.TryGetValue(requirement.ResourceSpObjectId, out var grantedScopes) + || !requirement.Scopes.All(grantedScopes.Contains)) + { + return false; + } + } + + return true; + } + /// /// Checks if admin consent already exists for specified scopes between client and resource service principals. /// Returns true if ALL required scopes are present in existing oauth2PermissionGrants. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/design.md b/src/Microsoft.Agents.A365.DevTools.Cli/design.md index 28141334..b994a1c2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/design.md +++ b/src/Microsoft.Agents.A365.DevTools.Cli/design.md @@ -458,8 +458,8 @@ return await new CommandLineBuilder(rootCommand) | Agent Type | Flag | What it creates | |---|---|---| -| **AI Teammate agent** | `--aiteammate` | Azure infra + Agent Blueprint + batch permissions (5 resources) + messaging endpoint | -| **Custom Engine Agent / Blueprint** (default) | omit `--aiteammate` | Agent Blueprint + batch permissions (Graph + A365 Tools only) + Agent Instance (Graph API) | +| **AI Teammate agent** | `--aiteammate` | Azure infra + Agent Blueprint + batch permissions (6 resources) + messaging endpoint | +| **Custom Engine Agent / Blueprint** (default) | omit `--aiteammate` | Agent Blueprint + baseline permissions (4 resources) + Agent Identity + Agent Registration | Non-DW blueprint agents do not use Azure Bot Service, so there is no infrastructure step, no manifest zip, and no messaging endpoint registration. The final step is `POST /beta/agentRegistry/agentInstances` instead. @@ -475,14 +475,14 @@ AllSubcommand.ExecuteAsync ├── DW path: │ ExecuteInfrastructureStepAsync(ctx) ← DW only │ ExecuteBlueprintStepAsync(ctx) ← shared - │ ExecuteBatchPermissionsStepAsync(ctx, dwSpecs) ← shared (5 resources) + │ ExecuteBatchPermissionsStepAsync(ctx, dwSpecs) ← shared (6 resources) │ ExecuteMessagingEndpointStepAsync(ctx) ← DW only │ └── Non-DW path: NonDwBlueprintSetupOrchestrator.ExecuteAsync(ctx) ExecuteBlueprintStepAsync(ctx) ← reuses DW step - ExecuteBatchPermissionsStepAsync(ctx, nonDwSpecs) ← reuses, 2 resources only - RegisterAgentInstanceAsync(...) ← non-DW final step + ExecuteBatchPermissionsStepAsync(ctx, nonDwSpecs) ← reuses, 4 baseline resources + Create Agent Identity + Agent Registration ← non-DW final steps ``` `SetupContext.Config` is intentionally mutable — the blueprint step reloads configuration from disk after writing `AgentBlueprintId`, and the updated instance must be visible to all subsequent steps. @@ -493,10 +493,11 @@ The non-DW spec list is a strict subset of the DW list: | Resource | DW | Non-DW Blueprint | |---|---|---| -| Microsoft Graph (delegated) | ✓ | — | -| Agent 365 Tools (delegated) | ✓ | — | -| Messaging Bot API | ✓ | — | -| Observability API | ✓ | — | +| Microsoft Graph (delegated) | ✓ | ✓ | +| Agent 365 Tools (delegated) | ✓ | ✓ | +| Messaging Bot API (with `--m365`) | ✓ | ✓ | +| Observability API | ✓ | — by default | +| Defender API | delegated + application | follows `--authmode` | | Power Platform API | ✓ | ✓ | --- diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs index bee6b981..4ba08fc3 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs @@ -292,6 +292,30 @@ public void BotSubcommand_ShouldHaveDryRunOption() dryRunOption!.Aliases.Should().Contain("--dry-run"); } + [Fact] + public async Task BotDryRun_WithoutConfig_MentionsDefender() + { + var root = new RootCommand(); + root.AddCommand(PermissionsSubcommand.CreateCommand( + _mockLogger, + _mockAuthValidator, + _mockConfigService, + _mockExecutor, + _mockGraphApiService, + _mockBlueprintService, + _mockConfirmationProvider)); + + var result = await root.InvokeAsync("permissions bot --dry-run"); + + result.Should().Be(0, + because: "the no-config permissions bot dry run must provide a usable generic preview"); + var messages = _mockLogger.ReceivedCalls() + .Select(call => call.GetArguments()[2]?.ToString() ?? string.Empty); + messages.Should().Contain( + message => message.Contains("Defender", StringComparison.OrdinalIgnoreCase), + because: "permissions bot configures Defender and the no-config preview must not omit that user-visible work"); + } + [Fact] public void BotSubcommand_DescriptionShouldMentionPrerequisites() { @@ -1031,4 +1055,3 @@ public async Task ConfigureBotPermissionsAsync_AdminPath_WhenS2SFailsWithExterna #endregion } - diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs index 983b5e14..8670af1e 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs @@ -961,6 +961,8 @@ public void DisplaySetupSummary_BlueprintOnly_EmitsPermissionsNextSteps() because: "the blueprint summary must point to the remaining MCP permissions step"); logger.AllOutput.Should().Contain("a365 setup permissions bot", because: "the blueprint summary must point to the bot/observability permissions step"); + logger.AllOutput.Should().Contain("Defender", + because: "the permissions bot next step configures Defender and must disclose that work"); } [Fact] diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AdminConsentHelperTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AdminConsentHelperTests.cs index c4eff2dc..5f9a89d7 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AdminConsentHelperTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AdminConsentHelperTests.cs @@ -41,6 +41,164 @@ await executor.Received(2).ExecuteAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } + [Fact] + public async Task PollAdminConsentAsync_DoesNotCompleteOnUnrelatedExistingGrant() + { + var executor = Substitute.For(Substitute.For>()); + var logger = Substitute.For(); + executor.ExecuteAsync( + "az", + Arg.Is(args => args.Contains("servicePrincipals")), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(Task.FromResult(new CommandResult + { + ExitCode = 0, + StandardOutput = """{"value":[{"id":"blueprint-sp"}]}""", + })); + + var unrelatedGrant = """ + { + "value": [ + { + "resourceId": "graph-sp", + "consentType": "AllPrincipals", + "scope": "User.Read" + } + ] + } + """; + var completeGrantSet = """ + { + "value": [ + { + "resourceId": "graph-sp", + "consentType": "AllPrincipals", + "scope": "User.Read" + }, + { + "resourceId": "defender-sp", + "consentType": "AllPrincipals", + "scope": "RealtimeProtection.Evaluate.All" + } + ] + } + """; + executor.ExecuteAsync( + "az", + Arg.Is(args => args.Contains("oauth2PermissionGrants")), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns( + Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = unrelatedGrant }), + Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = completeGrantSet })); + + var requirements = new[] + { + new AdminConsentRequirement( + "Defender API", + "86a21212-634e-4553-b3d6-e477e4c9d9ec", + ["RealtimeProtection.Evaluate.All"], + "defender-sp"), + }; + + var result = await AdminConsentHelper.PollAdminConsentAsync( + executor, + logger, + "11111111-1111-1111-1111-111111111111", + "All permissions", + timeoutSeconds: 10, + intervalSeconds: 0, + CancellationToken.None, + requiredGrants: requirements); + + result.Should().BeTrue( + because: "polling must wait until the requested Defender grant appears instead of completing on the pre-existing Graph grant"); + await executor.Received(2).ExecuteAsync( + "az", + Arg.Is(args => args.Contains("oauth2PermissionGrants")), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()); + } + + [Fact] + public void GrantsSatisfyRequirements_RejectsPrincipalAndUnrelatedGrants() + { + using var grants = JsonDocument.Parse(""" + { + "value": [ + { + "resourceId": "graph-sp", + "consentType": "AllPrincipals", + "scope": "RealtimeProtection.Evaluate.All" + }, + { + "resourceId": "defender-sp", + "consentType": "Principal", + "scope": "RealtimeProtection.Evaluate.All" + } + ] + } + """); + var requirements = new[] + { + new AdminConsentRequirement( + "Defender API", + "86a21212-634e-4553-b3d6-e477e4c9d9ec", + ["RealtimeProtection.Evaluate.All"], + "defender-sp"), + }; + + var result = AdminConsentHelper.GrantsSatisfyRequirements( + grants.RootElement.GetProperty("value"), + requirements); + + result.Should().BeFalse( + because: "only an AllPrincipals grant on the requested Defender resource may complete tenant-wide consent polling"); + } + + [Fact] + public void GrantsSatisfyRequirements_AggregatesScopesAcrossGrantRows() + { + using var grants = JsonDocument.Parse(""" + { + "value": [ + { + "resourceId": "resource-sp", + "consentType": "AllPrincipals", + "scope": "Scope.One" + }, + { + "resourceId": "resource-sp", + "consentType": "AllPrincipals", + "scope": "Scope.Two" + } + ] + } + """); + var requirements = new[] + { + new AdminConsentRequirement( + "Contoso API", + "22222222-2222-2222-2222-222222222222", + ["Scope.One", "Scope.Two"], + "resource-sp"), + }; + + var result = AdminConsentHelper.GrantsSatisfyRequirements( + grants.RootElement.GetProperty("value"), + requirements); + + result.Should().BeTrue( + because: "incremental Entra consent can split required scopes across multiple AllPrincipals grant rows"); + } + [Fact] public async Task PollAdminConsentAsync_PropagatesCancellation_WhenTokenCanceled() { From 4f7a738284f3a6a8415ec832dcfcb7e6bc922b84 Mon Sep 17 00:00:00 2001 From: slreznit Date: Fri, 9 Oct 2026 09:16:23 +0300 Subject: [PATCH 13/14] Normalize setup auth mode values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- .../Commands/SetupSubcommands/SetupContext.cs | 2 +- .../Commands/AllSubcommandTests.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs index 06c15e28..d1815db1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs @@ -179,7 +179,7 @@ public SetupContext( AgentInstanceOnly = agentInstanceOnly; IsBootstrap = isBootstrap; IsM365 = isM365; - AuthMode = string.IsNullOrWhiteSpace(authMode) ? null : authMode.ToLowerInvariant(); + AuthMode = string.IsNullOrWhiteSpace(authMode) ? null : authMode.Trim().ToLowerInvariant(); MessagingEndpointOverride = string.IsNullOrWhiteSpace(messagingEndpointOverride) ? null : messagingEndpointOverride.Trim(); SkipSpProvisioning = skipSpProvisioning; NonInteractive = nonInteractive; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index c709baed..c5c0e8e9 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs @@ -428,8 +428,11 @@ public async Task BuildPermissionSpecsAsync_StampsObservabilityApiUnlessSkipped( [Theory] [InlineData(null, true, false)] [InlineData("obo", true, false)] + [InlineData(" OBO ", true, false)] [InlineData("s2s", false, true)] + [InlineData(" S2S ", false, true)] [InlineData("both", true, true)] + [InlineData(" BOTH ", true, true)] public async Task BuildPermissionSpecsAsync_NonDw_DefenderPermissionsMatchAuthMode( string? authMode, bool expectDelegated, From 05b05a4c3ea76ddb9c6a2f68e8d674790ee39dd4 Mon Sep 17 00:00:00 2001 From: slreznit Date: Fri, 9 Oct 2026 23:52:18 +0300 Subject: [PATCH 14/14] Recover resource SPs before permission grants Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1657d63e-2cae-48fa-ba64-834616e6b2c7 --- .../BatchPermissionsOrchestrator.cs | 69 +++-- .../SetupSubcommands/PermissionsSubcommand.cs | 2 +- ...chPermissionsOrchestratorMissingSpTests.cs | 29 ++- .../BatchPermissionsOrchestratorTests.cs | 243 +++++++++++++++++- .../Commands/PermissionsSubcommandTests.cs | 2 + 5 files changed, 310 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index 4175f9a8..0b376696 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -137,6 +137,27 @@ internal static class BatchPermissionsOrchestrator : Models.RoleCheckResult.DoesNotHaveRole; var isGlobalAdmin = adminCheck == Models.RoleCheckResult.HasRole; + // Recover missing resource service principals before inheritance and app-role grants. + // Successful az provisioning updates both the app-id set and object-id map so every + // downstream phase can use the newly created resource in this same run. + if (phase1Result != null) + { + var resolvedSpObjectIds = phase1Result.ResourceSpObjectIds + .ToDictionary(pair => pair.Key, pair => pair.Value, StringComparer.OrdinalIgnoreCase); + var resolvedSpAppIds = resolvedSpObjectIds.Keys.ToHashSet(StringComparer.OrdinalIgnoreCase); + var missingSpecs = FindMissingResourceSpSpecs(specs, resolvedSpAppIds); + await EnsureMissingResourceSpsAsync( + graph, tenantId, blueprintAppId, missingSpecs, resolvedSpAppIds, permScopes, + skipSpProvisioning, logger, setupResults, ct, + commandExecutor: commandExecutor, + confirmationProvider: confirmationProvider, + knownMcpAudienceAppIds: knownMcpAudienceAppIds, + resolvedSpObjectIds: resolvedSpObjectIds); + phase1Result = new BlueprintPermissionsResult( + phase1Result.BlueprintSpObjectId, + resolvedSpObjectIds); + } + // --- Phase 2a: Inheritable permissions (Agent ID Admin or GA) --- // --- Phase 2b: OAuth2 grants (Global Administrator only) --- logger.LogInformation("Configuring inheritable permissions..."); @@ -282,7 +303,7 @@ internal static class BatchPermissionsOrchestrator // --- Admin consent --- var (consentGranted, consentUrl) = await GrantAdminConsentAsync( - graph, config, blueprintAppId, tenantId, specs, phase1Result, permScopes, logger, setupResults, ct, commandExecutor, adminCheck, confirmationProvider, skipSpProvisioning, knownMcpAudienceAppIds); + graph, config, blueprintAppId, tenantId, specs, phase1Result, permScopes, logger, setupResults, ct, commandExecutor, adminCheck, confirmationProvider, knownMcpAudienceAppIds); // Update in-memory ResourceConsents only when consent was directly verified (consentUrl == null). // AssumedComplete returns a non-null consentUrl — do not persist in that case since the grant @@ -610,7 +631,6 @@ private static void SetPendingBlueprintAppRoleSpecs(SetupResults setupResults, I CommandExecutor? commandExecutor = null, Models.RoleCheckResult adminCheck = Models.RoleCheckResult.Unknown, IConfirmationProvider? confirmationProvider = null, - bool skipSpProvisioning = false, IReadOnlyCollection? knownMcpAudienceAppIds = null) { // Hold onto the unfiltered spec list so the PowerShell consent fallback can attempt @@ -664,34 +684,14 @@ private static void SetPendingBlueprintAppRoleSpecs(SetupResults setupResults, I ? map.Keys.ToHashSet(StringComparer.OrdinalIgnoreCase) : new HashSet(StringComparer.OrdinalIgnoreCase); - // Find specs whose SP couldn't be resolved in Phase 1 and try to provision them in - // place by shelling out to 'az ad sp create --id {appId}' against the operator's - // existing az login (the per-app admin-consent URL pattern was removed because - // first-party MCP audiences fail it with AADSTS65003 — token-to-self consent). - // EnsureMissingResourceSpsAsync mutates the resolvedSpAppIds set on success and - // records MissingSpActions for the rest so the Action Required block renders the - // az command and, for delegated specs, a per-SP consent URL keyed to the blueprint. - // Skips entirely when skipSpProvisioning is true (flag or - // auto-detected from stdin) or when there is nothing missing. See helper for the - // full state machine. - if (resolvedSpAppIds.Count > 0) - { - var missingSpecs = FindMissingResourceSpSpecs(specs, resolvedSpAppIds); - await EnsureMissingResourceSpsAsync( - graph, tenantId, blueprintAppId, missingSpecs, resolvedSpAppIds, permScopes, - skipSpProvisioning, logger, setupResults, ct, - commandExecutor: commandExecutor, - confirmationProvider: confirmationProvider, - knownMcpAudienceAppIds: knownMcpAudienceAppIds); - } - // Apply the SP-resolution filter only when Phase 1 produced any results. When // Phase 1 returned no resolved SPs at all (auth failure earlier), keep the legacy // behavior of including every spec — that surfaces the auth failure path rather // than silently dropping every scope here. - var specsForUrl = resolvedSpAppIds.Count > 0 + var specsForUrl = phase1Result != null ? specs.Where(s => resolvedSpAppIds.Contains(s.ResourceAppId)).ToList() : specs.ToList(); + var hasUnresolvedDelegatedSpecs = HasUnresolvedDelegatedSpecs(specs, resolvedSpAppIds); var consentRequirements = specsForUrl .Where(spec => spec.Scopes is { Length: > 0 }) .Select(spec => @@ -725,7 +725,7 @@ await EnsureMissingResourceSpsAsync( // the Action Required block from setupResults if S2S work remains. if (consentUrl == null) { - return (true, null); + return (granted: !hasUnresolvedDelegatedSpecs, consentUrl: null); } // Section header — mirrors PerformS2SGrantsAsync's "Configuring S2S app role assignments..." @@ -805,7 +805,7 @@ await EnsureMissingResourceSpsAsync( } } - if (allConsented) + if (allConsented && !hasUnresolvedDelegatedSpecs) { using (logger.Indent()) logger.LogInformation("Delegated admin consent already granted for all required scopes"); @@ -813,6 +813,8 @@ await EnsureMissingResourceSpsAsync( setupResults.TenantWideConsentAlreadyExisted = true; return (true, null); } + if (allConsented) + return (false, consentUrl); } } @@ -961,6 +963,9 @@ await EnsureMissingResourceSpsAsync( // Return URL when either polling failed outright OR consent was assumed-complete but not // verified. Caller uses (consentGranted && consentUrl == null) as the 'safe to persist' gate. + if (hasUnresolvedDelegatedSpecs) + return (false, consentUrl); + return (consentGranted, consentVerified ? null : consentUrl); } @@ -973,6 +978,13 @@ internal static List FindMissingResourceSpSpecs( && !resolvedSpAppIds.Contains(spec.ResourceAppId)) .ToList(); + internal static bool HasUnresolvedDelegatedSpecs( + IReadOnlyList specs, + IReadOnlySet resolvedSpAppIds) => + specs.Any(spec => + spec.Scopes is { Length: > 0 } + && !resolvedSpAppIds.Contains(spec.ResourceAppId)); + /// /// Updates config.ResourceConsents in-memory for each spec based on phase results. /// The caller is responsible for persisting the config via configService.SaveStateAsync. @@ -1093,7 +1105,8 @@ internal static async Task EnsureMissingResourceSpsAsync( CancellationToken ct, CommandExecutor? commandExecutor = null, IConfirmationProvider? confirmationProvider = null, - IReadOnlyCollection? knownMcpAudienceAppIds = null) + IReadOnlyCollection? knownMcpAudienceAppIds = null, + IDictionary? resolvedSpObjectIds = null) { if (missingSpecs.Count == 0) return; @@ -1115,6 +1128,7 @@ internal static async Task EnsureMissingResourceSpsAsync( "Resource '{Name}' ({AppId}): service principal found in tenant — no provisioning needed.", spec.ResourceName, spec.ResourceAppId); resolvedSpAppIds.Add(spec.ResourceAppId); + resolvedSpObjectIds?[spec.ResourceAppId] = spId; } else { @@ -1231,6 +1245,7 @@ internal static async Task EnsureMissingResourceSpsAsync( { logger.LogInformation("Done. Service principal created for '{Name}' (id: {SpId}).", spec.ResourceName, newSpId); resolvedSpAppIds.Add(spec.ResourceAppId); + resolvedSpObjectIds?[spec.ResourceAppId] = newSpId; } else { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index a4ff7b60..8ae371ec 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs @@ -254,7 +254,7 @@ private static Command CreateBotSubcommand( IBootstrapConfigResolver? resolver = null) { var command = new Command("bot", - "Configure Messaging Bot API OAuth2 grants and inheritable permissions\n" + + "Configure Messaging Bot, Observability, Defender, and Power Platform API grants and inheritable permissions\n" + "Required role: Agent ID Developer; Global Administrator for tenant-wide OAuth2 consent\n" + "(non-admins receive a unified /v2.0/adminconsent URL to forward to a Global Administrator).\n\n" + "Prerequisites: Blueprint and MCP permissions (run 'a365 setup permissions mcp' first)\n" + diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs index 18ed5c47..58763ff8 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs @@ -82,6 +82,7 @@ public async Task PreflightFindsSp_AddsToResolvedSetAndDoesNotRunAz() .Returns(Task.FromResult(MailMcpSpObjectId)); var resolvedSpAppIds = new HashSet(StringComparer.OrdinalIgnoreCase); + var resolvedSpObjectIds = new Dictionary(StringComparer.OrdinalIgnoreCase); var setupResults = new SetupResults(); var missing = new[] { @@ -95,10 +96,13 @@ await BatchPermissionsOrchestrator.EnsureMissingResourceSpsAsync( _logger, setupResults: setupResults, ct: CancellationToken.None, - commandExecutor: _executor); + commandExecutor: _executor, + resolvedSpObjectIds: resolvedSpObjectIds); resolvedSpAppIds.Should().Contain(MailMcpAppId, because: "the pre-flight Graph lookup found the SP — the operator must have consented to it between Phase 1 and now, so the helper records it and skips the az shell-out"); + resolvedSpObjectIds.Should().ContainKey(MailMcpAppId).WhoseValue.Should().Be(MailMcpSpObjectId, + because: "downstream consent and grant phases need the recovered resource service-principal object ID in the same run"); setupResults.MissingSpActions.Should().BeEmpty( because: "the resource was successfully resolved without any operator intervention — no Action Required entry needed"); await _executor.DidNotReceive().ExecuteAsync( @@ -211,6 +215,23 @@ public void FindMissingResourceSpSpecs_IncludesApplicationOnlyDefender() because: "S2S application-role assignment cannot succeed until the Defender service principal exists"); } + [Fact] + public void HasUnresolvedDelegatedSpecs_DetectsMissingDefenderConsent() + { + var defenderSpec = new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + [ConfigConstants.DefenderApiRealtimeProtectionScope], + SetInheritable: true); + + var hasUnresolved = BatchPermissionsOrchestrator.HasUnresolvedDelegatedSpecs( + [defenderSpec], + new HashSet(StringComparer.OrdinalIgnoreCase)); + + hasUnresolved.Should().BeTrue( + because: "aggregate consent must remain incomplete until the Defender resource SP can receive its delegated grant"); + } + [Fact] public async Task NullCommandExecutor_FallsBackToWarningPathAndDoesNotPrompt() { @@ -317,6 +338,7 @@ public async Task ConfirmationProviderReturnsTrue_AzExitsZeroWithSpJson_AddsAppI accepting.ConfirmAsync(Arg.Any()).Returns(Task.FromResult(true)); var resolvedSpAppIds = new HashSet(StringComparer.OrdinalIgnoreCase); + var resolvedSpObjectIds = new Dictionary(StringComparer.OrdinalIgnoreCase); var setupResults = new SetupResults(); var missing = new[] { @@ -331,10 +353,13 @@ await BatchPermissionsOrchestrator.EnsureMissingResourceSpsAsync( setupResults: setupResults, ct: CancellationToken.None, commandExecutor: _executor, - confirmationProvider: accepting); + confirmationProvider: accepting, + resolvedSpObjectIds: resolvedSpObjectIds); resolvedSpAppIds.Should().Contain(TeamsMcpAppId, because: "az returned the SP JSON with an id — that is authoritative evidence the SP exists, so the caller's URL build must include this resource"); + resolvedSpObjectIds.Should().ContainKey(TeamsMcpAppId).WhoseValue.Should().Be("d42a47bf-9727-444c-ae57-17bd588613cd", + because: "the same-run consent precheck and app-role assignment need the object ID returned by az"); setupResults.MissingSpActions.Should().BeEmpty( because: "the SP was provisioned successfully — no recovery steps belong in Action Required for this resource"); // The helper trusts az output and does NOT issue a follow-up Graph lookup for the diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs index c4df3dc2..90281e38 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs @@ -189,14 +189,29 @@ public async Task ConfigureAllPermissions_NonAdmin_BuildsUnifiedConsentUrlCoveri _graph.IsCurrentUserAdminAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(RoleCheckResult.DoesNotHaveRole)); - // Prevent real network calls: Phase 1 resource SP resolution and Phase 2a inheritable - // permission writes must not reach Azure endpoints in CI. Return null SPs (not found) - // and simulate an insufficient-privileges failure so both phases skip cleanly without - // making any real HTTP requests. + // Prevent real network calls while preserving the production invariant that consent URLs + // contain only resources whose service principals were resolved. _graph.EnsureServicePrincipalForAppIdAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any?>(), Arg.Any()) - .Returns(Task.FromResult(null)); + .Returns(call => Task.FromResult(call.ArgAt(1) switch + { + AuthenticationConstants.MicrosoftGraphResourceAppId => "graph-sp", + ConfigConstants.MessagingBotApiAppId => "bot-sp", + ConfigConstants.ObservabilityApiAppId => "observability-sp", + _ => null, + })); + _graph.GetAvailableScopeNamesAsync( + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(call => Task.FromResult(call.ArgAt(1) switch + { + "graph-sp" => new HashSet(["Mail.ReadWrite"], StringComparer.OrdinalIgnoreCase), + "bot-sp" => new HashSet(["BotApi.Scope"], StringComparer.OrdinalIgnoreCase), + "observability-sp" => new HashSet([ConfigConstants.ObservabilityApiOtelWriteScope], StringComparer.OrdinalIgnoreCase), + _ => [], + })); _blueprintService.SetInheritablePermissionsAsync( Arg.Any(), Arg.Any(), Arg.Any(), @@ -322,6 +337,224 @@ private static ResourcePermissionSpec[] S2SSpec(bool includeDelegatedScope = tru AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope }) ]; + [Fact] + public async Task ConfigureAllPermissions_WhenApplicationOnlySpRecovered_GrantsRoleInSameRun() + { + var previousBypass = BatchPermissionsOrchestrator.BypassSpProvisioningForTests; + BatchPermissionsOrchestrator.BypassSpProvisioningForTests = false; + try + { + _graph.GraphGetAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult(JsonDocument.Parse("{\"id\":\"user-id\"}"))); + _graph.IsCurrentUserAdminAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(RoleCheckResult.HasRole)); + _graph.EnsureServicePrincipalForAppIdAsync( + Arg.Any(), + ConfigConstants.DefenderApiAppId, + Arg.Any(), + Arg.Any?>(), + Arg.Any()) + .Returns(Task.FromResult(null)); + _graph.LookupServicePrincipalByAppIdAsync( + Arg.Any(), + ConfigConstants.DefenderApiAppId, + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult(null)); + + _executor.ExecuteAsync( + "az", + Arg.Is(args => args.Contains($"ad sp create --id {ConfigConstants.DefenderApiAppId}")), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(Task.FromResult(new CommandResult + { + ExitCode = 0, + StandardOutput = $"{{\"id\":\"{S2SResourceSpId}\"}}", + })); + + _blueprintService.SetInheritablePermissionsAsync( + Arg.Any(), + Arg.Any(), + ConfigConstants.DefenderApiAppId, + Arg.Any>(), + Arg.Any?>(), + Arg.Any()) + .Returns(Task.FromResult((ok: true, alreadyExists: false, error: (string?)null))); + _blueprintService.VerifyInheritablePermissionsAsync( + Arg.Any(), + Arg.Any(), + ConfigConstants.DefenderApiAppId, + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult((exists: true, scopesAllAllowed: true, rolesAllAllowed: true, error: (string?)null))); + _blueprintService.GrantAppRoleAssignmentAsync( + Arg.Any(), + S2SBlueprintSpObjectId, + ConfigConstants.DefenderApiAppId, + Arg.Is(roles => roles.SequenceEqual(new[] { ConfigConstants.DefenderApiRealtimeProtectionScope })), + Arg.Any(), + Arg.Any()) + .Returns(Task.FromResult(new AppRoleGrantResult(AllSucceeded: true, AllAlreadyAssigned: false))); + + var confirmationProvider = Substitute.For(); + confirmationProvider.ConfirmAsync(Arg.Any()).Returns(Task.FromResult(true)); + var setupResults = new SetupResults(); + var specs = new[] + { + new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + [], + SetInheritable: true, + AppRoleScopes: [ConfigConstants.DefenderApiRealtimeProtectionScope]), + }; + + await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( + _graph, + _blueprintService, + new Agent365Config { TenantId = S2STenantId, AgentBlueprintId = S2SBlueprintAppId }, + S2SBlueprintAppId, + S2STenantId, + specs, + _logger, + setupResults, + ct: default, + knownBlueprintSpObjectId: S2SBlueprintSpObjectId, + confirmationProvider: confirmationProvider, + commandExecutor: _executor); + + await _blueprintService.Received(1).GrantAppRoleAssignmentAsync( + S2STenantId, + S2SBlueprintSpObjectId, + ConfigConstants.DefenderApiAppId, + Arg.Is(roles => roles.SequenceEqual(new[] { ConfigConstants.DefenderApiRealtimeProtectionScope })), + Arg.Any(), + Arg.Any()); + setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Granted, + because: "successful missing-SP recovery must let the same setup run complete the Defender app-role assignment"); + setupResults.MissingSpActions.Should().BeEmpty( + because: "successful provisioning and assignment leave no manual recovery action"); + } + finally + { + BatchPermissionsOrchestrator.BypassSpProvisioningForTests = previousBypass; + } + } + + [Fact] + public async Task ConfigureAllPermissions_WhenDelegatedSpUnresolved_DoesNotPersistAggregateConsent() + { + var previousBypass = BatchPermissionsOrchestrator.BypassSpProvisioningForTests; + BatchPermissionsOrchestrator.BypassSpProvisioningForTests = false; + try + { + const string graphSpId = "00000000-0000-0000-0000-000000000006"; + _graph.GraphGetAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult(JsonDocument.Parse("{\"id\":\"user-id\"}"))); + _graph.IsCurrentUserAdminAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(RoleCheckResult.DoesNotHaveRole)); + _graph.EnsureServicePrincipalForAppIdAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>(), + Arg.Any()) + .Returns(call => Task.FromResult( + call.ArgAt(1) == AuthenticationConstants.MicrosoftGraphResourceAppId + ? graphSpId + : null)); + _graph.LookupServicePrincipalByAppIdAsync( + Arg.Any(), + ConfigConstants.DefenderApiAppId, + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult(null)); + _graph.GetAvailableScopeNamesAsync( + Arg.Any(), + graphSpId, + Arg.Any()) + .Returns(new HashSet(["Mail.Read"], StringComparer.OrdinalIgnoreCase)); + + _blueprintService.SetInheritablePermissionsAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any?>(), + Arg.Any()) + .Returns(Task.FromResult((ok: true, alreadyExists: false, error: (string?)null))); + _blueprintService.VerifyInheritablePermissionsAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(Task.FromResult((exists: true, scopesAllAllowed: true, rolesAllAllowed: true, error: (string?)null))); + + var config = new Agent365Config + { + TenantId = S2STenantId, + AgentBlueprintId = S2SBlueprintAppId, + }; + var setupResults = new SetupResults(); + var specs = new[] + { + new ResourcePermissionSpec( + AuthenticationConstants.MicrosoftGraphResourceAppId, + "Microsoft Graph", + ["Mail.Read"], + SetInheritable: true), + new ResourcePermissionSpec( + ConfigConstants.DefenderApiAppId, + "Defender API", + [ConfigConstants.DefenderApiRealtimeProtectionScope], + SetInheritable: true), + }; + + var result = await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( + _graph, + _blueprintService, + config, + S2SBlueprintAppId, + S2STenantId, + specs, + _logger, + setupResults, + ct: default, + knownBlueprintSpObjectId: S2SBlueprintSpObjectId, + commandExecutor: _executor, + skipSpProvisioning: true); + + result.adminConsentGranted.Should().BeFalse( + because: "an unresolved Defender resource cannot be treated as fully consented merely because older Graph grants already exist"); + result.adminConsentUrl.Should().NotBeNull( + because: "the filtered handoff URL must remain available while Defender recovery is pending"); + result.adminConsentUrl.Should().NotContain(ConfigConstants.DefenderApiAppId, + because: "the filtered URL must not reintroduce the missing Defender resource and poison consent for resolved resources"); + config.ResourceConsents.Should().NotContain( + consent => consent.ResourceAppId == ConfigConstants.DefenderApiAppId, + because: "Defender consent must not be persisted until its resource SP and delegated grant are directly verified"); + setupResults.MissingSpActions.Should().ContainSingle( + action => action.ResourceAppId == ConfigConstants.DefenderApiAppId, + because: "the operator still needs the explicit Defender service-principal recovery action"); + } + finally + { + BatchPermissionsOrchestrator.BypassSpProvisioningForTests = previousBypass; + } + } + /// /// When the programmatic Graph API path for S2S fails (e.g. token lacks /// AppRoleAssignment.ReadWrite.All even for a GA) and the az rest fallback completes diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs index 4ba08fc3..9833df57 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PermissionsSubcommandTests.cs @@ -331,6 +331,8 @@ public void BotSubcommand_DescriptionShouldMentionPrerequisites() // Assert botSubcommand.Description.Should().Contain("Prerequisites"); + botSubcommand.Description.Should().Contain("Defender", + because: "the command description must disclose every fixed platform API that permissions bot configures"); } [Fact]