diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f625256..e8573d66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added - Re-enabled `a365 create-instance` command (previously deprecated) — creates agent identity, agent user, and assigns licenses in a single command. The custom client app now requires the `User.ReadWrite.All` delegated permission for user creation and license assignment; existing users may need to update admin consent on their client app. -- `Agent365.Observability.OtelWrite` scope now granted to all provisioned agent identities on the Observability API alongside `user_impersonation`, enabling agents to write OpenTelemetry data to the Agent 365 observability service +- `Agent365.Observability.OtelWrite` granted to all provisioned agent identities on the Observability API as both a **delegated** permission (OAuth2 grant) and an **application** permission (S2S app role assignment), enabling agents to write OpenTelemetry data to the Agent 365 observability service +- S2S app role assignment support in `a365 setup permissions` and `a365 setup admin` — the CLI now automatically grants application-type (`appRoleAssignments`) permissions on the blueprint service principal when a `ResourcePermissionSpec` defines `AppRoleScopes`. Global Administrator is required for S2S grants; non-admin users receive actionable PowerShell fallback instructions - `ChannelMessage.Read.All` and `ChannelMessage.Send` added to default blueprint Microsoft Graph delegated scopes (`agentIdentityScopes`) - `Files.ReadWrite.All`, `ChannelMessage.Read.All`, and `ChannelMessage.Send` added to default blueprint Microsoft Graph application scopes (`agentApplicationScopes`) - Server-driven notice system: security advisories and critical upgrade prompts are displayed at startup when a maintainer updates `notices.json`. Notices are suppressed once the user upgrades past the specified `minimumVersion`. Results are cached locally for 4 hours to avoid network calls on every invocation. @@ -22,6 +23,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - `A365CreateInstanceRunner` sponsor handling: sponsor is now required (Graph API rejects requests without one) — removed fallback that silently stripped the sponsor on retry, which caused `BadRequest` errors - Intermittent `ConnectionResetError (10054)` failures on corporate networks with TLS inspection proxies (Zscaler, Netskope) — Graph and ARM API calls now use direct MSAL.NET token acquisition instead of `az account get-access-token` subprocesses, bypassing the Python HTTP stack that triggered proxy resets (#321) - `a365 cleanup` blueprint deletion now succeeds for Global Administrators even when the blueprint was created by a different user +- Admin consent URL for the Observability API used the non-existent scope `Maven.ReadWrite.All` (AADSTS650053) — replaced with the correct delegated scope `Agent365.Observability.OtelWrite` +- `AppRoleAssignment.ReadWrite.All` (admin-only) was incorrectly included in `RequiredPermissionGrantScopes`, causing it to be requested on non-admin paths (`a365 deploy`, `setup permissions`) — moved to a dedicated `RequiredS2SGrantScopes` constant used only on Global Administrator paths - `a365 setup all` no longer times out for non-admin users — the CLI immediately surfaces a consent URL to share with an administrator instead of waiting for a browser prompt - `a365 setup all` requests admin consent once for all resources instead of prompting once per resource - Browser and WAM authentication blocked by Conditional Access Policy (AADSTS53003, AADSTS53000) now automatically falls back to device code flow (#294) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs index f88b9e12..6824f52f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs @@ -74,7 +74,7 @@ public static Command CreateCommand( logger, configService, executor, botConfigurator, authValidator, platformDetector, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService, armApiService)); command.AddCommand(AdminSubcommand.CreateCommand( - logger, configService, authValidator, graphApiService, confirmationProvider)); + logger, configService, authValidator, graphApiService, confirmationProvider, blueprintService)); return command; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AdminSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AdminSubcommand.cs index 8d2c257e..bb62d35d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AdminSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AdminSubcommand.cs @@ -38,7 +38,8 @@ public static Command CreateCommand( IConfigService configService, AzureAuthValidator authValidator, GraphApiService graphApiService, - IConfirmationProvider confirmationProvider) + IConfirmationProvider confirmationProvider, + AgentBlueprintService? blueprintService = null) { var command = new Command( "admin", @@ -215,7 +216,8 @@ await BatchPermissionsOrchestrator.GrantAdminPermissionsAsync( graphApiService, setupConfig, setupConfig.AgentBlueprintId!, setupConfig.TenantId, specs, logger, setupResults, ct, - knownBlueprintSpObjectId: setupConfig.AgentBlueprintServicePrincipalObjectId); + knownBlueprintSpObjectId: setupConfig.AgentBlueprintServicePrincipalObjectId, + blueprintService: blueprintService); setupResults.AdminConsentGranted = grantsConfigured; 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 c200713b..ed711d47 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -171,6 +171,12 @@ internal static class BatchPermissionsOrchestrator var grantsOk = await ConfigureOauth2GrantsAsync( graph, blueprintAppId, tenantId, specs, phase1Result, permScopes, logger, ct); + // S2S runs after delegated grants, regardless of their success, using elevated scopes. + var s2sScopes = permScopes.Concat(AuthenticationConstants.RequiredS2SGrantScopes).ToArray(); + if (!string.IsNullOrWhiteSpace(phase1Result?.BlueprintSpObjectId)) + await PerformS2SGrantsAsync(blueprintService, tenantId, phase1Result.BlueprintSpObjectId, specs, s2sScopes, logger, setupResults, ct); + // else: blueprint SP was not resolved — leave S2SAppRoleGranted = null (not attempted) + logger.LogInformation(""); if (grantsOk) { @@ -389,6 +395,7 @@ private static async Task UpdateBlueprintPermissions /// Phase 2b: Creates AllPrincipals (tenant-wide) OAuth2 permission grants for all specs. /// Requires Global Administrator. Only called when the current user is confirmed GA. /// Returns true if all grants succeeded, false if any grant failed. + /// S2S app role assignments are handled separately by . /// private static async Task ConfigureOauth2GrantsAsync( GraphApiService graph, @@ -443,6 +450,60 @@ private static async Task ConfigureOauth2GrantsAsync( return allGrantsOk; } + /// + /// Grants S2S app role assignments for all specs that carry . + /// Idempotent: skips roles already assigned. Sets on completion. + /// Requires Global Administrator and . + /// + private static async Task PerformS2SGrantsAsync( + AgentBlueprintService blueprintService, + string tenantId, + string blueprintSpObjectId, + IEnumerable specs, + string[] s2sScopes, + ILogger logger, + SetupResults? setupResults, + CancellationToken ct) + { + var s2sSpecs = specs.Where(s => s.AppRoleScopes is { Length: > 0 }).ToList(); + if (s2sSpecs.Count == 0) + { + if (setupResults is not null) setupResults.S2SAppRoleGranted = true; + return; + } + + logger.LogInformation(""); + logger.LogInformation("Configuring S2S app role assignments..."); + + var allS2SOk = true; + foreach (var spec in s2sSpecs) + { + logger.LogDebug( + " - App role assignment: blueprint -> {ResourceName} [{AppRoles}]", + spec.ResourceName, string.Join(' ', spec.AppRoleScopes!)); + + var s2sOk = await blueprintService.GrantAppRoleAssignmentAsync( + tenantId, + blueprintSpObjectId, + spec.ResourceAppId, + spec.AppRoleScopes!, + requiredScopes: s2sScopes, + ct: ct); + + if (s2sOk) + logger.LogInformation(" - S2S app role assigned for {ResourceName}", spec.ResourceName); + else + { + logger.LogWarning(" - Failed to assign S2S app role for {ResourceName}.", spec.ResourceName); + setupResults?.Warnings.Add($"S2S app role assignment failed for {spec.ResourceName}. Re-run 'a365 setup admin' to retry."); + allS2SOk = false; + } + } + + if (setupResults is not null) + setupResults.S2SAppRoleGranted = allS2SOk; + } + /// /// Phase 3: Checks for existing consent (skips browser if found), then either opens the /// browser for admins or returns a consolidated consent URL for non-admins. @@ -535,6 +596,9 @@ private static async Task ConfigureOauth2GrantsAsync( // we performed a role check and found the user lacks the GA role. if (adminCheck == Models.RoleCheckResult.DoesNotHaveRole) { + var s2sSpecs = specs.Where(s => s.AppRoleScopes is { Length: > 0 }).ToList(); + if (s2sSpecs.Count > 0 && setupResults is not null) + setupResults.S2SAppRoleGranted = false; return (false, consentUrl); } @@ -678,7 +742,8 @@ private record BlueprintPermissionsResult( ILogger logger, SetupResults setupResults, CancellationToken ct, - string? knownBlueprintSpObjectId = null) + string? knownBlueprintSpObjectId = null, + AgentBlueprintService? blueprintService = null) { if (specs.Count == 0) { @@ -689,8 +754,14 @@ private record BlueprintPermissionsResult( var effectiveSpecs = specs.Where(s => s.Scopes.Length > 0).ToList(); if (effectiveSpecs.Count == 0) { - logger.LogInformation("All permission specs have empty scope lists — nothing to grant."); - return (true, null); + var hasS2SSpecs = specs.Any(s => s.AppRoleScopes is { Length: > 0 }); + if (!hasS2SSpecs) + { + logger.LogInformation("All permission specs have empty scope and app role lists — nothing to grant."); + setupResults.S2SAppRoleGranted = true; + return (true, null); + } + logger.LogDebug("No delegated scopes to grant — proceeding with S2S app role assignments."); } var permScopes = AuthenticationConstants.RequiredPermissionGrantScopes; @@ -699,11 +770,17 @@ private record BlueprintPermissionsResult( logger.LogInformation(""); logger.LogInformation("Resolving service principals..."); + // When delegated specs are empty but S2S specs exist, resolve SPs from S2S specs instead + // so the blueprint SP object ID is available for app role assignment. + var specsForPhase1 = effectiveSpecs.Count > 0 + ? (IReadOnlyList)effectiveSpecs + : specs.Where(s => s.AppRoleScopes is { Length: > 0 }).ToList(); + BlueprintPermissionsResult? phase1Result = null; try { phase1Result = await UpdateBlueprintPermissionsAsync( - graph, blueprintAppId, tenantId, effectiveSpecs, permScopes, logger, ct, + graph, blueprintAppId, tenantId, specsForPhase1, permScopes, logger, ct, knownBlueprintSpObjectId); } catch (Exception ex) @@ -758,6 +835,12 @@ private record BlueprintPermissionsResult( } } + // S2S: Grant app role assignments for specs that carry AppRoleScopes. + var s2sScopes = permScopes.Concat(AuthenticationConstants.RequiredS2SGrantScopes).ToArray(); + if (blueprintService is not null && !string.IsNullOrWhiteSpace(phase1Result.BlueprintSpObjectId)) + await PerformS2SGrantsAsync(blueprintService, tenantId, phase1Result.BlueprintSpObjectId, specs, s2sScopes, logger, setupResults, ct); + // else: blueprint service unavailable or SP not resolved — leave S2SAppRoleGranted = null (not attempted) + return (allGrantsOk, phase1Result.BlueprintSpObjectId); } } 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 dbe2695c..5ca5d19e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/ResourcePermissionSpec.cs @@ -18,4 +18,5 @@ internal record ResourcePermissionSpec( string ResourceAppId, string ResourceName, string[] Scopes, - bool SetInheritable); + bool SetInheritable, + string[]? AppRoleScopes = null); 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 3d967ffc..b9e8e14f 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -16,6 +16,10 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; /// internal static class SetupHelpers { + internal const int DryRunValCol = 30; + internal static string DryRunRow(string label) => (" " + label).PadRight(DryRunValCol); + internal static string DryRunRow(int step, string label) => $" {step}. {label}".PadRight(DryRunValCol); + /// /// Returns the fixed-scope ResourcePermissionSpecs for the three platform APIs that every /// agent blueprint requires: Messaging Bot API, Observability API, and Power Platform API. @@ -31,8 +35,9 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInhe new ResourcePermissionSpec( ConfigConstants.ObservabilityApiAppId, "Observability API", - new[] { "user_impersonation", ConfigConstants.ObservabilityApiOtelWriteScope }, - setInheritable), + new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, + setInheritable, + AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope }), new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -105,61 +110,90 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) { logger.LogInformation(""); logger.LogInformation("Setup Summary"); + logger.LogInformation(""); var pendingAdminAction = !results.AdminConsentGranted && results.BatchPermissionsPhase2Completed; - // Completed steps — [OK] only - logger.LogInformation("Completed Steps:"); + // Numbered step rows — az CLI style: label padded to fixed column, then status word + var step = 0; + if (results.InfrastructureCreated) { - var status = results.InfrastructureAlreadyExisted ? "(already exists)" : "created"; - logger.LogInformation(" [OK] Infrastructure {Status}", status); + step++; + logger.LogInformation(DryRunRow(step, "Azure hosting") + (results.InfrastructureAlreadyExisted ? "reused" : "provisioned")); } + if (results.BlueprintCreated) { - var status = results.BlueprintAlreadyExisted ? "(already exists)" : "created"; - logger.LogInformation(" [OK] Agent blueprint {Status} ID: {BlueprintId}", status, results.BlueprintId ?? "unknown"); + step++; + var bpStatus = results.BlueprintAlreadyExisted ? "reused" : "created"; + logger.LogInformation(DryRunRow(step, "Blueprint") + "{Status} ID: {Id}", bpStatus, results.BlueprintId ?? "unknown"); } + if (results.BatchPermissionsPhase2Completed) { - logger.LogInformation(" [OK] Inheritable permissions configured and verified"); - if (results.AdminConsentGranted) - logger.LogInformation(" [OK] OAuth2 grants and admin consent configured"); + step++; + var grantStatus = results.AdminConsentGranted ? "ok" : "PENDING"; + logger.LogInformation(DryRunRow(step, "Permissions") + grantStatus); } + if (results.MessagingEndpointRegistered) { - var status = results.EndpointAlreadyExisted ? "(already exists)" : "created"; - logger.LogInformation(" [OK] Messaging endpoint {Status}", status); + step++; + logger.LogInformation(DryRunRow(step, "Messaging endpoint") + (results.EndpointAlreadyExisted ? "reused" : "registered")); } - // Action required — shown as its own section so it isn't conflated with completed work - var hasActionRequired = pendingAdminAction || results.ClientSecretManualActionRequired; + // Action required — one numbered section with inline instructions + var pendingS2SAction = results.S2SAppRoleGranted == false; + var hasActionRequired = pendingAdminAction || results.ClientSecretManualActionRequired || pendingS2SAction; if (hasActionRequired) { + var blueprintAppId = results.BlueprintId ?? ""; + var consentUrl = results.CombinedConsentUrl ?? results.AdminConsentUrl; + var itemNum = 0; + logger.LogInformation(""); logger.LogInformation("Action Required:"); + if (results.ClientSecretManualActionRequired) - logger.LogInformation(" Client secret - must be created manually in Entra ID and added to a365.generated.config.json (see instructions above)"); - if (pendingAdminAction) - logger.LogInformation(" OAuth2 grants — Global Administrator must grant consent (see Next Steps)"); + { + itemNum++; + logger.LogInformation(" {N}. Client secret — create manually in Entra ID and add to a365.generated.config.json (see instructions above)", itemNum); + } + + if (pendingAdminAction && !string.IsNullOrWhiteSpace(consentUrl)) + { + itemNum++; + logger.LogInformation(" {N}. OAuth2 permission grants — share this URL with your Global Administrator:", itemNum); + logger.LogInformation(" {ConsentUrl}", consentUrl); + } + + if (pendingS2SAction) + { + itemNum++; + logger.LogInformation(" {N}. Observability API S2S app role — run as Global Administrator (PowerShell):", itemNum); + logger.LogInformation(" Connect-MgGraph -Scopes 'AppRoleAssignment.ReadWrite.All'"); + logger.LogInformation(" $bp = Get-MgServicePrincipal -Filter \"appId eq '{BlueprintAppId}'\"", blueprintAppId); + logger.LogInformation(" $obs = Get-MgServicePrincipal -Filter \"appId eq '{ObsApiAppId}'\"", ConfigConstants.ObservabilityApiAppId); + logger.LogInformation(" $rid = ($obs.AppRoles | Where-Object {{ $_.Value -eq '{ObsScope}' }}).Id", ConfigConstants.ObservabilityApiOtelWriteScope); + logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $bp.Id -PrincipalId $bp.Id -ResourceId $obs.Id -AppRoleId $rid"); + } } - // Failed steps if (results.Errors.Count > 0) { logger.LogInformation(""); - logger.LogInformation("Failed Steps:"); + logger.LogInformation("Errors:"); foreach (var error in results.Errors) - logger.LogError(" [FAILED] {Error}", error); + logger.LogError(" {Error}", error); } - // Warnings if (results.Warnings.Count > 0) { logger.LogInformation(""); logger.LogInformation("Warnings:"); foreach (var warning in results.Warnings) - logger.LogInformation(" [WARN] {Warning}", warning); + logger.LogInformation(" {Warning}", warning); } logger.LogInformation(""); @@ -178,25 +212,6 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) } } - if (pendingAdminAction) - { - logger.LogInformation(""); - logger.LogInformation("Next Steps — Global Administrator action required:"); - logger.LogInformation(" OAuth2 permission grants require a Global Administrator."); - logger.LogInformation(" Option 1 — Run the CLI as a Global Administrator:"); - logger.LogInformation(" a365 setup admin --config-dir \"\""); - if (!string.IsNullOrWhiteSpace(results.CombinedConsentUrl)) - { - logger.LogInformation(" Option 2 — Share a single consent URL with your Global Administrator:"); - logger.LogInformation(" {ConsentUrl}", results.CombinedConsentUrl); - } - else if (!string.IsNullOrWhiteSpace(results.AdminConsentUrl)) - { - logger.LogInformation(" Alternatively, a Global Administrator can grant Graph consent at:"); - logger.LogInformation(" {ConsentUrl}", results.AdminConsentUrl); - } - } - if (!results.HasErrors && !hasActionRequired) { if (results.HasWarnings) @@ -315,7 +330,7 @@ static string Build(string tenant, string client, string resourceUri, IEnumerabl urls.Add(("Agent 365 Tools", Build(tenantId, blueprintClientId, McpConstants.Agent365ToolsIdentifierUri, mcpScopeList))); urls.Add(("Messaging Bot API", Build(tenantId, blueprintClientId, ConfigConstants.MessagingBotApiIdentifierUri, new[] { ConfigConstants.MessagingBotApiAdminConsentScope }))); - urls.Add(("Observability API", Build(tenantId, blueprintClientId, ConfigConstants.ObservabilityApiIdentifierUri, new[] { ConfigConstants.ObservabilityApiAdminConsentScope }))); + urls.Add(("Observability API", Build(tenantId, blueprintClientId, ConfigConstants.ObservabilityApiIdentifierUri, new[] { ConfigConstants.ObservabilityApiOtelWriteScope }))); urls.Add(("Power Platform API", Build(tenantId, blueprintClientId, PowerPlatformConstants.PowerPlatformApiIdentifierUri, new[] { PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead }))); return urls; @@ -338,7 +353,7 @@ internal static string BuildCombinedConsentUrl( foreach (var s in mcpScopes) allScopes.Add($"{McpConstants.Agent365ToolsIdentifierUri}/{s}"); allScopes.Add($"{ConfigConstants.MessagingBotApiIdentifierUri}/{ConfigConstants.MessagingBotApiAdminConsentScope}"); - allScopes.Add($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiAdminConsentScope}"); + allScopes.Add($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes); } @@ -354,19 +369,28 @@ public static void DisplayAdminSetupSummary( { logger.LogInformation(""); logger.LogInformation("Admin Setup Summary"); - logger.LogInformation("Completed Steps:"); + logger.LogInformation(""); + + var adminStep = 0; if (results.AdminConsentGranted) { - logger.LogInformation(" [OK] OAuth2 grants configured (tenant-wide)"); + adminStep++; + logger.LogInformation(DryRunRow(adminStep, "Permission grants") + "ok (tenant-wide)"); + } + + if (results.S2SAppRoleGranted == true) + { + adminStep++; + logger.LogInformation(DryRunRow(adminStep, "S2S app role") + "ok ({Scope})", ConfigConstants.ObservabilityApiOtelWriteScope); } if (results.Errors.Count > 0) { logger.LogInformation(""); - logger.LogInformation("Failed Steps:"); + logger.LogInformation("Errors:"); foreach (var error in results.Errors) - logger.LogError(" [FAILED] {Error}", error); + logger.LogError(" {Error}", error); } if (results.Warnings.Count > 0) @@ -374,7 +398,7 @@ public static void DisplayAdminSetupSummary( logger.LogInformation(""); logger.LogInformation("Warnings:"); foreach (var warning in results.Warnings) - logger.LogInformation(" [WARN] {Warning}", warning); + logger.LogInformation(" {Warning}", warning); } logger.LogInformation(""); 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 b5175f70..b29df131 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -35,6 +35,12 @@ public class SetupResults /// public bool AdminConsentGranted { get; set; } + /// + /// True when all S2S app role assignments were successfully granted. False means pending + /// (non-admin user; Global Administrator action required). Null means not attempted. + /// + public bool? S2SAppRoleGranted { get; set; } + /// /// Error message when Microsoft Graph inheritable permissions fail to configure. /// Non-null indicates failure. This is critical for agent token exchange functionality. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs index 363fecd1..749834bb 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs @@ -201,7 +201,18 @@ public static string[] GetRequiredRedirectUris(string clientAppId) { "Application.ReadWrite.All", "DelegatedPermissionGrant.ReadWrite.All", - "AgentIdentityBlueprint.UpdateAuthProperties.All" + "AgentIdentityBlueprint.UpdateAuthProperties.All", + }; + + /// + /// Additional scopes required only on Global Administrator paths that grant S2S app role + /// assignments. Kept separate from so that + /// non-admin flows (deploy, setup permissions) do not request an admin-only scope and + /// trigger unexpected consent prompts. + /// + public static readonly string[] RequiredS2SGrantScopes = new[] + { + "AppRoleAssignment.ReadWrite.All", }; /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs index f28eecc6..eba8131e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/ConfigConstants.cs @@ -76,13 +76,6 @@ public static class ConfigConstants /// public const string MessagingBotApiAdminConsentScope = "AgentData.ReadWrite"; - /// - /// Observability API scope used for admin consent URL construction. - /// Note: the orchestrator grants "user_impersonation" + ObservabilityApiOtelWriteScope via OAuth2 - /// permission grants; this scope is the consent-URL-facing name for the same resource. - /// - public const string ObservabilityApiAdminConsentScope = "Maven.ReadWrite.All"; - /// /// Observability API scope for writing OpenTelemetry data. /// Granted alongside "user_impersonation" to all provisioned agent identities. diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index e79243a8..1631f7bd 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -948,6 +948,149 @@ private async Task ResolveBlueprintObjectIdAsync( return blueprintAppId; } + /// + /// Grants application role assignments (appRoleAssignments) on the blueprint's service principal + /// for each named app role on the given resource. This enables S2S (service-to-service) access + /// where the blueprint identity calls the resource using a client-credentials token with no user + /// context. Idempotent: existing assignments for the same role are skipped. + /// Requires Global Administrator. + /// + /// Tenant ID. + /// Object ID of the blueprint's service principal. + /// Application ID of the resource (e.g. Observability API). + /// Names of the app roles to assign (e.g. "Agent365.Observability.OtelWrite"). + /// Graph scopes required to perform the operation. + /// Cancellation token. + /// True when all assignments succeeded or already existed; false if any failed. + public virtual async Task GrantAppRoleAssignmentAsync( + string tenantId, + string blueprintSpObjectId, + string resourceAppId, + IEnumerable appRoleNames, + IEnumerable? requiredScopes = null, + CancellationToken ct = default) + { + // De-dup upfront: duplicate names map to the same role ID and would cause a redundant POST. + var roleNames = appRoleNames? + .Where(n => !string.IsNullOrWhiteSpace(n)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList() ?? new List(); + if (roleNames.Count == 0) return true; + + try + { + // Resolve the resource service principal. + var resourceSpId = await _graphApiService.LookupServicePrincipalByAppIdAsync( + tenantId, resourceAppId, ct, requiredScopes); + if (string.IsNullOrWhiteSpace(resourceSpId)) + { + _logger.LogWarning("Resource SP not found for app ID {ResourceAppId} — S2S app role assignment skipped.", resourceAppId); + return false; + } + + // Fetch the resource SP's app roles to map names -> IDs. + using var resourceSpDoc = await _graphApiService.GraphGetAsync( + tenantId, $"/v1.0/servicePrincipals/{resourceSpId}?$select=appRoles", ct, + scopes: requiredScopes); + if (resourceSpDoc == null) + { + _logger.LogError("Failed to retrieve app roles for resource SP {ResourceSpId}.", resourceSpId); + return false; + } + + // Build a name -> id map from the resource SP's appRoles array. + var roleIdByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + if (resourceSpDoc.RootElement.TryGetProperty("appRoles", out var appRolesEl) && + appRolesEl.ValueKind == JsonValueKind.Array) + { + foreach (var role in appRolesEl.EnumerateArray()) + { + if (role.TryGetProperty("value", out var valEl) && + role.TryGetProperty("id", out var idEl)) + { + var name = valEl.GetString(); + var id = idEl.GetString(); + if (!string.IsNullOrWhiteSpace(name) && !string.IsNullOrWhiteSpace(id)) + roleIdByName[name] = id; + } + } + } + + // Fetch existing assignments on the blueprint SP to avoid duplicates. + var existingRoleIds = new HashSet(StringComparer.OrdinalIgnoreCase); + using var existingDoc = await _graphApiService.GraphGetAsync( + tenantId, + $"/v1.0/servicePrincipals/{blueprintSpObjectId}/appRoleAssignments", + ct, scopes: requiredScopes); + if (existingDoc != null && + existingDoc.RootElement.TryGetProperty("value", out var existingArr) && + existingArr.ValueKind == JsonValueKind.Array) + { + foreach (var assignment in existingArr.EnumerateArray()) + { + if (assignment.TryGetProperty("resourceId", out var resId) && + resId.GetString()?.Equals(resourceSpId, StringComparison.OrdinalIgnoreCase) == true && + assignment.TryGetProperty("appRoleId", out var roleIdEl)) + { + var id = roleIdEl.GetString(); + if (!string.IsNullOrWhiteSpace(id)) existingRoleIds.Add(id); + } + } + } + + var allOk = true; + foreach (var roleName in roleNames) + { + if (!roleIdByName.TryGetValue(roleName, out var appRoleId)) + { + _logger.LogWarning("App role '{RoleName}' not found on resource {ResourceAppId} — assignment skipped.", roleName, resourceAppId); + allOk = false; + continue; + } + + if (existingRoleIds.Contains(appRoleId)) + { + _logger.LogDebug("App role '{RoleName}' already assigned on blueprint SP {BpSpId}.", roleName, blueprintSpObjectId); + continue; + } + + var payload = new + { + principalId = blueprintSpObjectId, + resourceId = resourceSpId, + appRoleId = appRoleId + }; + + var resp = await _graphApiService.GraphPostWithResponseAsync( + tenantId, + $"/v1.0/servicePrincipals/{blueprintSpObjectId}/appRoleAssignments", + payload, + ct, + requiredScopes); + + if (resp.IsSuccess) + { + _logger.LogDebug("App role '{RoleName}' assigned to blueprint SP {BpSpId}.", roleName, blueprintSpObjectId); + existingRoleIds.Add(appRoleId); // prevent duplicate POST if same ID appears again + } + else + { + _logger.LogWarning( + "Failed to assign app role '{RoleName}': HTTP {Status} {Reason} — {Body}", + roleName, (int)resp.StatusCode, resp.ReasonPhrase, resp.Body); + allOk = false; + } + } + + return allOk; + } + catch (Exception ex) + { + _logger.LogError(ex, "Exception granting app role assignments on {ResourceAppId}: {Message}", resourceAppId, ex.Message); + return false; + } + } + private static List ParseInheritableScopesFromJson(JsonElement entry) { var scopesList = new List(); 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 f23e76de..d099434e 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 @@ -68,7 +68,7 @@ public void BuildAdminConsentUrls_ObservabilityApi_UsesCorrectScopeConstant() var urls = SetupHelpers.BuildAdminConsentUrls(TenantId, BlueprintClientId, new[] { "Mail.Send" }, new[] { "scope" }); var obsUrl = urls.First(u => u.ResourceName == "Observability API").ConsentUrl; - obsUrl.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiAdminConsentScope}"), + obsUrl.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"), because: "scope URIs are Uri.EscapeDataString-encoded in the query string — required by AAD for adminconsent"); } @@ -220,7 +220,8 @@ public void BuildCombinedConsentUrl_AlwaysIncludesAllThreeFixedResources() url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.MessagingBotApiIdentifierUri}/{ConfigConstants.MessagingBotApiAdminConsentScope}"), because: "scope URIs are Uri.EscapeDataString-encoded in the query string — required by AAD for adminconsent"); - url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiAdminConsentScope}")); + url.Should().Contain(Uri.EscapeDataString($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"), + because: "scope URIs are Uri.EscapeDataString-encoded in the query string — required by AAD for adminconsent"); url.Should().Contain(Uri.EscapeDataString($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}")); } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs index d85e33af..d702f2ce 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/AgentBlueprintServiceTests.cs @@ -464,10 +464,195 @@ private static IAuthenticationService FakeAuth() // Pass a no-op login hint resolver to skip the real 'az account show' process spawned by // AzCliHelper.ResolveLoginHintAsync — that static call bypasses the mocked CommandExecutor // and causes each test to wait several seconds for the real az CLI. - var graphService = new GraphApiService(_mockGraphLogger, executor, Substitute.For(), handler, _mockTokenProvider, + var graphService = new GraphApiService(_mockGraphLogger, executor, FakeAuth(), handler, _mockTokenProvider, loginHintResolver: () => Task.FromResult(null)); return (new AgentBlueprintService(_mockLogger, graphService), handler); } + + // ── GrantAppRoleAssignmentAsync tests ────────────────────────────────────── + + [Fact] + public async Task GrantAppRoleAssignmentAsync_WhenResourceSpNotFound_ReturnsFalse() + { + // Arrange + var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // SP lookup returns empty value array + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[]}") + }); + + // Act + var result = await service.GrantAppRoleAssignmentAsync( + "tenant-id", "blueprint-sp-id", "resource-app-id", + new[] { "Agent365.Observability.OtelWrite" }); + + // Assert + result.Should().BeFalse(); + } + } + + [Fact] + public async Task GrantAppRoleAssignmentAsync_WhenRoleNotFoundOnResourceSp_ReturnsFalse() + { + // Arrange + var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // SP lookup succeeds + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[{\"id\":\"resource-sp-id\"}]}") + }); + // Resource SP has no matching app roles + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"appRoles\":[]}") + }); + // Existing assignments + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[]}") + }); + + // Act + var result = await service.GrantAppRoleAssignmentAsync( + "tenant-id", "blueprint-sp-id", "resource-app-id", + new[] { "Agent365.Observability.OtelWrite" }); + + // Assert + result.Should().BeFalse(); + } + } + + [Fact] + public async Task GrantAppRoleAssignmentAsync_WhenRoleAlreadyAssigned_ReturnsTrueWithoutPost() + { + // Arrange + var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // SP lookup + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[{\"id\":\"resource-sp-id\"}]}") + }); + // Resource SP app roles + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"appRoles\":[{\"value\":\"Agent365.Observability.OtelWrite\",\"id\":\"role-id-1\"}]}") + }); + // Existing assignments — role already assigned + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[{\"resourceId\":\"resource-sp-id\",\"appRoleId\":\"role-id-1\"}]}") + }); + // No POST should be queued — if the handler is called a 4th time it returns 404 + + // Act + var result = await service.GrantAppRoleAssignmentAsync( + "tenant-id", "blueprint-sp-id", "resource-app-id", + new[] { "Agent365.Observability.OtelWrite" }); + + // Assert + result.Should().BeTrue(); + } + } + + [Fact] + public async Task GrantAppRoleAssignmentAsync_WhenPostSucceeds_ReturnsTrue() + { + // Arrange + var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // SP lookup + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[{\"id\":\"resource-sp-id\"}]}") + }); + // Resource SP app roles + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"appRoles\":[{\"value\":\"Agent365.Observability.OtelWrite\",\"id\":\"role-id-1\"}]}") + }); + // Existing assignments — none + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[]}") + }); + // POST succeeds + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Created) + { + Content = new StringContent("{\"id\":\"assignment-id\"}") + }); + + // Act + var result = await service.GrantAppRoleAssignmentAsync( + "tenant-id", "blueprint-sp-id", "resource-app-id", + new[] { "Agent365.Observability.OtelWrite" }); + + // Assert + result.Should().BeTrue(); + } + } + + [Fact] + public async Task GrantAppRoleAssignmentAsync_WhenPostFails_ReturnsFalse() + { + // Arrange + var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // SP lookup + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[{\"id\":\"resource-sp-id\"}]}") + }); + // Resource SP app roles + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"appRoles\":[{\"value\":\"Agent365.Observability.OtelWrite\",\"id\":\"role-id-1\"}]}") + }); + // Existing assignments — none + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{\"value\":[]}") + }); + // POST fails + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Forbidden) + { + Content = new StringContent("{\"error\":{\"code\":\"Authorization_RequestDenied\"}}") + }); + + // Act + var result = await service.GrantAppRoleAssignmentAsync( + "tenant-id", "blueprint-sp-id", "resource-app-id", + new[] { "Agent365.Observability.OtelWrite" }); + + // Assert + result.Should().BeFalse(); + } + } + + [Fact] + public async Task GrantAppRoleAssignmentAsync_WithEmptyRoleNames_ReturnsTrue() + { + // Arrange + var (service, handler) = CreateServiceWithFakeHandler(); + using (handler) + { + // Act — no HTTP calls should be made for an empty role list + var result = await service.GrantAppRoleAssignmentAsync( + "tenant-id", "blueprint-sp-id", "resource-app-id", + Array.Empty()); + + // Assert + result.Should().BeTrue(); + } + } } // Simple fake handler that returns queued responses sequentially