diff --git a/CHANGELOG.md b/CHANGELOG.md index 9367e48d..4163fe75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,13 @@ Agents that export telemetry through the delegated (OBO) route need `Agent365.Ob Blueprint agents that export telemetry through the app-only S2S endpoint don't need these permissions, and `a365 setup all` no longer requests them for blueprint agents (#501). +#### Existing agents: grant Defender API permissions + +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` 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/QueryEntraCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs index 6a5b0cb7..e77d7011 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs @@ -670,6 +670,7 @@ private static Command CreateInstanceScopesSubcommand( null or "" => null, AuthenticationConstants.MicrosoftGraphResourceAppId => "Microsoft Graph", ConfigConstants.MessagingBotApiAppId => "Messaging Bot 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/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..0b376696 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); } @@ -135,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..."); @@ -280,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 @@ -415,12 +438,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) @@ -604,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 @@ -658,36 +684,27 @@ 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 - // 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 - // 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(); - 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 => + { + 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 @@ -708,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..." @@ -788,7 +805,7 @@ await EnsureMissingResourceSpsAsync( } } - if (allConsented) + if (allConsented && !hasUnresolvedDelegatedSpecs) { using (logger.Indent()) logger.LogInformation("Delegated admin consent already granted for all required scopes"); @@ -796,6 +813,8 @@ await EnsureMissingResourceSpsAsync( setupResults.TenantWideConsentAlreadyExisted = true; return (true, null); } + if (allConsented) + return (false, consentUrl); } } @@ -840,7 +859,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 @@ -855,7 +875,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; } @@ -942,9 +963,28 @@ 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); } + 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(); + + 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. @@ -954,7 +994,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); @@ -966,7 +1009,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); @@ -1062,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; @@ -1084,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 { @@ -1200,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 { @@ -1270,11 +1316,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. @@ -1299,14 +1345,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 c249e315..6eb8ac20 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -20,7 +20,7 @@ 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: -/// Power Platform API and custom; Observability API is not requested). MAC reads +/// Defender API, Power Platform API, custom, and optionally Observability API). 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 @@ -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) @@ -126,20 +137,17 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i logger.LogInformation(sub + "create managed identity"); } - // 3. Inheritable Permissions — non-DW spec set (Power Platform API and custom; Observability API is - // not requested) 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(); + // 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. logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for {Resources} (Global Administrator required; consent URL printed if absent)", - skipObservabilityPermissions ? "Power Platform API and custom permissions" : "Observability API, Power Platform API, and custom permissions"); + skipObservabilityPermissions + ? "Defender API, Power Platform API, and custom permissions" + : "Observability API, Defender API, Power Platform API, and custom permissions"); 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 — 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") @@ -282,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. @@ -465,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 }); @@ -722,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/PermissionsSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs index 090be96d..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" + @@ -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; } @@ -306,6 +306,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.DefenderApiRealtimeProtectionScope); logger.LogInformation(" - Power Platform API: Connectivity.Connections.Read"); logger.LogInformation("No changes made. Run without --dry-run to execute."); return; @@ -848,6 +849,7 @@ internal static async Task RemoveStaleCustomPermissionsAsync( envAtgAppId, ConfigConstants.MessagingBotApiAppId, observabilityAppId, + ConfigConstants.DefenderApiAppId, PowerPlatformConstants.PowerPlatformApiResourceAppId, AuthenticationConstants.MicrosoftGraphResourceAppId, }; 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/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/SetupContext.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs index 57632ed7..d1815db1 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). /// @@ -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/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index 741c3f6a..5083d22b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -49,18 +49,18 @@ internal static void PrintDryRunBlueprintReuseRows(ILogger logger, string bluepr /// Returns the fixed-scope ResourcePermissionSpecs for the platform APIs that every /// agent blueprint requires. /// - /// Power Platform API is always included. Observability API, using the app ID for + /// Defender API and Power Platform API are always included. Observability API, using the app ID for /// 's cloud, is included unless - /// is false. Messaging Bot API is - /// included only when is true — non-M365 (blueprint-only) agents - /// have no messaging surface so Bot scopes serve no purpose. + /// is false. 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, string? environment = null, - bool includeObservability = true) + bool includeObservability = true, + DefenderPermissionMode defenderPermissionMode = DefenderPermissionMode.Both) { var specs = new List(); if (isM365) @@ -88,6 +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", + defenderDelegatedScopes, + setInheritable, + AppRoleScopes: defenderAppRoleScopes)); specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -142,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) @@ -181,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()) { @@ -470,8 +484,8 @@ internal static async Task ResolveBootstrapEnvironmentAsync( /// /// 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 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). /// @@ -479,25 +493,37 @@ 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"), ("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; } /// /// 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. /// /// @@ -658,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) @@ -927,7 +952,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:"); @@ -950,7 +975,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( @@ -990,7 +1023,6 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str s2sTargets.Add((true, results.PendingAgentIdentityAppRoleSpecs.Where(spec => spec.AppRoleScopes is { Length: > 0 }).Distinct().ToList())); if (blueprintS2sFailed) s2sTargets.Add((false, results.PendingBlueprintAppRoleSpecs.Where(spec => spec.AppRoleScopes is { Length: > 0 }).Distinct().ToList())); - foreach (var (isAgentIdentityTarget, pendingAppRoleSpecs) in s2sTargets) { actionCount++; @@ -1074,6 +1106,11 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri '{GraphBaseUrl}/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'", resolvedGraphBaseUrl); 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.DefenderApiRealtimeProtectionScope); + logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri '{GraphBaseUrl}/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'", resolvedGraphBaseUrl); + 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); @@ -1122,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. @@ -1133,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); + } } } } @@ -1219,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) @@ -1238,10 +1279,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. @@ -1252,7 +1292,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; @@ -1262,11 +1303,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 @@ -1280,6 +1323,7 @@ internal static List PopulateAdminConsentUrls( ["Agent 365 Tools"] = mcpResourceAppId, ["Messaging Bot API"] = ConfigConstants.MessagingBotApiAppId, ["Observability API"] = observabilityResourceAppId, + ["Defender API"] = ConfigConstants.DefenderApiAppId, ["Power Platform API"] = PowerPlatformConstants.PowerPlatformApiResourceAppId, }; @@ -1387,6 +1431,8 @@ internal static string GetResourceIdentifierUri( return ConfigConstants.MessagingBotApiIdentifierUri; if (ConfigConstants.IsObservabilityApiAppId(resourceAppId)) return ConfigConstants.BuildObservabilityApiIdentifierUri(resourceAppId); + 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 @@ -1445,7 +1491,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 @@ -1465,7 +1512,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)>(); @@ -1534,6 +1582,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 @@ -1562,7 +1613,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) @@ -1598,6 +1650,8 @@ 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($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes, authorityHost); } @@ -1607,11 +1661,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. /// /// @@ -1624,25 +1676,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); } /// @@ -1656,6 +1714,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. /// @@ -1704,7 +1771,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/Commands/SetupSubcommands/SetupResults.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs index 088761c2..a4786952 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. /// @@ -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/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index 2168ee4c..1a7618e8 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -108,6 +108,16 @@ public static class ConfigConstants /// public const string ObservabilityApiIdentifierUri = "api://9b975845-388f-4429-889e-eab1ef63949c"; + /// + /// Defender API App ID + /// + public const string DefenderApiAppId = "86a21212-634e-4553-b3d6-e477e4c9d9ec"; + + /// + /// Defender API identifier URI. + /// + public const string DefenderApiIdentifierUri = "api://86a21212-634e-4553-b3d6-e477e4c9d9ec"; + /// /// Single source of truth for the Messaging Bot API delegated scope. /// The resource SP (appId 5a807f24-c9de-44ee-a3a7-329e88a00ffc) exposes exactly @@ -125,6 +135,12 @@ public static class ConfigConstants /// public const string ObservabilityApiOtelWriteScope = "Agent365.Observability.OtelWrite"; + /// + /// 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.Evaluate.All"; + /// /// 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/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/Services/LogRedactionService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs index d8402d9d..b101f1d1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/LogRedactionService.cs @@ -60,6 +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 + ConfigConstants.DefenderApiAppId, ConfigConstants.GccObservabilityApiAppId, ConfigConstants.GccHighObservabilityApiAppId, ConfigConstants.DodObservabilityApiAppId, 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/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index 432272d6..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 @@ -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,12 +419,66 @@ 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 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(" 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, + 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() { @@ -426,14 +487,57 @@ 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, 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/BatchPermissionsOrchestratorMissingSpTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorMissingSpTests.cs index 84e4f13f..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 @@ -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. /// /// /// @@ -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( @@ -147,6 +151,87 @@ 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 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() { @@ -253,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[] { @@ -267,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 fa985611..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(), @@ -312,16 +327,234 @@ 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 }) ]; + [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 @@ -501,8 +734,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 +770,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"); } // ────────────────────────────────────────────────────────────────────────────────────── @@ -903,6 +1139,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, 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 0f9765ad..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() @@ -157,10 +155,32 @@ public void PrintDryRunPlan_AuthModeObo_ShowsDelegatedGrantsOnAgentIdentity() AnyLogContains("delegated").Should().BeTrue(because: "OBO mode applies principal-scoped delegated grants to the agent identity SP"); } + /// + /// OBO mode requests the Defender delegated permission only. + /// + [Theory] + [InlineData("obo")] + [InlineData(null)] + public void PrintDryRunPlan_AuthModeObo_OmitsDefenderApplicationRoleGrant(string? authMode) + { + NonDwBlueprintSetupOrchestrator.PrintDryRunPlan( + BuildConfig(), + _logger, + authMode: authMode, + skipObservabilityPermissions: true); + + 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() @@ -172,17 +192,17 @@ public void PrintDryRunPlan_AuthModeS2s_ShowsAppPermsOnAgentIdentity_NoDelegated } /// - /// Blueprint agents skip OtelWrite by default, which leaves the S2S half with no app roles to assign. + /// Skipping OtelWrite does not remove the Defender application role required for S2S evaluation. /// [Fact] - public void PrintDryRunPlan_AuthModeS2s_WhenObservabilitySkipped_ShowsNoAppRolesToGrant() + public void PrintDryRunPlan_AuthModeS2s_WhenObservabilitySkipped_ShowsDefenderAppRoleGrant() { NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(BuildConfig(), _logger, authMode: "s2s", skipObservabilityPermissions: true); - AnyLogContains("not required (no S2S app roles to grant)").Should().BeTrue( - because: "the dry-run plan must match the real summary when the spec list has no app roles"); - AnyLogContains("Global Administrator required if 403").Should().BeFalse( - because: "there is no S2S grant to perform when blueprint agents skip OtelWrite"); + AnyLogContains("S2S app roles").Should().BeTrue( + because: "RealtimeProtection.Evaluate.All remains required when Observability permissions are skipped"); + AnyLogContains("Global Administrator required if 403").Should().BeTrue( + because: "assigning the Defender application role may require Global Administrator"); } [Fact] @@ -202,8 +222,8 @@ public void PrintDryRunPlan_CustomObservabilityPermission_DoesNotClaimObservabil AnyLogContains("Observability API not requested").Should().BeFalse( because: "a custom Observability permission explicitly opts back into requesting Observability permissions"); - AnyLogContains("not required (no S2S app roles to grant)").Should().BeTrue( - because: "custom permissions carry delegated scopes only, so there is still no S2S app role to grant"); + AnyLogContains("S2S app roles").Should().BeTrue( + because: "the fixed Defender permission still carries an application role when Observability is configured as a custom delegated permission"); } /// @@ -220,17 +240,17 @@ public void PrintDryRunPlan_AuthModeBoth_ShowsBothGrantRowsOnAgentIdentity() } /// - /// Both mode still has delegated consent work, but no S2S grant when no spec carries app roles. + /// Both mode retains delegated consent work and the Defender S2S app role when OtelWrite is skipped. /// [Fact] - public void PrintDryRunPlan_AuthModeBoth_WhenObservabilitySkipped_ShowsDelegatedOnly() + public void PrintDryRunPlan_AuthModeBoth_WhenObservabilitySkipped_ShowsDelegatedAndDefenderAppRole() { NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(BuildConfig(), _logger, authMode: "both", skipObservabilityPermissions: true); - AnyLogContains("delegated grants for the signed-in principal; no S2S app roles to grant").Should().BeTrue( - because: "both mode must preserve the delegated half while saying the S2S half has no role to assign"); - AnyLogContains("Global Administrator required for S2S if 403").Should().BeFalse( - because: "there is no S2S grant fallback to describe when blueprint agents skip OtelWrite"); + AnyLogContains("delegated grants for the signed-in principal + S2S app roles").Should().BeTrue( + because: "both mode must preserve delegated grants and the Defender application role when OtelWrite is skipped"); + AnyLogContains("Global Administrator required for S2S if 403").Should().BeTrue( + because: "the Defender S2S app role still needs an administrative fallback"); } /// 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..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 @@ -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() { @@ -307,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] @@ -1031,4 +1057,3 @@ public async Task ConfigureBotPermissionsAsync_AdminPath_WhenS2SFailsWithExterna #endregion } - 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>()); } 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 63b64cd5..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 @@ -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 }, @@ -280,6 +281,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"); @@ -306,6 +308,34 @@ 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.DefenderApiRealtimeProtectionScope }, + 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.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"); + } + + [Fact] + 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.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"); + } + [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 03fcd851..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 @@ -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.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"); + defenderLines[0].Should().Contain("Delegated", + because: "only delegated grants are needed for OBO — no Application permissions"); + } + [Fact] public void LogNonDwAdminConsentInstructions_DoesNotEmitOptionBPowerShell() { @@ -146,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/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index cfa16533..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 @@ -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,16 @@ 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_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 its delegated permission under the api://{appId} audience"); + } + [Fact] public void BuildAdminConsentUrls_PowerPlatformApi_UsesCorrectScopeConstant() { @@ -215,9 +226,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()); @@ -226,6 +237,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.DefenderApiRealtimeProtectionScope}"), + 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}")); } @@ -288,8 +301,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] 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..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 @@ -431,6 +431,106 @@ 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_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() { @@ -861,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() { 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() {