From 3c0aaccea8859eccc7447793b26ebc7aafba1bdb Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 6 Jan 2026 12:51:07 -0800 Subject: [PATCH 1/4] Idempotent setup: blueprint/infrastructure discovery Implement complete idempotency for a365 setup commands to handle repeated executions gracefully and provide accurate user feedback. Changes: - Add BlueprintLookupService for dual-path blueprint discovery (objectId primary, displayName fallback for migration scenarios) - Add FederatedCredentialService for FIC existence checks and creation - Persist blueprint objectIds (app + service principal) to config for authoritative future lookups - Track idempotency flags: InfrastructureAlreadyExisted, BlueprintAlreadyExisted, EndpointAlreadyExisted - Update setup summary to display "configured (already exists)" vs "created" Fixes: - Move endpoint error log after idempotency check (no more false ERR) - Convert verbose internal logs to LogDebug ("Saved dynamic state", auth checks) - Fix SaveStateAsync usage to prevent service principal ID overwrites Testing: - Add BlueprintLookupServiceTests (8 tests) - Add FederatedCredentialServiceTests (11 tests) - All 777/777 tests passing Wire new services through DI container and all setup subcommands. --- .../Commands/SetupCommand.cs | 6 +- .../SetupSubcommands/AllSubcommand.cs | 54 +-- .../SetupSubcommands/BlueprintSubcommand.cs | 214 ++++++++-- .../InfrastructureSubcommand.cs | 46 +- .../Commands/SetupSubcommands/SetupHelpers.cs | 13 +- .../Commands/SetupSubcommands/SetupResults.cs | 9 + .../Models/Agent365Config.cs | 15 + .../Program.cs | 6 +- .../Services/AgentBlueprintService.cs | 67 +++ .../Services/AzureAuthValidator.cs | 6 +- .../Services/AzureEnvironmentValidator.cs | 16 +- .../Services/AzureValidator.cs | 2 +- .../Services/BlueprintLookupService.cs | 335 +++++++++++++++ .../Services/BotConfigurator.cs | 25 +- .../Services/ConfigService.cs | 10 +- .../Services/FederatedCredentialService.cs | 318 ++++++++++++++ .../Services/GraphApiService.cs | 4 +- .../Commands/BlueprintSubcommandTests.cs | 44 +- .../Commands/SetupCommandTests.cs | 22 +- .../Services/BlueprintLookupServiceTests.cs | 283 ++++++++++++ .../FederatedCredentialServiceTests.cs | 404 ++++++++++++++++++ 21 files changed, 1739 insertions(+), 160 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs create mode 100644 src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs index 4b327b52..b0757d93 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs @@ -25,6 +25,8 @@ public static Command CreateCommand( PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, + BlueprintLookupService blueprintLookupService, + FederatedCredentialService federatedCredentialService, IClientAppValidator clientAppValidator) { var command = new Command("setup", @@ -47,13 +49,13 @@ public static Command CreateCommand( logger, configService, azureValidator, webAppCreator, platformDetector, executor)); command.AddCommand(BlueprintSubcommand.CreateCommand( - logger, configService, executor, azureValidator, webAppCreator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator)); + logger, configService, executor, azureValidator, webAppCreator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); command.AddCommand(PermissionsSubcommand.CreateCommand( logger, configService, executor, graphApiService, blueprintService)); command.AddCommand(AllSubcommand.CreateCommand( - logger, configService, executor, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, blueprintService, clientAppValidator)); + logger, configService, executor, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); return command; } 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 ff2e7640..0e7c5960 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -29,7 +29,9 @@ public static Command CreateCommand( PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, - IClientAppValidator clientAppValidator) + IClientAppValidator clientAppValidator, + BlueprintLookupService blueprintLookupService, + FederatedCredentialService federatedCredentialService) { var command = new Command("all", "Run complete Agent 365 setup (all steps in sequence)\n" + @@ -134,9 +136,7 @@ public static Command CreateCommand( // PHASE 0: CHECK REQUIREMENTS (if not skipped) if (!skipRequirements) { - logger.LogInformation("Step 0: Requirements Check"); - logger.LogInformation("Validating system prerequisites..."); - logger.LogInformation(""); + logger.LogDebug("Validating system prerequisites..."); try { @@ -164,18 +164,16 @@ public static Command CreateCommand( } else { - logger.LogInformation("Skipping requirements validation (--skip-requirements flag used)"); - logger.LogInformation(""); + logger.LogDebug("Skipping requirements validation (--skip-requirements flag used)"); } // PHASE 1: VALIDATE ALL PREREQUISITES UPFRONT - logger.LogInformation("Validating all prerequisites..."); - logger.LogInformation(""); + logger.LogDebug("Validating all prerequisites..."); var allErrors = new List(); // Validate Azure CLI authentication first - logger.LogInformation("Validating Azure CLI authentication..."); + logger.LogDebug("Validating Azure CLI authentication..."); if (!await azureValidator.ValidateAllAsync(setupConfig.SubscriptionId)) { allErrors.Add("Azure CLI authentication failed or subscription not set correctly"); @@ -183,13 +181,13 @@ public static Command CreateCommand( } else { - logger.LogInformation("Azure CLI authentication: OK"); + logger.LogDebug("Azure CLI authentication: OK"); } // Validate Infrastructure prerequisites if (!skipInfrastructure && setupConfig.NeedDeployment) { - logger.LogInformation("Validating Infrastructure prerequisites..."); + logger.LogDebug("Validating Infrastructure prerequisites..."); var infraErrors = await InfrastructureSubcommand.ValidateAsync(setupConfig, azureValidator, CancellationToken.None); if (infraErrors.Count > 0) { @@ -197,12 +195,12 @@ public static Command CreateCommand( } else { - logger.LogInformation("Infrastructure prerequisites: OK"); + logger.LogDebug("Infrastructure prerequisites: OK"); } } // Validate Blueprint prerequisites - logger.LogInformation("Validating Blueprint prerequisites..."); + logger.LogDebug("Validating Blueprint prerequisites..."); var blueprintErrors = await BlueprintSubcommand.ValidateAsync(setupConfig, azureValidator, clientAppValidator, CancellationToken.None); if (blueprintErrors.Count > 0) { @@ -210,7 +208,7 @@ public static Command CreateCommand( } else { - logger.LogInformation("Blueprint prerequisites: OK"); + logger.LogDebug("Blueprint prerequisites: OK"); } // Stop if any validation failed @@ -229,9 +227,7 @@ public static Command CreateCommand( return; } - logger.LogInformation(""); - logger.LogInformation("All validations passed. Starting setup execution..."); - logger.LogInformation(""); + logger.LogDebug("All validations passed. Starting setup execution..."); var generatedConfigPath = Path.Combine( config.DirectoryName ?? Environment.CurrentDirectory, @@ -240,10 +236,8 @@ public static Command CreateCommand( // Step 1: Infrastructure (optional) try { - logger.LogInformation("Step 1:"); - logger.LogInformation(""); - bool setupInfra = await InfrastructureSubcommand.CreateInfrastructureImplementationAsync( + var (setupInfra, infraAlreadyExisted) = await InfrastructureSubcommand.CreateInfrastructureImplementationAsync( logger, config.FullName, generatedConfigPath, @@ -254,6 +248,7 @@ public static Command CreateCommand( CancellationToken.None); setupResults.InfrastructureCreated = skipInfrastructure ? false : setupInfra; + setupResults.InfrastructureAlreadyExisted = infraAlreadyExisted; } catch (Agent365Exception infraEx) { @@ -270,10 +265,6 @@ public static Command CreateCommand( } // Step 2: Blueprint - logger.LogInformation(""); - logger.LogInformation("Step 2:"); - logger.LogInformation(""); - try { var result = await BlueprintSubcommand.CreateBlueprintImplementationAsync( @@ -288,11 +279,15 @@ public static Command CreateCommand( botConfigurator, platformDetector, graphApiService, - blueprintService + blueprintService, + blueprintLookupService, + federatedCredentialService ); setupResults.BlueprintCreated = result.BlueprintCreated; + setupResults.BlueprintAlreadyExisted = result.BlueprintAlreadyExisted; setupResults.MessagingEndpointRegistered = result.EndpointRegistered; + setupResults.EndpointAlreadyExisted = result.EndpointAlreadyExisted; if (result.EndpointAlreadyExisted) { @@ -350,10 +345,6 @@ public static Command CreateCommand( } // Step 3: MCP Permissions - logger.LogInformation(""); - logger.LogInformation("Step 3:"); - logger.LogInformation(""); - try { bool mcpPermissionSetup = await PermissionsSubcommand.ConfigureMcpPermissionsAsync( @@ -381,11 +372,6 @@ public static Command CreateCommand( } // Step 4: Bot API Permissions - - logger.LogInformation(""); - logger.LogInformation("Step 4:"); - logger.LogInformation(""); - try { bool botPermissionSetup = await PermissionsSubcommand.ConfigureBotPermissionsAsync( diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index 60baa3be..0ddaf0e9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -25,6 +25,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; internal class BlueprintCreationResult { public bool BlueprintCreated { get; set; } + public bool BlueprintAlreadyExisted { get; set; } public bool EndpointRegistered { get; set; } public bool EndpointAlreadyExisted { get; set; } /// @@ -99,7 +100,9 @@ public static Command CreateCommand( IBotConfigurator botConfigurator, GraphApiService graphApiService, AgentBlueprintService blueprintService, - IClientAppValidator clientAppValidator) + IClientAppValidator clientAppValidator, + BlueprintLookupService blueprintLookupService, + FederatedCredentialService federatedCredentialService) { var command = new Command("blueprint", "Create agent blueprint (Entra ID application registration)\n" + @@ -195,6 +198,8 @@ await CreateBlueprintImplementationAsync( platformDetector, graphApiService, blueprintService, + blueprintLookupService, + federatedCredentialService, skipEndpointRegistration ); @@ -216,6 +221,8 @@ public static async Task CreateBlueprintImplementationA PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, + BlueprintLookupService blueprintLookupService, + FederatedCredentialService federatedCredentialService, bool skipEndpointRegistration = false, CancellationToken cancellationToken = default) { @@ -278,14 +285,9 @@ public static async Task CreateBlueprintImplementationA // Phase 2.1: Delegated Consent // ======================================================================== - logger.LogInformation(""); - logger.LogInformation("==> Creating Agent Blueprint"); - // CRITICAL: Grant AgentApplication.Create permission BEFORE creating blueprint // This replaces the PowerShell call to DelegatedAgentApplicationCreateConsent.ps1 - logger.LogInformation(""); - logger.LogInformation("==> Ensuring AgentApplication.Create Permission"); - logger.LogInformation("This permission is required to create Agent Blueprints"); + logger.LogDebug("Ensuring AgentApplication.Create permission"); var consentResult = await EnsureDelegatedConsentWithRetriesAsync( delegatedConsentService, @@ -308,9 +310,6 @@ public static async Task CreateBlueprintImplementationA // Phase 2.2: Create Blueprint // ======================================================================== - logger.LogInformation(""); - logger.LogInformation("==> Creating Agent Blueprint Application"); - // Validate required config if (string.IsNullOrWhiteSpace(setupConfig.AgentBlueprintDisplayName)) { @@ -324,6 +323,8 @@ public static async Task CreateBlueprintImplementationA executor, graphService, blueprintService, + blueprintLookupService, + federatedCredentialService, setupConfig.TenantId, setupConfig.AgentBlueprintDisplayName, setupConfig.AgentIdentityDisplayName, @@ -331,6 +332,8 @@ public static async Task CreateBlueprintImplementationA useManagedIdentity, generatedConfig, setupConfig, + configService, + config, cancellationToken); if (!blueprintResult.success) @@ -346,12 +349,10 @@ public static async Task CreateBlueprintImplementationA var blueprintAppId = blueprintResult.appId; var blueprintObjectId = blueprintResult.objectId; + var blueprintAlreadyExisted = blueprintResult.alreadyExisted; - logger.LogInformation("Agent Blueprint Details:"); - logger.LogInformation(" - Display Name: {Name}", setupConfig.AgentBlueprintDisplayName); - logger.LogInformation(" - App ID: {Id}", blueprintAppId); - logger.LogInformation(" - Object ID: {Id}", blueprintObjectId); - logger.LogInformation(" - Identifier URI: api://{Id}", blueprintAppId); + logger.LogDebug("Blueprint created: {Name} (Object ID: {ObjectId}, App ID: {AppId})", + setupConfig.AgentBlueprintDisplayName, blueprintObjectId, blueprintAppId); // Convert to camelCase and save var camelCaseConfig = new JsonObject @@ -373,9 +374,6 @@ public static async Task CreateBlueprintImplementationA // Phase 2.5: Create Client Secret (logging handled by method) // ======================================================================== - logger.LogInformation(""); - logger.LogInformation("==> Creating Client Secret for Agent Blueprint"); - await CreateBlueprintClientSecretAsync( blueprintObjectId!, blueprintAppId!, @@ -383,10 +381,18 @@ await CreateBlueprintClientSecretAsync( generatedConfigPath, graphService, setupConfig, + configService, logger); logger.LogInformation(""); - logger.LogInformation("Agent blueprint created successfully"); + if (blueprintAlreadyExisted) + { + logger.LogInformation("Agent blueprint configured successfully"); + } + else + { + logger.LogInformation("Agent blueprint created successfully"); + } logger.LogInformation("Generated config saved: {Path}", generatedConfigPath); logger.LogInformation(""); @@ -423,7 +429,7 @@ await CreateBlueprintClientSecretAsync( logger.LogWarning("Setup will continue to configure Bot API permissions"); logger.LogWarning(""); logger.LogWarning("To resolve endpoint registration issues:"); - logger.LogWarning(" 1. Delete existing endpoint: a365 cleanup azure"); + logger.LogWarning(" 1. Delete existing endpoint: a365 cleanup blueprint"); logger.LogWarning(" 2. Register endpoint again: a365 setup blueprint --endpoint-only"); logger.LogWarning(" Or rerun full setup: a365 setup blueprint"); logger.LogWarning(""); @@ -459,6 +465,7 @@ await CreateBlueprintClientSecretAsync( return new BlueprintCreationResult { BlueprintCreated = true, + BlueprintAlreadyExisted = blueprintAlreadyExisted, EndpointRegistered = endpointRegistered, EndpointAlreadyExisted = endpointAlreadyExisted, EndpointRegistrationAttempted = !skipEndpointRegistration @@ -516,14 +523,17 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( /// /// Creates Agent Blueprint application using Graph API + /// Implements dual-path discovery for idempotency: checks objectId from config first, falls back to displayName query. /// Used by: BlueprintSubcommand and A365SetupRunner Phase 2.2 - /// Returns: (success, appId, objectId, servicePrincipalId) + /// Returns: (success, appId, objectId, servicePrincipalId, alreadyExisted) /// - public static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId)> CreateAgentBlueprintAsync( + public static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId, bool alreadyExisted)> CreateAgentBlueprintAsync( ILogger logger, CommandExecutor executor, GraphApiService graphApiService, AgentBlueprintService blueprintService, + BlueprintLookupService blueprintLookupService, + FederatedCredentialService federatedCredentialService, string tenantId, string displayName, string? agentIdentityDisplayName, @@ -531,8 +541,92 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( bool useManagedIdentity, JsonObject generatedConfig, Models.Agent365Config setupConfig, + IConfigService configService, + FileInfo configFile, CancellationToken ct) { + // ======================================================================== + // Idempotency Check: Dual-Path Discovery + // ======================================================================== + + string? existingObjectId = setupConfig.AgentBlueprintObjectId; + string? existingAppId = null; + string? existingServicePrincipalId = setupConfig.AgentBlueprintServicePrincipalObjectId; + bool blueprintAlreadyExists = false; + bool requiresPersistence = false; + + // Primary path: Check if we have objectId in config + if (!string.IsNullOrWhiteSpace(existingObjectId)) + { + logger.LogDebug("Checking for existing blueprint with objectId: {ObjectId}...", existingObjectId); + var lookupResult = await blueprintLookupService.GetApplicationByObjectIdAsync(tenantId, existingObjectId, ct); + + if (lookupResult.Found) + { + logger.LogInformation("Blueprint '{DisplayName}' already exists", displayName); + + existingAppId = lookupResult.AppId; + blueprintAlreadyExists = true; + } + else + { + logger.LogWarning("ObjectId in config not found in Entra ID - will try discovery by display name"); + existingObjectId = null; + } + } + + // Fallback path: Query by displayName for migration scenarios + if (!blueprintAlreadyExists && !string.IsNullOrWhiteSpace(displayName)) + { + logger.LogDebug("Searching for existing blueprint by display name: {DisplayName}...", displayName); + var lookupResult = await blueprintLookupService.GetApplicationByDisplayNameAsync(tenantId, displayName, cancellationToken: ct); + + if (lookupResult.Found) + { + logger.LogDebug("Discovered existing blueprint from previous setup, updating config with objectId for faster lookups"); + + existingObjectId = lookupResult.ObjectId; + existingAppId = lookupResult.AppId; + blueprintAlreadyExists = true; + requiresPersistence = lookupResult.RequiresPersistence; + } + } + + // If blueprint exists, get service principal if we don't have it + if (blueprintAlreadyExists && !string.IsNullOrWhiteSpace(existingAppId)) + { + if (string.IsNullOrWhiteSpace(existingServicePrincipalId)) + { + logger.LogDebug("Looking up service principal for blueprint..."); + var spLookup = await blueprintLookupService.GetServicePrincipalByAppIdAsync(tenantId, existingAppId, ct); + + if (spLookup.Found) + { + logger.LogDebug("Service principal found: {ObjectId}", spLookup.ObjectId); + existingServicePrincipalId = spLookup.ObjectId; + requiresPersistence = true; + } + } + + // Persist objectIds if needed (migration scenario or new discovery) + if (requiresPersistence) + { + logger.LogDebug("Persisting blueprint metadata to config for faster future lookups..."); + setupConfig.AgentBlueprintObjectId = existingObjectId; + setupConfig.AgentBlueprintServicePrincipalObjectId = existingServicePrincipalId; + setupConfig.AgentBlueprintId = existingAppId; + + await configService.SaveStateAsync(setupConfig); + logger.LogDebug("Config updated with blueprint identifiers"); + } + + // Blueprint exists, skip creation and move to FIC/secret validation + return (true, existingAppId, existingObjectId, existingServicePrincipalId, alreadyExisted: true); + } + + // ======================================================================== + // Blueprint Creation: No existing blueprint found + // ======================================================================== try { logger.LogInformation("Creating Agent Blueprint using Microsoft Graph SDK..."); @@ -577,7 +671,7 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( if (string.IsNullOrEmpty(graphToken)) { logger.LogError("Failed to extract access token from Graph client"); - return (false, null, null, null); + return (false, null, null, null, alreadyExisted: false); } // Create the application using Microsoft Graph SDK @@ -621,13 +715,13 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( { errorContent = await appResponse.Content.ReadAsStringAsync(ct); logger.LogError("Failed to create application (fallback): {Status} - {Error}", appResponse.StatusCode, errorContent); - return (false, null, null, null); + return (false, null, null, null, alreadyExisted: false); } } else { logger.LogError("Failed to create application: {Status} - {Error}", appResponse.StatusCode, errorContent); - return (false, null, null, null); + return (false, null, null, null, alreadyExisted: false); } } @@ -657,7 +751,7 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( if (!appAvailable) { logger.LogError("Application object not available after creation and retries. Aborting setup."); - return (false, null, null, null); + return (false, null, null, null, alreadyExisted: false); } logger.LogInformation("Application object verified in directory"); @@ -746,26 +840,71 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( } } + // Persist blueprint identifiers immediately after creation + logger.LogInformation("Persisting blueprint metadata to config..."); + setupConfig.AgentBlueprintObjectId = objectId; + setupConfig.AgentBlueprintServicePrincipalObjectId = servicePrincipalId; + setupConfig.AgentBlueprintId = appId; + + logger.LogDebug("About to save config with: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", + objectId, servicePrincipalId, appId); + + await configService.SaveStateAsync(setupConfig); + logger.LogInformation("Config updated with blueprint identifiers"); + + logger.LogDebug("After save, config has: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", + setupConfig.AgentBlueprintObjectId, setupConfig.AgentBlueprintServicePrincipalObjectId, setupConfig.AgentBlueprintId); + // Create Federated Identity Credential ONLY when MSI is relevant (if managed identity provided) if (useManagedIdentity && !string.IsNullOrWhiteSpace(managedIdentityPrincipalId)) { logger.LogInformation("Creating Federated Identity Credential for Managed Identity..."); var credentialName = $"{displayName.Replace(" ", "")}-MSI"; - var ficSuccess = await CreateFederatedIdentityCredentialAsync( + // Check if FIC already exists before creating + var ficExistsResult = await federatedCredentialService.CheckFederatedCredentialExistsAsync( tenantId, objectId, - credentialName, managedIdentityPrincipalId, - graphToken, - logger, + $"https://login.microsoftonline.com/{tenantId}/v2.0", ct); - if (ficSuccess) + bool ficSuccess; + if (ficExistsResult.Exists) { - logger.LogInformation("Federated Identity Credential created successfully"); + logger.LogInformation("Federated credential already exists (skipping)"); + logger.LogInformation(" - Subject (MSI Principal ID): {MsiId}", managedIdentityPrincipalId); + ficSuccess = true; } else + { + logger.LogInformation("Creating new federated credential..."); + var ficCreateResult = await federatedCredentialService.CreateFederatedCredentialAsync( + tenantId, + objectId, + credentialName, + $"https://login.microsoftonline.com/{tenantId}/v2.0", + managedIdentityPrincipalId, + new List { "api://AzureADTokenExchange" }, + ct); + + ficSuccess = ficCreateResult.Success; + + if (ficCreateResult.AlreadyExisted) + { + logger.LogInformation("Federated credential was created by another process (race condition handled)"); + } + else if (ficSuccess) + { + logger.LogInformation("Federated Identity Credential created successfully"); + } + else + { + logger.LogWarning("Failed to create Federated Identity Credential: {Error}", ficCreateResult.ErrorMessage ?? "Unknown error"); + } + } + + if (!ficSuccess) { logger.LogWarning("Failed to create Federated Identity Credential"); } @@ -878,12 +1017,12 @@ await SetupHelpers.EnsureResourcePermissionsAsync( logger.LogWarning("Consent URL: {Url}", consentUrlGraph); } - return (true, appId, objectId, servicePrincipalId); + return (true, appId, objectId, servicePrincipalId, alreadyExisted: false); } catch (Exception ex) { logger.LogError(ex, "Failed to create agent blueprint: {Message}", ex.Message); - return (false, null, null, null); + return (false, null, null, null, alreadyExisted: false); } } @@ -980,6 +1119,7 @@ public static async Task CreateBlueprintClientSecretAsync( string generatedConfigPath, GraphApiService graphService, Models.Agent365Config setupConfig, + IConfigService configService, ILogger logger, CancellationToken ct = default) { @@ -1038,10 +1178,8 @@ public static async Task CreateBlueprintClientSecretAsync( setupConfig.AgentBlueprintClientSecret = protectedSecret; setupConfig.AgentBlueprintClientSecretProtected = isProtected; - await File.WriteAllTextAsync( - generatedConfigPath, - generatedConfig.ToJsonString(new JsonSerializerOptions { WriteIndented = true }), - ct); + // Use SaveStateAsync to preserve all existing dynamic properties (especially agentBlueprintServicePrincipalObjectId) + await configService.SaveStateAsync(setupConfig); logger.LogInformation("Client secret created successfully!"); logger.LogInformation($" - Secret stored in generated config (encrypted: {isProtected})"); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs index 2c1b5097..7d27a9df 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs @@ -193,7 +193,7 @@ await CreateInfrastructureImplementationAsync( #region Public Static Methods (Reusable by A365SetupRunner) - public static async Task CreateInfrastructureImplementationAsync( + public static async Task<(bool success, bool anyAlreadyExisted)> CreateInfrastructureImplementationAsync( ILogger logger, string configPath, string generatedConfigPath, @@ -206,7 +206,7 @@ public static async Task CreateInfrastructureImplementationAsync( if (!File.Exists(configPath)) { logger.LogError("Config file not found at {Path}", configPath); - return false; + return (false, false); } JsonObject cfg; @@ -217,7 +217,7 @@ public static async Task CreateInfrastructureImplementationAsync( catch (Exception ex) { logger.LogError(ex, "Failed to parse config JSON: {Path}", configPath); - return false; + return (false, false); } string Get(string name) => cfg.TryGetPropertyValue(name, out var node) && node is JsonValue jv && jv.TryGetValue(out string? s) ? s ?? string.Empty : string.Empty; @@ -244,7 +244,7 @@ public static async Task CreateInfrastructureImplementationAsync( logger.LogError( "Config missing required properties for Azure hosting. " + "Need subscriptionId, resourceGroup, appServicePlanName, webAppName, location."); - return false; + return (false, false); } } else @@ -288,7 +288,7 @@ public static async Task CreateInfrastructureImplementationAsync( if (!isValidated) { - return false; + return (false, false); } } else @@ -296,7 +296,7 @@ public static async Task CreateInfrastructureImplementationAsync( logger.LogInformation("==> Skipping Azure management authentication (--skipInfrastructure or External hosting)"); } - await CreateInfrastructureAsync( + var (principalId, anyAlreadyExisted) = await CreateInfrastructureAsync( commandExecutor, subscriptionId, tenantId, @@ -314,7 +314,7 @@ await CreateInfrastructureAsync( externalHosting, cancellationToken); - return true; + return (true, anyAlreadyExisted); } /// @@ -348,11 +348,11 @@ public static async Task ValidateAzureCliAuthenticationAsync( } else { - logger.LogInformation("Azure CLI already authenticated"); + logger.LogDebug("Azure CLI already authenticated"); } // Verify we have the management scope - logger.LogInformation("Verifying access to Azure management resources..."); + logger.LogDebug("Verifying access to Azure management resources..."); var tokenCheck = await executor.ExecuteAsync( "az", "account get-access-token --resource https://management.core.windows.net/ --query accessToken -o tsv", @@ -390,15 +390,13 @@ public static async Task ValidateAzureCliAuthenticationAsync( } else { - logger.LogInformation("Management scope token acquired successfully!"); + logger.LogDebug("Management scope token acquired successfully!"); } } else { - logger.LogInformation("Management scope verified successfully"); + logger.LogDebug("Management scope verified successfully"); } - - logger.LogInformation(""); return true; } @@ -406,8 +404,9 @@ public static async Task ValidateAzureCliAuthenticationAsync( /// Phase 1: Create Azure infrastructure (Resource Group, App Service Plan, Web App, Managed Identity) /// Equivalent to A365SetupRunner Phase 1 (lines 223-334) /// Returns the Managed Identity Principal ID (or null if not assigned) + /// and whether any infrastructure already existed (for idempotent summary reporting) /// - public static async Task CreateInfrastructureAsync( + public static async Task<(string? principalId, bool anyAlreadyExisted)> CreateInfrastructureAsync( CommandExecutor executor, string subscriptionId, string tenantId, @@ -425,6 +424,7 @@ public static async Task CreateInfrastructureAsync( bool externalHosting, CancellationToken cancellationToken = default) { + bool anyAlreadyExisted = false; string? principalId = null; JsonObject generatedConfig = new JsonObject(); @@ -469,6 +469,7 @@ public static async Task CreateInfrastructureAsync( } logger.LogInformation(""); + return (principalId, false); // Skip infra means nothing was created/modified } else { @@ -489,6 +490,7 @@ public static async Task CreateInfrastructureAsync( if (rgExists.Success && rgExists.StandardOutput.Trim().Equals("true", StringComparison.OrdinalIgnoreCase)) { logger.LogInformation("Resource group already exists: {RG} (skipping creation)", resourceGroup); + anyAlreadyExisted = true; } else { @@ -497,7 +499,11 @@ public static async Task CreateInfrastructureAsync( } // App Service plan - await EnsureAppServicePlanExistsAsync(executor, logger, resourceGroup, planName, planSku, location, subscriptionId); + bool planAlreadyExisted = await EnsureAppServicePlanExistsAsync(executor, logger, resourceGroup, planName, planSku, location, subscriptionId); + if (planAlreadyExisted) + { + anyAlreadyExisted = true; + } // Web App var webShow = await executor.ExecuteAsync("az", $"webapp show -g {resourceGroup} -n {webAppName} --subscription {subscriptionId}", captureOutput: true, suppressErrorLogging: true); @@ -559,6 +565,7 @@ public static async Task CreateInfrastructureAsync( } else { + anyAlreadyExisted = true; var linuxFxVersion = await GetLinuxFxVersionForPlatformAsync(platform, deploymentProjectPath, executor, logger, cancellationToken); logger.LogInformation("Web app already exists: {App} (skipping creation)", webAppName); logger.LogInformation("Configuring web app to use {Platform} runtime ({LinuxFxVersion})...", platform, linuxFxVersion); @@ -637,6 +644,8 @@ public static async Task CreateInfrastructureAsync( logger.LogInformation("Generated config updated with MSI principalId: {Id}", principalId); } } + + return (principalId, anyAlreadyExisted); } /// @@ -699,9 +708,10 @@ private static async Task AzWarnAsync(CommandExecutor executor, ILogger logger, } /// - /// Ensures that an App Service Plan exists, creating it if necessary and verifying its existence. + /// Ensures the App Service plan exists or creates it if missing. + /// Returns true if plan already existed, false if newly created. /// - internal static async Task EnsureAppServicePlanExistsAsync( + internal static async Task EnsureAppServicePlanExistsAsync( CommandExecutor executor, ILogger logger, string resourceGroup, @@ -716,6 +726,7 @@ internal static async Task EnsureAppServicePlanExistsAsync( if (planShow.Success) { logger.LogInformation("App Service plan already exists: {Plan} (skipping creation)", planName); + return true; // Already existed } else { @@ -819,6 +830,7 @@ internal static async Task EnsureAppServicePlanExistsAsync( $"Verification failed after {maxRetries} attempts. The plan may still be propagating in Azure."); } logger.LogInformation("App Service plan created and verified successfully: {Plan}", planName); + return false; // Newly created } } 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 3f164c02..dced44d1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -84,11 +84,13 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) logger.LogInformation("Completed Steps:"); if (results.InfrastructureCreated) { - logger.LogInformation(" [OK] Infrastructure created"); + var status = results.InfrastructureAlreadyExisted ? "configured (already exists)" : "created"; + logger.LogInformation(" [OK] Infrastructure {Status}", status); } if (results.BlueprintCreated) { - logger.LogInformation(" [OK] Agent blueprint created (Blueprint ID: {BlueprintId})", results.BlueprintId ?? "unknown"); + var status = results.BlueprintAlreadyExisted ? "configured (already exists)" : "created"; + logger.LogInformation(" [OK] Agent blueprint {Status} (Blueprint ID: {BlueprintId})", status, results.BlueprintId ?? "unknown"); } if (results.McpPermissionsConfigured) logger.LogInformation(" [OK] MCP server permissions configured"); @@ -97,7 +99,10 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) if (results.BotApiPermissionsConfigured) logger.LogInformation(" [OK] Messaging Bot API permissions configured"); if (results.MessagingEndpointRegistered) - logger.LogInformation(" [OK] Messaging endpoint registered"); + { + var status = results.EndpointAlreadyExisted ? "configured (already exists)" : "created"; + logger.LogInformation(" [OK] Messaging endpoint {Status}", status); + } // Show what failed if (results.Errors.Count > 0) @@ -148,7 +153,7 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) if (!results.MessagingEndpointRegistered) { logger.LogInformation(" - Messaging Endpoint: Run 'a365 setup blueprint --endpoint-only' to retry"); - logger.LogInformation(" Or delete conflicting endpoint first: a365 cleanup azure"); + logger.LogInformation(" Or delete conflicting endpoint first: a365 cleanup blueprint"); } } else if (results.HasWarnings) 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 cfd9e064..16a6e435 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -16,6 +16,15 @@ public class SetupResults public bool MessagingEndpointRegistered { get; set; } public bool InheritablePermissionsConfigured { get; set; } + // Idempotency tracking flags + public bool InfrastructureAlreadyExisted { get; set; } + public bool BlueprintAlreadyExisted { get; set; } + public bool BlueprintDiscoveredAndPersisted { get; set; } + public bool ServicePrincipalDiscovered { get; set; } + public bool FicAlreadyExisted { get; set; } + public bool SecretReused { get; set; } + public bool EndpointAlreadyExisted { get; set; } + public List Errors { get; } = new(); public List Warnings { get; } = new(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs index e31b8d3b..c1456deb 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/Agent365Config.cs @@ -323,6 +323,21 @@ public string BotName [JsonPropertyName("agentBlueprintId")] public string? AgentBlueprintId { get; set; } + /// + /// Azure AD object ID for the agent blueprint application. + /// Used as authoritative identifier for all blueprint operations to handle cases + /// where multiple blueprints may exist with the same display name. + /// + [JsonPropertyName("agentBlueprintObjectId")] + public string? AgentBlueprintObjectId { get; set; } + + /// + /// Azure AD object ID for the service principal associated with the agent blueprint. + /// Required for OAuth2 permission grants and inheritable permissions configuration. + /// + [JsonPropertyName("agentBlueprintServicePrincipalObjectId")] + public string? AgentBlueprintServicePrincipalObjectId { get; set; } + /// /// Azure AD application/identity ID for the agentic app. /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index 4bc5cbe4..b8b606b9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -72,6 +72,8 @@ static async Task Main(string[] args) var graphApiService = serviceProvider.GetRequiredService(); var agentPublishService = serviceProvider.GetRequiredService(); var agentBlueprintService = serviceProvider.GetRequiredService(); + var blueprintLookupService = serviceProvider.GetRequiredService(); + var federatedCredentialService = serviceProvider.GetRequiredService(); var webAppCreator = serviceProvider.GetRequiredService(); var platformDetector = serviceProvider.GetRequiredService(); var processService = serviceProvider.GetRequiredService(); @@ -81,7 +83,7 @@ static async Task Main(string[] args) rootCommand.AddCommand(DevelopCommand.CreateCommand(developLogger, configService, executor, authService, graphApiService, agentBlueprintService, processService)); rootCommand.AddCommand(DevelopMcpCommand.CreateCommand(developLogger, toolingService)); rootCommand.AddCommand(SetupCommand.CreateCommand(setupLogger, configService, executor, - deploymentService, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, agentBlueprintService, clientAppValidator)); + deploymentService, botConfigurator, azureValidator, webAppCreator, platformDetector, graphApiService, agentBlueprintService, blueprintLookupService, federatedCredentialService, clientAppValidator)); rootCommand.AddCommand(CreateInstanceCommand.CreateCommand(createInstanceLogger, configService, executor, botConfigurator, graphApiService, azureValidator)); rootCommand.AddCommand(DeployCommand.CreateCommand(deployLogger, configService, executor, @@ -217,6 +219,8 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); // For AgentApplication.Create permission services.AddSingleton(); // For publish command template extraction diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index 32641dee..f2bd8fce 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -620,6 +620,58 @@ public virtual async Task AddRequiredResourceAccessAsync( } } + /// + /// Get password credentials (client secrets) for an application. + /// Note: This only returns metadata (hint, displayName, expiration), not the actual secret values. + /// + /// The tenant ID for authentication + /// The application object ID + /// Cancellation token + /// List of password credential metadata + public async Task> GetPasswordCredentialsAsync( + string tenantId, + string applicationObjectId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Retrieving password credentials for application: {ObjectId}", applicationObjectId); + + var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/v1.0/applications/{applicationObjectId}", + cancellationToken); + + var credentials = new List(); + + if (doc != null && doc.RootElement.TryGetProperty("passwordCredentials", out var credsArray)) + { + foreach (var cred in credsArray.EnumerateArray()) + { + var displayName = cred.TryGetProperty("displayName", out var dn) ? dn.GetString() : null; + var hint = cred.TryGetProperty("hint", out var h) ? h.GetString() : null; + var keyId = cred.TryGetProperty("keyId", out var kid) ? kid.GetString() : null; + var endDateTime = cred.TryGetProperty("endDateTime", out var ed) ? ed.GetDateTime() : (DateTime?)null; + + credentials.Add(new PasswordCredentialInfo + { + DisplayName = displayName, + Hint = hint, + KeyId = keyId, + EndDateTime = endDateTime + }); + } + } + + return credentials; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to retrieve password credentials for application: {ObjectId}", applicationObjectId); + return new List(); + } + } + private async Task ResolveBlueprintObjectIdAsync( string tenantId, string blueprintAppId, @@ -677,3 +729,18 @@ private static List ParseInheritableScopesFromJson(JsonElement entry) return scopesList; } } + +/// +/// Information about a password credential (client secret). +/// Note: The actual secret value cannot be retrieved from Graph API. +/// +public class PasswordCredentialInfo +{ + public string? DisplayName { get; set; } + public string? Hint { get; set; } + public string? KeyId { get; set; } + public DateTime? EndDateTime { get; set; } + + public bool IsExpired => EndDateTime.HasValue && EndDateTime.Value < DateTime.UtcNow; + public bool IsExpiringSoon => EndDateTime.HasValue && EndDateTime.Value < DateTime.UtcNow.AddDays(30); +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs index 6c252b0e..c81b6094 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureAuthValidator.cs @@ -57,8 +57,8 @@ public async Task ValidateAuthenticationAsync(string? expectedSubscription var subscriptionName = root.GetProperty("name").GetString() ?? string.Empty; var userName = root.GetProperty("user").GetProperty("name").GetString() ?? string.Empty; - _logger.LogInformation("Azure CLI authenticated as: {UserName}", userName); - _logger.LogInformation(" Active subscription: {SubscriptionName} ({SubscriptionId})", + _logger.LogDebug("Azure CLI authenticated as: {UserName}", userName); + _logger.LogDebug(" Active subscription: {SubscriptionName} ({SubscriptionId})", subscriptionName, subscriptionId); // Validate subscription if specified @@ -76,7 +76,7 @@ public async Task ValidateAuthenticationAsync(string? expectedSubscription return false; } - _logger.LogInformation("Using correct subscription: {SubscriptionId}", expectedSubscriptionId); + _logger.LogDebug("Using correct subscription: {SubscriptionId}", expectedSubscriptionId); } return true; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs index 22b10138..2a15a48a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs @@ -62,21 +62,7 @@ private async Task ValidateAzureCliArchitectureAsync() // Check if Azure CLI is using 32-bit Python on 64-bit Windows if (result.StandardOutput.Contains("32 bit", StringComparison.OrdinalIgnoreCase)) { - _logger.LogWarning("Azure CLI Performance Notice"); - _logger.LogInformation(""); - _logger.LogInformation(" Azure CLI is using 32-bit Python on your 64-bit Windows system."); - _logger.LogInformation(" This may cause performance warnings during Azure operations."); - _logger.LogInformation(""); - _logger.LogInformation("To improve performance and eliminate warnings:"); - _logger.LogInformation(""); - _logger.LogInformation(" 1. Uninstall current Azure CLI:"); - _logger.LogInformation(" winget uninstall Microsoft.AzureCLI"); - _logger.LogInformation(""); - _logger.LogInformation(" 2. Install 64-bit version:"); - _logger.LogInformation(" winget install --exact --id Microsoft.AzureCLI"); - _logger.LogInformation(""); - _logger.LogInformation(" This will not affect functionality, only performance."); - _logger.LogInformation(""); + _logger.LogDebug("Azure CLI is using 32-bit Python (performance may be affected)"); } else if (result.StandardOutput.Contains("64 bit", StringComparison.OrdinalIgnoreCase)) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs index d89f5aae..eb6a0b54 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureValidator.cs @@ -37,7 +37,7 @@ public AzureValidator( /// public async Task ValidateAllAsync(string subscriptionId) { - _logger.LogInformation("Validating Azure CLI authentication and subscription..."); + _logger.LogDebug("Validating Azure CLI authentication and subscription..."); // Authentication validation (critical - stops execution if failed) if (!await _authValidator.ValidateAuthenticationAsync(subscriptionId)) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs new file mode 100644 index 00000000..bd13ffc5 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs @@ -0,0 +1,335 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Service for discovering and looking up agent blueprint applications and service principals. +/// Implements dual-path discovery: primary lookup by objectId, fallback to query by displayName. +/// +public class BlueprintLookupService +{ + private readonly ILogger _logger; + private readonly GraphApiService _graphApiService; + + public BlueprintLookupService(ILogger logger, GraphApiService graphApiService) + { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _graphApiService = graphApiService ?? throw new ArgumentNullException(nameof(graphApiService)); + } + + /// + /// Gets or sets the custom client app ID to use for Microsoft Graph authentication. + /// + public string? CustomClientAppId + { + get => _graphApiService.CustomClientAppId; + set => _graphApiService.CustomClientAppId = value; + } + + /// + /// Get blueprint application by object ID (primary path). + /// + /// The tenant ID for authentication + /// The blueprint application object ID + /// Cancellation token + /// Lookup result with blueprint details if found + public async Task GetApplicationByObjectIdAsync( + string tenantId, + string objectId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Looking up blueprint by objectId: {ObjectId}", objectId); + + var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/beta/applications/{objectId}", + cancellationToken); + + if (doc == null) + { + _logger.LogDebug("Blueprint not found with objectId: {ObjectId}", objectId); + return new BlueprintLookupResult + { + Found = false, + LookupMethod = "objectId" + }; + } + + var root = doc.RootElement; + var appId = root.GetProperty("appId").GetString(); + var displayName = root.GetProperty("displayName").GetString(); + + _logger.LogDebug("Found blueprint: {DisplayName} (ObjectId: {ObjectId}, AppId: {AppId})", + displayName, objectId, appId); + + return new BlueprintLookupResult + { + Found = true, + ObjectId = objectId, + AppId = appId, + DisplayName = displayName, + LookupMethod = "objectId" + }; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to look up blueprint by objectId: {ObjectId}", objectId); + return new BlueprintLookupResult + { + Found = false, + LookupMethod = "objectId", + ErrorMessage = ex.Message + }; + } + } + + /// + /// Get blueprint application by display name and sign-in audience (fallback path for migration). + /// + /// The tenant ID for authentication + /// The blueprint display name to search for + /// The sign-in audience (default: AzureADMultipleOrgs) + /// Cancellation token + /// Lookup result with blueprint details if found + public async Task GetApplicationByDisplayNameAsync( + string tenantId, + string displayName, + string signInAudience = "AzureADMultipleOrgs", + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Looking up blueprint by displayName: {DisplayName}", displayName); + + // Escape single quotes in displayName for OData filter + var escapedDisplayName = displayName.Replace("'", "''"); + var filter = $"displayName eq '{escapedDisplayName}' and signInAudience eq '{signInAudience}'"; + + var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/beta/applications?$filter={Uri.EscapeDataString(filter)}", + cancellationToken); + + if (doc == null) + { + _logger.LogDebug("No blueprints found with displayName: {DisplayName}", displayName); + return new BlueprintLookupResult + { + Found = false, + LookupMethod = "displayName" + }; + } + + var root = doc.RootElement; + if (!root.TryGetProperty("value", out var valueElement) || valueElement.GetArrayLength() == 0) + { + _logger.LogDebug("No blueprints found with displayName: {DisplayName}", displayName); + return new BlueprintLookupResult + { + Found = false, + LookupMethod = "displayName" + }; + } + + // Take first match (if multiple exist, log warning) + var firstMatch = valueElement[0]; + var objectId = firstMatch.GetProperty("id").GetString(); + var appId = firstMatch.GetProperty("appId").GetString(); + var foundDisplayName = firstMatch.GetProperty("displayName").GetString(); + + if (valueElement.GetArrayLength() > 1) + { + _logger.LogWarning("Multiple blueprints found with displayName '{DisplayName}'. Using first match: {ObjectId}", + displayName, objectId); + } + + _logger.LogDebug("Found blueprint: {DisplayName} (ObjectId: {ObjectId}, AppId: {AppId})", + foundDisplayName, objectId, appId); + + return new BlueprintLookupResult + { + Found = true, + ObjectId = objectId, + AppId = appId, + DisplayName = foundDisplayName, + LookupMethod = "displayName", + RequiresPersistence = true + }; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to look up blueprint by displayName: {DisplayName}", displayName); + return new BlueprintLookupResult + { + Found = false, + LookupMethod = "displayName", + ErrorMessage = ex.Message + }; + } + } + + /// + /// Get service principal by object ID (primary path). + /// + /// The tenant ID for authentication + /// The service principal object ID + /// Cancellation token + /// Lookup result with service principal details if found + public async Task GetServicePrincipalByObjectIdAsync( + string tenantId, + string objectId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Looking up service principal by objectId: {ObjectId}", objectId); + + var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/v1.0/servicePrincipals/{objectId}", + cancellationToken); + + if (doc == null) + { + _logger.LogDebug("Service principal not found with objectId: {ObjectId}", objectId); + return new ServicePrincipalLookupResult + { + Found = false, + LookupMethod = "objectId" + }; + } + + var root = doc.RootElement; + var appId = root.GetProperty("appId").GetString(); + var displayName = root.GetProperty("displayName").GetString(); + + _logger.LogDebug("Found service principal: {DisplayName} (ObjectId: {ObjectId}, AppId: {AppId})", + displayName, objectId, appId); + + return new ServicePrincipalLookupResult + { + Found = true, + ObjectId = objectId, + AppId = appId, + DisplayName = displayName, + LookupMethod = "objectId" + }; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to look up service principal by objectId: {ObjectId}", objectId); + return new ServicePrincipalLookupResult + { + Found = false, + LookupMethod = "objectId", + ErrorMessage = ex.Message + }; + } + } + + /// + /// Get service principal by app ID (fallback path for migration). + /// + /// The tenant ID for authentication + /// The application (client) ID to search for + /// Cancellation token + /// Lookup result with service principal details if found + public async Task GetServicePrincipalByAppIdAsync( + string tenantId, + string appId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Looking up service principal by appId: {AppId}", appId); + + var filter = $"appId eq '{appId}'"; + var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/v1.0/servicePrincipals?$filter={Uri.EscapeDataString(filter)}", + cancellationToken); + + if (doc == null) + { + _logger.LogDebug("No service principal found with appId: {AppId}", appId); + return new ServicePrincipalLookupResult + { + Found = false, + LookupMethod = "appId" + }; + } + + var root = doc.RootElement; + if (!root.TryGetProperty("value", out var valueElement) || valueElement.GetArrayLength() == 0) + { + _logger.LogDebug("No service principal found with appId: {AppId}", appId); + return new ServicePrincipalLookupResult + { + Found = false, + LookupMethod = "appId" + }; + } + + var firstMatch = valueElement[0]; + var objectId = firstMatch.GetProperty("id").GetString(); + var displayName = firstMatch.GetProperty("displayName").GetString(); + + _logger.LogDebug("Found service principal: {DisplayName} (ObjectId: {ObjectId}, AppId: {AppId})", + displayName, objectId, appId); + + return new ServicePrincipalLookupResult + { + Found = true, + ObjectId = objectId, + AppId = appId, + DisplayName = displayName, + LookupMethod = "appId", + RequiresPersistence = true + }; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Failed to look up service principal by appId: {AppId}", appId); + return new ServicePrincipalLookupResult + { + Found = false, + LookupMethod = "appId", + ErrorMessage = ex.Message + }; + } + } +} + +/// +/// Result of blueprint application lookup operation. +/// +public class BlueprintLookupResult +{ + public bool Found { get; set; } + public string? ObjectId { get; set; } + public string? AppId { get; set; } + public string? DisplayName { get; set; } + public string? LookupMethod { get; set; } + public bool RequiresPersistence { get; set; } + public string? ErrorMessage { get; set; } +} + +/// +/// Result of service principal lookup operation. +/// +public class ServicePrincipalLookupResult +{ + public bool Found { get; set; } + public string? ObjectId { get; set; } + public string? AppId { get; set; } + public string? DisplayName { get; set; } + public string? LookupMethod { get; set; } + public bool RequiresPersistence { get; set; } + public string? ErrorMessage { get; set; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs index 25d329b9..d7f9b875 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs @@ -19,6 +19,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; /// public class BotConfigurator : IBotConfigurator { + private const string AlreadyExistsErrorMessage = "already exists"; + private readonly ILogger _logger; private readonly CommandExecutor _executor; @@ -127,23 +129,28 @@ public async Task CreateEndpointWithAgentBlueprintAs if (!response.IsSuccessStatusCode) { - _logger.LogError("Failed to call create endpoint. Status: {Status}", response.StatusCode); - var errorContent = await response.Content.ReadAsStringAsync(); - // Only treat HTTP 409 Conflict as "already exists" success case - // InternalServerError (500) with "already exists" message is an actual failure - if (response.StatusCode == System.Net.HttpStatusCode.Conflict) + // Check for "already exists" condition in multiple forms: + // 1. HTTP 409 Conflict (standard REST pattern) + // 2. HTTP 500 InternalServerError with "already exists" in message body (Azure Bot Service pattern) + bool isAlreadyExists = response.StatusCode == System.Net.HttpStatusCode.Conflict || + (errorContent.Contains(AlreadyExistsErrorMessage, StringComparison.OrdinalIgnoreCase)); + + if (isAlreadyExists) { - _logger.LogWarning("Endpoint '{EndpointName}' already exists in the resource group", endpointName); - _logger.LogInformation("Endpoint registration completed (already exists)"); + _logger.LogWarning("Endpoint '{EndpointName}' {AlreadyExistsMessage} in the resource group", endpointName, AlreadyExistsErrorMessage); + _logger.LogInformation("Endpoint registration completed ({AlreadyExistsMessage})", AlreadyExistsErrorMessage); _logger.LogInformation(""); _logger.LogInformation("If you need to update the endpoint:"); - _logger.LogInformation(" 1. Delete existing endpoint: a365 cleanup azure"); + _logger.LogInformation(" 1. Delete existing endpoint: a365 cleanup blueprint"); _logger.LogInformation(" 2. Register new endpoint: a365 setup blueprint --endpoint-only"); return EndpointRegistrationResult.AlreadyExists; } + // Log error only for actual failures (not idempotent "already exists" scenarios) + _logger.LogError("Failed to call create endpoint. Status: {Status}", response.StatusCode); + if (errorContent.Contains("Failed to provision bot resource via Azure Management API. Status: BadRequest", StringComparison.OrdinalIgnoreCase)) { _logger.LogError("Please ensure that the Agent 365 CLI is supported in the selected region ('{Location}') and that your web app name ('{EndpointName}') is globally unique.", location, endpointName); @@ -154,7 +161,7 @@ public async Task CreateEndpointWithAgentBlueprintAs _logger.LogError(""); _logger.LogError("To resolve this issue:"); _logger.LogError(" 1. Check if endpoint exists: Review error details above"); - _logger.LogError(" 2. Delete conflicting endpoint: a365 cleanup azure"); + _logger.LogError(" 2. Delete conflicting endpoint: a365 cleanup blueprint"); _logger.LogError(" 3. Try registration again: a365 setup blueprint --endpoint-only"); return EndpointRegistrationResult.Failed; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs index 0ac28d32..fee5718b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs @@ -121,7 +121,7 @@ private async Task SyncConfigToGlobalDirectoryAsync(string fileName, strin // Write the config content to the global directory await File.WriteAllTextAsync(globalPath, content); - _logger?.LogInformation("Synced configuration to global directory: {Path}", globalPath); + _logger?.LogDebug("Synced configuration to global directory: {Path}", globalPath); return true; } catch (Exception ex) @@ -249,7 +249,7 @@ public async Task LoadAsync( var staticConfig = JsonSerializer.Deserialize(staticJson, DefaultJsonOptions) ?? throw new JsonException($"Failed to deserialize static configuration from {resolvedConfigPath}"); - _logger?.LogInformation("Loaded static configuration from: {ConfigPath}", resolvedConfigPath); + _logger?.LogDebug("Loaded static configuration from: {ConfigPath}", resolvedConfigPath); // Sync static config to global directory if loaded from current directory // This ensures portability - user can run CLI commands from any directory @@ -296,11 +296,11 @@ public async Task LoadAsync( // Merge dynamic properties into static config MergeDynamicProperties(staticConfig, stateData); - _logger?.LogInformation("Merged dynamic state from: {StatePath}", actualStatePath); + _logger?.LogDebug("Merged dynamic state from: {StatePath}", actualStatePath); } else { - _logger?.LogInformation("No dynamic state file found at: {StatePath}", resolvedStatePath); + _logger?.LogDebug("No dynamic state file found at: {StatePath}", resolvedStatePath); } // Validate the merged configuration @@ -357,7 +357,7 @@ public async Task SaveStateAsync( { // Save the state to the local current directory await File.WriteAllTextAsync(currentDirPath, json); - _logger?.LogInformation("Saved dynamic state to: {StatePath}", currentDirPath); + _logger?.LogDebug("Saved dynamic state to: {StatePath}", currentDirPath); } catch (Exception ex) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs new file mode 100644 index 00000000..6a1a63cd --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs @@ -0,0 +1,318 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; +using Microsoft.Extensions.Logging; + +namespace Microsoft.Agents.A365.DevTools.Cli.Services; + +/// +/// Service for managing federated identity credentials for agent blueprint applications. +/// Handles checking existing FICs and creating new ones with idempotency. +/// +public class FederatedCredentialService +{ + private readonly ILogger _logger; + private readonly GraphApiService _graphApiService; + + public FederatedCredentialService(ILogger logger, GraphApiService graphApiService) + { + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _graphApiService = graphApiService ?? throw new ArgumentNullException(nameof(graphApiService)); + } + + /// + /// Gets or sets the custom client app ID to use for Microsoft Graph authentication. + /// + public string? CustomClientAppId + { + get => _graphApiService.CustomClientAppId; + set => _graphApiService.CustomClientAppId = value; + } + + /// + /// Get all federated credentials for a blueprint application. + /// + /// The tenant ID for authentication + /// The blueprint application object ID + /// Cancellation token + /// List of federated credentials + public async Task> GetFederatedCredentialsAsync( + string tenantId, + string blueprintObjectId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Retrieving federated credentials for blueprint: {ObjectId}", blueprintObjectId); + + var doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/beta/applications/{blueprintObjectId}/federatedIdentityCredentials", + cancellationToken); + + if (doc == null) + { + _logger.LogDebug("No federated credentials found for blueprint: {ObjectId}", blueprintObjectId); + return new List(); + } + + var root = doc.RootElement; + if (!root.TryGetProperty("value", out var valueElement)) + { + return new List(); + } + + var credentials = new List(); + foreach (var item in valueElement.EnumerateArray()) + { + var id = item.GetProperty("id").GetString(); + var name = item.GetProperty("name").GetString(); + var issuer = item.GetProperty("issuer").GetString(); + var subject = item.GetProperty("subject").GetString(); + + var audiences = new List(); + if (item.TryGetProperty("audiences", out var audiencesElement)) + { + foreach (var audience in audiencesElement.EnumerateArray()) + { + audiences.Add(audience.GetString() ?? string.Empty); + } + } + + credentials.Add(new FederatedCredentialInfo + { + Id = id, + Name = name, + Issuer = issuer, + Subject = subject, + Audiences = audiences + }); + } + + _logger.LogDebug("Found {Count} federated credential(s) for blueprint: {ObjectId}", + credentials.Count, blueprintObjectId); + + return credentials; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to retrieve federated credentials for blueprint: {ObjectId}", blueprintObjectId); + return new List(); + } + } + + /// + /// Check if a federated credential exists with matching subject and issuer. + /// + /// The tenant ID for authentication + /// The blueprint application object ID + /// The subject to match (typically MSI principal ID) + /// The issuer to match + /// Cancellation token + /// True if matching credential exists, false otherwise + public async Task CheckFederatedCredentialExistsAsync( + string tenantId, + string blueprintObjectId, + string subject, + string issuer, + CancellationToken cancellationToken = default) + { + try + { + var credentials = await GetFederatedCredentialsAsync(tenantId, blueprintObjectId, cancellationToken); + + var match = credentials.FirstOrDefault(c => + string.Equals(c.Subject, subject, StringComparison.OrdinalIgnoreCase) && + string.Equals(c.Issuer, issuer, StringComparison.OrdinalIgnoreCase)); + + if (match != null) + { + _logger.LogDebug("Found existing federated credential: {Name} (Subject: {Subject})", + match.Name, subject); + + return new FederatedCredentialCheckResult + { + Exists = true, + ExistingCredential = match + }; + } + + _logger.LogDebug("No existing federated credential found with subject: {Subject}", subject); + return new FederatedCredentialCheckResult + { + Exists = false + }; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to check federated credential existence"); + return new FederatedCredentialCheckResult + { + Exists = false, + ErrorMessage = ex.Message + }; + } + } + + /// + /// Create a new federated identity credential for a blueprint. + /// Handles HTTP 409 (already exists) as a success case. + /// + /// The tenant ID for authentication + /// The blueprint application object ID + /// The name for the federated credential + /// The issuer URL + /// The subject (typically MSI principal ID) + /// List of audiences (typically ["api://AzureADTokenExchange"]) + /// Cancellation token + /// Result indicating success or failure + public async Task CreateFederatedCredentialAsync( + string tenantId, + string blueprintObjectId, + string name, + string issuer, + string subject, + List audiences, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Creating federated credential: {Name} for blueprint: {ObjectId}", + name, blueprintObjectId); + + var payload = new + { + name, + issuer, + subject, + audiences + }; + + var payloadJson = JsonSerializer.Serialize(payload); + + // Try the standard endpoint first + var endpoint = $"/beta/applications/{blueprintObjectId}/federatedIdentityCredentials"; + + var response = await _graphApiService.GraphPostWithResponseAsync( + tenantId, + endpoint, + payloadJson, + cancellationToken); + + if (response.IsSuccess) + { + _logger.LogInformation("Successfully created federated credential: {Name}", name); + return new FederatedCredentialCreateResult + { + Success = true, + AlreadyExisted = false + }; + } + + // Check for HTTP 409 (Conflict) - credential already exists + if (response.StatusCode == 409) + { + _logger.LogDebug("Federated credential already exists (HTTP 409): {Name}", name); + return new FederatedCredentialCreateResult + { + Success = true, + AlreadyExisted = true + }; + } + + // Log error details from standard endpoint + _logger.LogWarning("Standard endpoint failed: HTTP {StatusCode} {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); + if (!string.IsNullOrWhiteSpace(response.Body)) + { + _logger.LogDebug("Response body: {Body}", response.Body); + } + + // Try fallback endpoint for agent blueprint + _logger.LogDebug("Trying fallback endpoint for agent blueprint federated credential"); + endpoint = $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{blueprintObjectId}/federatedIdentityCredentials"; + + response = await _graphApiService.GraphPostWithResponseAsync( + tenantId, + endpoint, + payloadJson, + cancellationToken); + + if (response.IsSuccess) + { + _logger.LogInformation("Successfully created federated credential using fallback endpoint: {Name}", name); + return new FederatedCredentialCreateResult + { + Success = true, + AlreadyExisted = false + }; + } + + // Check for HTTP 409 (Conflict) - credential already exists + if (response.StatusCode == 409) + { + _logger.LogDebug("Federated credential already exists (HTTP 409) on fallback endpoint: {Name}", name); + return new FederatedCredentialCreateResult + { + Success = true, + AlreadyExisted = true + }; + } + + // Log error details from fallback endpoint + _logger.LogError("Fallback endpoint also failed: HTTP {StatusCode} {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); + if (!string.IsNullOrWhiteSpace(response.Body)) + { + _logger.LogDebug("Response body: {Body}", response.Body); + } + + _logger.LogError("Failed to create federated credential: {Name}. Both standard and fallback endpoints returned errors.", name); + return new FederatedCredentialCreateResult + { + Success = false, + ErrorMessage = $"HTTP {response.StatusCode}: {response.ReasonPhrase}" + }; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to create federated credential: {Name}", name); + return new FederatedCredentialCreateResult + { + Success = false, + ErrorMessage = ex.Message + }; + } + } +} + +/// +/// Information about a federated identity credential. +/// +public class FederatedCredentialInfo +{ + public string? Id { get; set; } + public string? Name { get; set; } + public string? Issuer { get; set; } + public string? Subject { get; set; } + public List Audiences { get; set; } = new(); +} + +/// +/// Result of checking if a federated credential exists. +/// +public class FederatedCredentialCheckResult +{ + public bool Exists { get; set; } + public FederatedCredentialInfo? ExistingCredential { get; set; } + public string? ErrorMessage { get; set; } +} + +/// +/// Result of creating a federated credential. +/// +public class FederatedCredentialCreateResult +{ + public bool Success { get; set; } + public bool AlreadyExisted { get; set; } + public string? ErrorMessage { get; set; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index 99cf1495..f336660a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -220,7 +220,7 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo return JsonDocument.Parse(json); } - public async Task GraphPostAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) + public virtual async Task GraphPostAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) { if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) return null; var url = relativePath.StartsWith("http", StringComparison.OrdinalIgnoreCase) @@ -237,7 +237,7 @@ private async Task EnsureGraphHeadersAsync(string tenantId, CancellationTo /// /// POST to Graph but always return HTTP response details (status, body, parsed JSON) /// - public async Task GraphPostWithResponseAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) + public virtual async Task GraphPostWithResponseAsync(string tenantId, string relativePath, object payload, CancellationToken ct = default, IEnumerable? scopes = null) { if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) { diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs index 822285f8..52d9cb6c 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs @@ -30,6 +30,8 @@ public class BlueprintSubcommandTests private readonly GraphApiService _mockGraphApiService; private readonly AgentBlueprintService _mockBlueprintService; private readonly IClientAppValidator _mockClientAppValidator; + private readonly BlueprintLookupService _mockBlueprintLookupService; + private readonly FederatedCredentialService _mockFederatedCredentialService; public BlueprintSubcommandTests() { @@ -45,6 +47,8 @@ public BlueprintSubcommandTests() _mockGraphApiService = Substitute.ForPartsOf(Substitute.For>(), _mockExecutor); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); _mockClientAppValidator = Substitute.For(); + _mockBlueprintLookupService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); + _mockFederatedCredentialService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); } [Fact] @@ -59,7 +63,7 @@ public void CreateCommand_ShouldHaveCorrectName() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert command.Name.Should().Be("blueprint"); @@ -77,7 +81,7 @@ public void CreateCommand_ShouldHaveDescription() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert command.Description.Should().NotBeNullOrEmpty(); @@ -96,7 +100,7 @@ public void CreateCommand_ShouldHaveConfigOption() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert var configOption = command.Options.FirstOrDefault(o => o.Name == "config"); @@ -117,7 +121,7 @@ public void CreateCommand_ShouldHaveVerboseOption() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert var verboseOption = command.Options.FirstOrDefault(o => o.Name == "verbose"); @@ -138,7 +142,7 @@ public void CreateCommand_ShouldHaveDryRunOption() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert var dryRunOption = command.Options.FirstOrDefault(o => o.Name == "dry-run"); @@ -167,7 +171,7 @@ public async Task DryRun_ShouldLoadConfigAndNotExecute() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -202,7 +206,7 @@ public async Task DryRun_ShouldDisplayBlueprintInformation() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -255,7 +259,7 @@ public async Task CreateBlueprintImplementation_WithMissingDisplayName_ShouldThr _mockConfigService, _mockBotConfigurator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert - Should return false when consent service fails result.Should().NotBeNull(); @@ -292,7 +296,7 @@ public async Task CreateBlueprintImplementation_WithAzureValidationFailure_Shoul _mockConfigService, _mockBotConfigurator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert result.Should().NotBeNull(); @@ -313,7 +317,7 @@ public void CommandDescription_ShouldMentionRequiredPermissions() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert command.Description.Should().Contain("Agent ID Developer"); @@ -341,7 +345,7 @@ public async Task DryRun_WithCustomConfigPath_ShouldLoadCorrectFile() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -377,7 +381,7 @@ public async Task DryRun_ShouldNotCreateServicePrincipal() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -405,7 +409,7 @@ public void CreateCommand_ShouldHandleAllOptions() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert - Verify all expected options are present command.Options.Should().HaveCountGreaterOrEqualTo(3); @@ -431,7 +435,7 @@ public async Task DryRun_WithMissingConfig_ShouldHandleGracefully() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -453,7 +457,7 @@ public void CreateCommand_DefaultConfigPath_ShouldBeA365ConfigJson() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert - Verify the config option exists and has expected aliases var configOption = command.Options.First(o => o.Name == "config"); @@ -490,7 +494,7 @@ public async Task CreateBlueprintImplementation_ShouldLogProgressMessages() _mockConfigService, _mockBotConfigurator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert result.Should().NotBeNull(); @@ -518,7 +522,7 @@ public void CommandDescription_ShouldBeInformativeAndActionable() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert - Verify description provides context and guidance command.Description.Should().NotBeNullOrEmpty(); @@ -546,7 +550,7 @@ public async Task DryRun_WithVerboseFlag_ShouldSucceed() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -580,7 +584,7 @@ public async Task DryRun_ShouldShowWhatWouldBeDone() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -612,7 +616,7 @@ public void CreateCommand_ShouldBeUsableInCommandPipeline() _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); // Assert - Verify command can be added to a parser var parser = new CommandLineBuilder(command).Build(); 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 91767ff9..b96d3af8 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 @@ -30,6 +30,8 @@ public class SetupCommandTests private readonly GraphApiService _mockGraphApiService; private readonly AgentBlueprintService _mockBlueprintService; private readonly IClientAppValidator _mockClientAppValidator; + private readonly BlueprintLookupService _mockBlueprintLookupService; + private readonly FederatedCredentialService _mockFederatedCredentialService; public SetupCommandTests() { @@ -56,6 +58,8 @@ public SetupCommandTests() _mockGraphApiService = Substitute.For(); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); _mockClientAppValidator = Substitute.For(); + _mockBlueprintLookupService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); + _mockFederatedCredentialService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); } [Fact] @@ -85,7 +89,7 @@ public async Task SetupAllCommand_DryRun_ValidConfig_OnlyValidatesConfig() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -132,7 +136,7 @@ public async Task SetupAllCommand_SkipInfrastructure_SkipsInfrastructureStep() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -160,7 +164,7 @@ public void SetupCommand_HasRequiredSubcommands() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); // Assert - Verify all required subcommands exist var subcommandNames = command.Subcommands.Select(c => c.Name).ToList(); @@ -185,7 +189,7 @@ public void SetupCommand_PermissionsSubcommand_HasMcpAndBotSubcommands() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var permissionsCmd = command.Subcommands.FirstOrDefault(c => c.Name == "permissions"); @@ -213,7 +217,7 @@ public void SetupCommand_ErrorMessages_ShouldBeInformativeAndActionable() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); // Assert - Command structure should support clear error messaging command.Should().NotBeNull(); @@ -259,7 +263,7 @@ public async Task InfrastructureSubcommand_DryRun_CompletesSuccessfully() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -302,7 +306,7 @@ public async Task BlueprintSubcommand_DryRun_CompletesSuccessfully() _mockAzureValidator, _mockWebAppCreator, _mockPlatformDetector, - _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator); + _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -346,7 +350,7 @@ public async Task RequirementsSubcommand_ValidConfig_CompletesSuccessfully() _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, - _mockClientAppValidator); + _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); @@ -390,7 +394,7 @@ public async Task RequirementsSubcommand_WithCategoryFilter_RunsFilteredChecks() _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, - _mockClientAppValidator); + _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); var parser = new CommandLineBuilder(command).Build(); var testConsole = new TestConsole(); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs new file mode 100644 index 00000000..e3afc6e9 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs @@ -0,0 +1,283 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using NSubstitute; +using System.Text.Json; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +public class BlueprintLookupServiceTests +{ + private readonly ILogger _logger; + private readonly GraphApiService _graphApiService; + private readonly BlueprintLookupService _service; + private const string TestTenantId = "12345678-1234-1234-1234-123456789012"; + private const string TestObjectId = "87654321-4321-4321-4321-210987654321"; + private const string TestAppId = "11111111-1111-1111-1111-111111111111"; + private const string TestDisplayName = "Test Blueprint"; + + public BlueprintLookupServiceTests() + { + _logger = Substitute.For>(); + _graphApiService = Substitute.For(); + _service = new BlueprintLookupService(_logger, _graphApiService); + } + + [Fact] + public async Task GetApplicationByObjectIdAsync_WhenBlueprintExists_ReturnsFoundWithDetails() + { + // Arrange + var jsonResponse = $@"{{ + ""id"": ""{TestObjectId}"", + ""appId"": ""{TestAppId}"", + ""displayName"": ""{TestDisplayName}"" + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestObjectId}", + Arg.Any(), + null) + .Returns(jsonDoc); + + // Act + var result = await _service.GetApplicationByObjectIdAsync(TestTenantId, TestObjectId); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeTrue(); + result.ObjectId.Should().Be(TestObjectId); + result.AppId.Should().Be(TestAppId); + result.DisplayName.Should().Be(TestDisplayName); + result.LookupMethod.Should().Be("objectId"); + result.RequiresPersistence.Should().BeFalse(); // objectId lookup doesn't require persistence + } + + [Fact] + public async Task GetApplicationByObjectIdAsync_WhenBlueprintNotFound_ReturnsNotFound() + { + // Arrange + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestObjectId}", + Arg.Any()) + .Returns((JsonDocument?)null); + + // Act + var result = await _service.GetApplicationByObjectIdAsync(TestTenantId, TestObjectId); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeFalse(); + result.LookupMethod.Should().Be("objectId"); + } + + [Fact] + public async Task GetApplicationByDisplayNameAsync_WhenBlueprintExists_ReturnsFoundWithRequiresPersistence() + { + // Arrange + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""{TestObjectId}"", + ""appId"": ""{TestAppId}"", + ""displayName"": ""{TestDisplayName}"" + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + Arg.Is(s => s.Contains("/beta/applications?$filter=")), + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.GetApplicationByDisplayNameAsync(TestTenantId, TestDisplayName); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeTrue(); + result.ObjectId.Should().Be(TestObjectId); + result.AppId.Should().Be(TestAppId); + result.DisplayName.Should().Be(TestDisplayName); + result.LookupMethod.Should().Be("displayName"); + result.RequiresPersistence.Should().BeTrue(); // displayName lookup requires persistence for migration + } + + [Fact] + public async Task GetApplicationByDisplayNameAsync_WhenNoBlueprintsFound_ReturnsNotFound() + { + // Arrange + var jsonResponse = @"{""value"": []}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + Arg.Is(s => s.Contains("/beta/applications?$filter=")), + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.GetApplicationByDisplayNameAsync(TestTenantId, TestDisplayName); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeFalse(); + result.LookupMethod.Should().Be("displayName"); + result.RequiresPersistence.Should().BeFalse(); + } + + [Fact] + public async Task GetApplicationByDisplayNameAsync_EscapesSingleQuotes() + { + // Arrange + var displayNameWithQuotes = "Test'Blueprint'Name"; + var jsonResponse = @"{""value"": []}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + Arg.Is(s => s.Contains("Test%27%27Blueprint%27%27Name")), // URL encoded double single quotes + Arg.Any(), + null) + .Returns(jsonDoc); + + // Act + var result = await _service.GetApplicationByDisplayNameAsync(TestTenantId, displayNameWithQuotes); + + // Assert + await _graphApiService.Received(1).GraphGetAsync( + TestTenantId, + Arg.Is(s => s.Contains("Test%27%27Blueprint%27%27Name")), + Arg.Any(), + null); + } + + [Fact] + public async Task GetServicePrincipalByAppIdAsync_WhenSPExists_ReturnsFoundWithDetails() + { + // Arrange + var spObjectId = "22222222-2222-2222-2222-222222222222"; + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""{spObjectId}"", + ""appId"": ""{TestAppId}"", + ""displayName"": ""Test SP"" + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + // The filter will be URL-escaped: "appId eq '...'" becomes "appId%20eq%20%27...%27" + _graphApiService.GraphGetAsync( + TestTenantId, + Arg.Is(s => s.Contains($"appId%20eq%20%27{TestAppId}%27")), + Arg.Any(), + null) + .Returns(jsonDoc); + + // Act + var result = await _service.GetServicePrincipalByAppIdAsync(TestTenantId, TestAppId); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeTrue(); + result.ObjectId.Should().Be(spObjectId); + result.AppId.Should().Be(TestAppId); + result.LookupMethod.Should().Be("appId"); + } + + [Fact] + public async Task GetServicePrincipalByObjectIdAsync_WhenSPExists_ReturnsFoundWithDetails() + { + // Arrange + var spObjectId = "33333333-3333-3333-3333-333333333333"; + var jsonResponse = $@"{{ + ""id"": ""{spObjectId}"", + ""appId"": ""{TestAppId}"", + ""displayName"": ""Test SP"" + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/v1.0/servicePrincipals/{spObjectId}", + Arg.Any(), + null) + .Returns(jsonDoc); + + // Act + var result = await _service.GetServicePrincipalByObjectIdAsync(TestTenantId, spObjectId); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeTrue(); + result.ObjectId.Should().Be(spObjectId); + result.AppId.Should().Be(TestAppId); + result.LookupMethod.Should().Be("objectId"); + } + + [Fact] + public async Task GetApplicationByObjectIdAsync_OnException_ReturnsNotFoundWithError() + { + // Arrange + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestObjectId}", + Arg.Any()) + .Returns(Task.FromException(new Exception("Graph API error"))); + + // Act + var result = await _service.GetApplicationByObjectIdAsync(TestTenantId, TestObjectId); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeFalse(); + result.ErrorMessage.Should().Contain("Graph API error"); + } + + [Fact] + public async Task GetApplicationByDisplayNameAsync_WhenMultipleBlueprintsFound_ReturnsFirst() + { + // Arrange - Simulate multiple results (shouldn't happen with proper naming, but test resilience) + var objectId1 = "44444444-4444-4444-4444-444444444444"; + var objectId2 = "55555555-5555-5555-5555-555555555555"; + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""{objectId1}"", + ""appId"": ""{TestAppId}"", + ""displayName"": ""{TestDisplayName}"" + }}, + {{ + ""id"": ""{objectId2}"", + ""appId"": ""66666666-6666-6666-6666-666666666666"", + ""displayName"": ""{TestDisplayName}"" + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + Arg.Is(s => s.Contains("/beta/applications?$filter=")), + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.GetApplicationByDisplayNameAsync(TestTenantId, TestDisplayName); + + // Assert + result.Should().NotBeNull(); + result.Found.Should().BeTrue(); + result.ObjectId.Should().Be(objectId1); // Should return the first match + } +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs new file mode 100644 index 00000000..b4167b23 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs @@ -0,0 +1,404 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using System.Text.Json; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +public class FederatedCredentialServiceTests +{ + private readonly ILogger _logger; + private readonly GraphApiService _graphApiService; + private readonly FederatedCredentialService _service; + private const string TestTenantId = "12345678-1234-1234-1234-123456789012"; + private const string TestBlueprintObjectId = "87654321-4321-4321-4321-210987654321"; + private const string TestMsiPrincipalId = "11111111-1111-1111-1111-111111111111"; + private const string TestIssuer = "https://login.microsoftonline.com/12345678-1234-1234-1234-123456789012/v2.0"; + private const string TestCredentialName = "TestCredential-MSI"; + + public FederatedCredentialServiceTests() + { + _logger = Substitute.For>(); + _graphApiService = Substitute.For(); + _service = new FederatedCredentialService(_logger, _graphApiService); + } + + [Fact] + public async Task GetFederatedCredentialsAsync_WhenCredentialsExist_ReturnsListOfCredentials() + { + // Arrange + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""cred-id-1"", + ""name"": ""Credential1"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""{TestMsiPrincipalId}"", + ""audiences"": [""api://AzureADTokenExchange""] + }}, + {{ + ""id"": ""cred-id-2"", + ""name"": ""Credential2"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""different-principal"", + ""audiences"": [""api://AzureADTokenExchange""] + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.GetFederatedCredentialsAsync(TestTenantId, TestBlueprintObjectId); + + // Assert + result.Should().NotBeNull(); + result.Should().HaveCount(2); + result[0].Name.Should().Be("Credential1"); + result[0].Subject.Should().Be(TestMsiPrincipalId); + result[1].Name.Should().Be("Credential2"); + } + + [Fact] + public async Task GetFederatedCredentialsAsync_WhenNoCredentials_ReturnsEmptyList() + { + // Arrange + var jsonResponse = @"{""value"": []}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.GetFederatedCredentialsAsync(TestTenantId, TestBlueprintObjectId); + + // Assert + result.Should().NotBeNull(); + result.Should().BeEmpty(); + } + + [Fact] + public async Task CheckFederatedCredentialExistsAsync_WhenMatchingCredentialExists_ReturnsTrue() + { + // Arrange + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""cred-id-1"", + ""name"": ""Credential1"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""{TestMsiPrincipalId}"", + ""audiences"": [""api://AzureADTokenExchange""] + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.CheckFederatedCredentialExistsAsync( + TestTenantId, + TestBlueprintObjectId, + TestMsiPrincipalId, + TestIssuer); + + // Assert + result.Should().NotBeNull(); + result.Exists.Should().BeTrue(); + result.ExistingCredential.Should().NotBeNull(); + result.ExistingCredential!.Name.Should().Be("Credential1"); + result.ExistingCredential.Subject.Should().Be(TestMsiPrincipalId); + } + + [Fact] + public async Task CheckFederatedCredentialExistsAsync_WhenNoMatchingCredential_ReturnsFalse() + { + // Arrange + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""cred-id-1"", + ""name"": ""Credential1"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""different-principal"", + ""audiences"": [""api://AzureADTokenExchange""] + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.CheckFederatedCredentialExistsAsync( + TestTenantId, + TestBlueprintObjectId, + TestMsiPrincipalId, + TestIssuer); + + // Assert + result.Should().NotBeNull(); + result.Exists.Should().BeFalse(); + result.ExistingCredential.Should().BeNull(); + } + + [Fact] + public async Task CheckFederatedCredentialExistsAsync_IsCaseInsensitive() + { + // Arrange + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""cred-id-1"", + ""name"": ""Credential1"", + ""issuer"": ""{TestIssuer.ToLower()}"", + ""subject"": ""{TestMsiPrincipalId.ToUpper()}"", + ""audiences"": [""api://AzureADTokenExchange""] + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Returns(jsonDoc); + + // Act - Pass in different casing + var result = await _service.CheckFederatedCredentialExistsAsync( + TestTenantId, + TestBlueprintObjectId, + TestMsiPrincipalId.ToLower(), + TestIssuer.ToUpper()); + + // Assert + result.Should().NotBeNull(); + result.Exists.Should().BeTrue(); + } + + [Fact] + public async Task CreateFederatedCredentialAsync_WhenSuccessful_ReturnsSuccess() + { + // Arrange + var jsonResponse = $@"{{ + ""id"": ""cred-id-new"", + ""name"": ""{TestCredentialName}"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""{TestMsiPrincipalId}"" + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + var successResponse = new GraphApiService.GraphResponse + { + IsSuccess = true, + StatusCode = 200, + ReasonPhrase = "OK", + Body = jsonResponse, + Json = jsonDoc + }; + + _graphApiService.GraphPostWithResponseAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(successResponse); + + // Act + var result = await _service.CreateFederatedCredentialAsync( + TestTenantId, + TestBlueprintObjectId, + TestCredentialName, + TestIssuer, + TestMsiPrincipalId, + new List { "api://AzureADTokenExchange" }); + + // Assert + result.Should().NotBeNull(); + result.Success.Should().BeTrue(); + result.AlreadyExisted.Should().BeFalse(); + result.ErrorMessage.Should().BeNull(); + } + + [Fact] + public async Task CreateFederatedCredentialAsync_WhenHttp409Conflict_ReturnsSuccessWithAlreadyExisted() + { + // Arrange - Return HTTP 409 Conflict + var conflictResponse = new GraphApiService.GraphResponse + { + IsSuccess = false, + StatusCode = 409, + ReasonPhrase = "Conflict", + Body = @"{""error"": {""code"": ""Request_ResourceExists"", ""message"": ""Resource already exists""}}", + Json = null + }; + + _graphApiService.GraphPostWithResponseAsync( + TestTenantId, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(conflictResponse); + + // Act + var result = await _service.CreateFederatedCredentialAsync( + TestTenantId, + TestBlueprintObjectId, + TestCredentialName, + TestIssuer, + TestMsiPrincipalId, + new List { "api://AzureADTokenExchange" }); + + // Assert + result.Should().NotBeNull(); + result.Success.Should().BeTrue(); // 409 is treated as success + result.AlreadyExisted.Should().BeTrue(); + } + + [Fact] + public async Task CreateFederatedCredentialAsync_WhenStandardEndpointFails_TriesFallbackEndpoint() + { + // Arrange + var standardEndpoint = $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials"; + var fallbackEndpoint = $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{TestBlueprintObjectId}/federatedIdentityCredentials"; + + // Standard endpoint returns failure (e.g., HTTP 400) + var standardFailureResponse = new GraphApiService.GraphResponse + { + IsSuccess = false, + StatusCode = 400, + ReasonPhrase = "Bad Request", + Body = @"{""error"": {""message"": ""Agent Blueprints not supported on this endpoint""}}", + Json = null + }; + + _graphApiService.GraphPostWithResponseAsync( + TestTenantId, + standardEndpoint, + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(standardFailureResponse); + + // Fallback endpoint succeeds + var jsonResponse = $@"{{ + ""id"": ""cred-id-new"", + ""name"": ""{TestCredentialName}"" + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + var fallbackSuccessResponse = new GraphApiService.GraphResponse + { + IsSuccess = true, + StatusCode = 200, + ReasonPhrase = "OK", + Body = jsonResponse, + Json = jsonDoc + }; + + _graphApiService.GraphPostWithResponseAsync( + TestTenantId, + fallbackEndpoint, + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Returns(fallbackSuccessResponse); + + // Act + var result = await _service.CreateFederatedCredentialAsync( + TestTenantId, + TestBlueprintObjectId, + TestCredentialName, + TestIssuer, + TestMsiPrincipalId, + new List { "api://AzureADTokenExchange" }); + + // Assert + result.Should().NotBeNull(); + result.Success.Should().BeTrue(); + + // Verify both endpoints were called + await _graphApiService.Received(1).GraphPostWithResponseAsync( + TestTenantId, + standardEndpoint, + Arg.Any(), + Arg.Any(), + Arg.Any?>()); + + await _graphApiService.Received(1).GraphPostWithResponseAsync( + TestTenantId, + fallbackEndpoint, + Arg.Any(), + Arg.Any(), + Arg.Any?>()); + } + + [Fact] + public async Task CreateFederatedCredentialAsync_WhenBothEndpointsFail_ReturnsFailure() + { + // Arrange + _graphApiService.GraphPostWithResponseAsync( + TestTenantId, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any?>()) + .Throws(new Exception("General API failure")); + + // Act + var result = await _service.CreateFederatedCredentialAsync( + TestTenantId, + TestBlueprintObjectId, + TestCredentialName, + TestIssuer, + TestMsiPrincipalId, + new List { "api://AzureADTokenExchange" }); + + // Assert + result.Should().NotBeNull(); + result.Success.Should().BeFalse(); + result.ErrorMessage.Should().Contain("General API failure"); + } + + [Fact] + public async Task GetFederatedCredentialsAsync_OnException_ReturnsEmptyList() + { + // Arrange + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Throws(new Exception("Network error")); + + // Act + var result = await _service.GetFederatedCredentialsAsync(TestTenantId, TestBlueprintObjectId); + + // Assert + result.Should().NotBeNull(); + result.Should().BeEmpty(); + } +} From 3633ebe13e68f2e04a02c570bd8849992ecd1db5 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Tue, 6 Jan 2026 18:08:45 -0800 Subject: [PATCH 2/4] Improve federated credential and consent handling in CLI - Cleanup now deletes federated credentials before blueprint removal, using new FederatedCredentialService methods with endpoint fallback. - FederatedCredentialService enhanced for robust listing, creation, deletion, and error handling; skips malformed entries and supports propagation retries. - Blueprint setup refactored to consistently handle federated credential creation/validation and admin consent, with consent checked before prompting. - Added AdminConsentHelper.CheckConsentExistsAsync to verify existing grants. - Improved idempotency tracking and summary logging for permissions. - Updated CleanupCommand and tests to require/use FederatedCredentialService. - Expanded tests for federated credential and consent logic. - Refined bot endpoint deletion to avoid false positives. - Improved logging and user feedback throughout blueprint and cleanup flows. --- .../Commands/CleanupCommand.cs | 73 ++- .../SetupSubcommands/BlueprintSubcommand.cs | 500 +++++++++++++----- .../Commands/SetupSubcommands/SetupHelpers.cs | 42 +- .../Commands/SetupSubcommands/SetupResults.cs | 9 +- .../Program.cs | 2 +- .../Services/BotConfigurator.cs | 17 +- .../Services/FederatedCredentialService.cs | 350 +++++++++--- .../Services/Helpers/AdminConsentHelper.cs | 81 +++ .../CleanupCommandBotEndpointTests.cs | 8 +- .../Commands/CleanupCommandTests.cs | 53 +- .../Services/AdminConsentHelperTests.cs | 167 ++++++ .../Services/BlueprintLookupServiceTests.cs | 2 +- .../FederatedCredentialServiceTests.cs | 122 ++++- 13 files changed, 1160 insertions(+), 266 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs index a86b6aa7..9eefcc52 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs @@ -18,7 +18,8 @@ public static Command CreateCommand( IBotConfigurator botConfigurator, CommandExecutor executor, AgentBlueprintService agentBlueprintService, - IConfirmationProvider confirmationProvider) + IConfirmationProvider confirmationProvider, + FederatedCredentialService federatedCredentialService) { var cleanupCommand = new Command("cleanup", "Clean up ALL resources (blueprint, instance, Azure) - use subcommands for granular cleanup"); @@ -40,11 +41,11 @@ public static Command CreateCommand( // Set default handler for 'a365 cleanup' (without subcommand) - cleans up everything cleanupCommand.SetHandler(async (configFile, verbose) => { - await ExecuteAllCleanupAsync(logger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, configFile); + await ExecuteAllCleanupAsync(logger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService, configFile); }, configOption, verboseOption); // Add subcommands for granular control - cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService)); + cleanupCommand.AddCommand(CreateBlueprintCleanupCommand(logger, configService, botConfigurator, executor, agentBlueprintService, federatedCredentialService)); cleanupCommand.AddCommand(CreateAzureCleanupCommand(logger, configService, executor)); cleanupCommand.AddCommand(CreateInstanceCleanupCommand(logger, configService, executor)); @@ -56,7 +57,8 @@ private static Command CreateBlueprintCleanupCommand( IConfigService configService, IBotConfigurator botConfigurator, CommandExecutor executor, - AgentBlueprintService agentBlueprintService) + AgentBlueprintService agentBlueprintService, + FederatedCredentialService federatedCredentialService) { var command = new Command("blueprint", "Remove Entra ID blueprint application and service principal"); @@ -124,7 +126,32 @@ private static Command CreateBlueprintCleanupCommand( return; } + // Delete federated credentials first before deleting the blueprint + logger.LogInformation(""); + logger.LogInformation("Deleting federated credentials from blueprint..."); + + // Configure FederatedCredentialService with custom client app ID if available + if (!string.IsNullOrWhiteSpace(config.ClientAppId)) + { + federatedCredentialService.CustomClientAppId = config.ClientAppId; + } + + var ficsDeleted = await federatedCredentialService.DeleteAllFederatedCredentialsAsync( + config.TenantId, + config.AgentBlueprintId); + + if (!ficsDeleted) + { + logger.LogWarning("Some federated credentials may not have been deleted successfully"); + logger.LogWarning("Continuing with blueprint deletion..."); + } + else + { + logger.LogInformation("Federated credentials deleted successfully"); + } + // Delete the agent blueprint using the special Graph API endpoint + logger.LogInformation(""); logger.LogInformation("Deleting agent blueprint application..."); var deleted = await agentBlueprintService.DeleteAgentBlueprintAsync( config.TenantId, @@ -395,6 +422,7 @@ private static async Task ExecuteAllCleanupAsync( CommandExecutor executor, AgentBlueprintService agentBlueprintService, IConfirmationProvider confirmationProvider, + FederatedCredentialService federatedCredentialService, FileInfo? configFile) { var cleanupSucceeded = false; @@ -447,7 +475,34 @@ private static async Task ExecuteAllCleanupAsync( logger.LogInformation("Starting complete cleanup..."); - // 1. Delete agent blueprint application + // 1. Delete federated credentials from agent blueprint (if exists) + if (!string.IsNullOrWhiteSpace(config.AgentBlueprintId)) + { + logger.LogInformation("Deleting federated credentials from blueprint..."); + + // Configure FederatedCredentialService with custom client app ID if available + if (!string.IsNullOrWhiteSpace(config.ClientAppId)) + { + federatedCredentialService.CustomClientAppId = config.ClientAppId; + } + + var ficsDeleted = await federatedCredentialService.DeleteAllFederatedCredentialsAsync( + config.TenantId, + config.AgentBlueprintId); + + if (!ficsDeleted) + { + logger.LogWarning("Some federated credentials may not have been deleted successfully"); + logger.LogWarning("Continuing with blueprint deletion..."); + hasFailures = true; + } + else + { + logger.LogInformation("Federated credentials deleted successfully"); + } + } + + // 2. Delete agent blueprint application if (!string.IsNullOrWhiteSpace(config.AgentBlueprintId)) { logger.LogInformation("Deleting agent blueprint application..."); @@ -467,7 +522,7 @@ private static async Task ExecuteAllCleanupAsync( } } - // 2. Delete agent identity application + // 3. Delete agent identity application if (!string.IsNullOrWhiteSpace(config.AgenticAppId)) { logger.LogInformation("Deleting agent identity application..."); @@ -488,7 +543,7 @@ private static async Task ExecuteAllCleanupAsync( } } - // 3. Delete agent user + // 4. Delete agent user if (!string.IsNullOrWhiteSpace(config.AgenticUserId)) { logger.LogInformation("Deleting agent user..."); @@ -496,7 +551,7 @@ private static async Task ExecuteAllCleanupAsync( logger.LogInformation("Agent user deleted"); } - // 4. Delete bot messaging endpoint using shared helper + // 5. Delete bot messaging endpoint using shared helper if (!string.IsNullOrWhiteSpace(config.BotName)) { var endpointDeleted = await DeleteMessagingEndpointAsync(logger, config, botConfigurator); @@ -506,7 +561,7 @@ private static async Task ExecuteAllCleanupAsync( } } - // 5. Delete Azure resources (Web App and App Service Plan) + // 6. Delete Azure resources (Web App and App Service Plan) if (!string.IsNullOrWhiteSpace(config.WebAppName) && !string.IsNullOrWhiteSpace(config.ResourceGroup)) { logger.LogInformation("Deleting Azure resources..."); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index 0ddaf0e9..c2f0d209 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -377,8 +377,6 @@ public static async Task CreateBlueprintImplementationA await CreateBlueprintClientSecretAsync( blueprintObjectId!, blueprintAppId!, - generatedConfig, - generatedConfigPath, graphService, setupConfig, configService, @@ -583,7 +581,9 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( if (lookupResult.Found) { - logger.LogDebug("Discovered existing blueprint from previous setup, updating config with objectId for faster lookups"); + logger.LogInformation("Found existing blueprint by display name - updating config with current identifiers"); + logger.LogInformation(" - Object ID: {ObjectId}", lookupResult.ObjectId); + logger.LogInformation(" - App ID: {AppId}", lookupResult.AppId); existingObjectId = lookupResult.ObjectId; existingAppId = lookupResult.AppId; @@ -620,8 +620,33 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( logger.LogDebug("Config updated with blueprint identifiers"); } - // Blueprint exists, skip creation and move to FIC/secret validation - return (true, existingAppId, existingObjectId, existingServicePrincipalId, alreadyExisted: true); + // Blueprint exists - complete configuration (FIC validation + admin consent) + // Validate required identifiers before proceeding + if (string.IsNullOrWhiteSpace(existingAppId) || string.IsNullOrWhiteSpace(existingObjectId)) + { + logger.LogError("Existing blueprint found but required identifiers are missing (AppId: {AppId}, ObjectId: {ObjectId})", + existingAppId ?? "(null)", existingObjectId ?? "(null)"); + return (false, null, null, null, alreadyExisted: false); + } + + return await CompleteBlueprintConfigurationAsync( + logger, + executor, + graphApiService, + blueprintService, + blueprintLookupService, + federatedCredentialService, + tenantId, + displayName, + managedIdentityPrincipalId, + useManagedIdentity, + generatedConfig, + setupConfig, + existingAppId, + existingObjectId, + existingServicePrincipalId, + alreadyExisted: true, + ct); } // ======================================================================== @@ -840,28 +865,84 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( } } - // Persist blueprint identifiers immediately after creation - logger.LogInformation("Persisting blueprint metadata to config..."); + // Store blueprint identifiers in config object (will be persisted after secret creation) setupConfig.AgentBlueprintObjectId = objectId; setupConfig.AgentBlueprintServicePrincipalObjectId = servicePrincipalId; setupConfig.AgentBlueprintId = appId; - logger.LogDebug("About to save config with: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", + logger.LogDebug("Blueprint identifiers staged for persistence: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", objectId, servicePrincipalId, appId); - - await configService.SaveStateAsync(setupConfig); - logger.LogInformation("Config updated with blueprint identifiers"); - - logger.LogDebug("After save, config has: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", - setupConfig.AgentBlueprintObjectId, setupConfig.AgentBlueprintServicePrincipalObjectId, setupConfig.AgentBlueprintId); - // Create Federated Identity Credential ONLY when MSI is relevant (if managed identity provided) - if (useManagedIdentity && !string.IsNullOrWhiteSpace(managedIdentityPrincipalId)) - { - logger.LogInformation("Creating Federated Identity Credential for Managed Identity..."); - var credentialName = $"{displayName.Replace(" ", "")}-MSI"; + // Complete configuration (FIC validation + admin consent) + return await CompleteBlueprintConfigurationAsync( + logger, + executor, + graphApiService, + blueprintService, + blueprintLookupService, + federatedCredentialService, + tenantId, + displayName, + managedIdentityPrincipalId, + useManagedIdentity, + generatedConfig, + setupConfig, + appId, + objectId, + servicePrincipalId, + alreadyExisted: false, + ct); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to create agent blueprint: {Message}", ex.Message); + return (false, null, null, null, alreadyExisted: false); + } + } - // Check if FIC already exists before creating + /// + /// Completes blueprint configuration by validating/creating federated credentials and requesting admin consent. + /// Called by both existing blueprint and new blueprint paths to ensure consistent configuration. + /// + private static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId, bool alreadyExisted)> CompleteBlueprintConfigurationAsync( + ILogger logger, + CommandExecutor executor, + GraphApiService graphApiService, + AgentBlueprintService blueprintService, + BlueprintLookupService blueprintLookupService, + FederatedCredentialService federatedCredentialService, + string tenantId, + string displayName, + string? managedIdentityPrincipalId, + bool useManagedIdentity, + JsonObject generatedConfig, + Models.Agent365Config setupConfig, + string appId, + string objectId, + string? servicePrincipalId, + bool alreadyExisted, + CancellationToken ct) + { + // ======================================================================== + // Federated Identity Credential Validation/Creation + // ======================================================================== + + // Create Federated Identity Credential ONLY when MSI is relevant (if managed identity provided) + if (useManagedIdentity && !string.IsNullOrWhiteSpace(managedIdentityPrincipalId)) + { + logger.LogInformation("Configuring Federated Identity Credential for Managed Identity..."); + // Include full blueprint object ID to ensure absolute uniqueness across tenant + // FIC names have tenant-wide uniqueness constraints, not scoped to individual blueprints + // Using full objectId guarantees no collisions even with rapid create/delete cycles + var credentialName = $"{displayName.Replace(" ", "")}-{objectId}-MSI"; + + // For existing blueprints, check if FIC already exists to provide better UX + // For new blueprints, we skip this and go straight to create (avoiding race conditions) + bool ficSuccess; + if (alreadyExisted) + { + // Blueprint exists - check if FIC is already configured + logger.LogDebug("Checking for existing federated credential with subject: {Subject}", managedIdentityPrincipalId); var ficExistsResult = await federatedCredentialService.CheckFederatedCredentialExistsAsync( tenantId, objectId, @@ -869,30 +950,50 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( $"https://login.microsoftonline.com/{tenantId}/v2.0", ct); - bool ficSuccess; if (ficExistsResult.Exists) { - logger.LogInformation("Federated credential already exists (skipping)"); + logger.LogInformation("Federated Identity Credential already configured"); + logger.LogInformation(" - Credential Name: {Name}", ficExistsResult.ExistingCredential?.Name ?? "(unknown)"); logger.LogInformation(" - Subject (MSI Principal ID): {MsiId}", managedIdentityPrincipalId); ficSuccess = true; } else { - logger.LogInformation("Creating new federated credential..."); - var ficCreateResult = await federatedCredentialService.CreateFederatedCredentialAsync( - tenantId, - objectId, - credentialName, - $"https://login.microsoftonline.com/{tenantId}/v2.0", - managedIdentityPrincipalId, - new List { "api://AzureADTokenExchange" }, + // FIC doesn't exist on existing blueprint - create it with retry logic + logger.LogInformation("Creating Federated Identity Credential for existing blueprint..."); + logger.LogDebug(" - Name: {Name}", credentialName); + logger.LogDebug(" - Subject: {Subject}", managedIdentityPrincipalId); + logger.LogDebug(" - Issuer: https://login.microsoftonline.com/{TenantId}/v2.0", tenantId); + + var retryHelper = new RetryHelper(logger); + FederatedCredentialCreateResult? ficCreateResult = null; + + var ficReady = await retryHelper.ExecuteWithRetryAsync( + async ct => + { + ficCreateResult = await federatedCredentialService.CreateFederatedCredentialAsync( + tenantId, + objectId, + credentialName, + $"https://login.microsoftonline.com/{tenantId}/v2.0", + managedIdentityPrincipalId, + new List { "api://AzureADTokenExchange" }, + ct); + + // Return true if successful or already exists + // Return false if should retry (HTTP 404) + return ficCreateResult.Success || ficCreateResult.AlreadyExisted; + }, + result => !result, // Retry while result is false + maxRetries: 10, + baseDelaySeconds: 3, ct); - ficSuccess = ficCreateResult.Success; + ficSuccess = (ficCreateResult?.Success ?? false) || (ficCreateResult?.AlreadyExisted ?? false); - if (ficCreateResult.AlreadyExisted) + if (ficCreateResult?.AlreadyExisted == true) { - logger.LogInformation("Federated credential was created by another process (race condition handled)"); + logger.LogInformation("Federated Identity Credential already exists (detected during creation)"); } else if (ficSuccess) { @@ -900,130 +1001,270 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( } else { - logger.LogWarning("Failed to create Federated Identity Credential: {Error}", ficCreateResult.ErrorMessage ?? "Unknown error"); + logger.LogError("Failed to create Federated Identity Credential: {Error}", ficCreateResult?.ErrorMessage ?? "Unknown error"); + logger.LogError("The agent instance may not be able to authenticate using Managed Identity"); } } + } + else + { + // Brand new blueprint - create with retry logic for propagation delays + logger.LogInformation("Creating Federated Identity Credential for new blueprint..."); + logger.LogDebug(" - Name: {Name}", credentialName); + logger.LogDebug(" - Subject: {Subject}", managedIdentityPrincipalId); + logger.LogDebug(" - Issuer: https://login.microsoftonline.com/{TenantId}/v2.0", tenantId); + + var retryHelper = new RetryHelper(logger); + FederatedCredentialCreateResult? ficCreateResult = null; + + var ficReady = await retryHelper.ExecuteWithRetryAsync( + async ct => + { + ficCreateResult = await federatedCredentialService.CreateFederatedCredentialAsync( + tenantId, + objectId, + credentialName, + $"https://login.microsoftonline.com/{tenantId}/v2.0", + managedIdentityPrincipalId, + new List { "api://AzureADTokenExchange" }, + ct); + + // Return true if successful or already exists + // Return false if should retry (HTTP 404) + return ficCreateResult.Success || ficCreateResult.AlreadyExisted; + }, + result => !result, // Retry while result is false + maxRetries: 10, + baseDelaySeconds: 3, + ct); - if (!ficSuccess) + ficSuccess = (ficCreateResult?.Success ?? false) || (ficCreateResult?.AlreadyExisted ?? false); + + if (ficCreateResult?.AlreadyExisted == true) { - logger.LogWarning("Failed to create Federated Identity Credential"); + logger.LogInformation("Federated Identity Credential configured (idempotent)"); + } + else if (ficSuccess) + { + logger.LogInformation("Federated Identity Credential created successfully"); + } + else + { + logger.LogError("Failed to create Federated Identity Credential: {Error}", ficCreateResult?.ErrorMessage ?? "Unknown error"); + logger.LogError("The agent instance may not be able to authenticate using Managed Identity"); } } - else if (!useManagedIdentity) - { - logger.LogInformation("Skipping Federated Identity Credential creation (external hosting / no MSI configured)"); - } - else + + if (!ficSuccess) { - logger.LogInformation("Skipping Federated Identity Credential creation (no MSI Principal ID provided)"); + logger.LogWarning("Federated Identity Credential configuration incomplete"); + logger.LogWarning("You may need to create the credential manually in Entra ID"); } + } + else if (!useManagedIdentity) + { + logger.LogInformation("Skipping Federated Identity Credential creation (external hosting / no MSI configured)"); + } + else + { + logger.LogInformation("Skipping Federated Identity Credential creation (no MSI Principal ID provided)"); + } - // Request admin consent - logger.LogInformation("Requesting admin consent for application"); + // ======================================================================== + // Admin Consent + // ======================================================================== + + var (consentSuccess, consentUrlGraph) = await EnsureAdminConsentAsync( + logger, + executor, + graphApiService, + blueprintService, + blueprintLookupService, + tenantId, + appId, + objectId, + servicePrincipalId, + setupConfig, + alreadyExisted, + ct); - // Get application scopes from config (fallback to hardcoded defaults) - var applicationScopes = new List(); + // Add Graph API consent to the resource consents collection + var applicationScopes = GetApplicationScopes(setupConfig, logger); + var resourceConsents = new JsonArray(); + resourceConsents.Add(new JsonObject + { + ["resourceName"] = "Microsoft Graph", + ["resourceAppId"] = AuthenticationConstants.MicrosoftGraphResourceAppId, + ["consentUrl"] = consentUrlGraph, + ["consentGranted"] = consentSuccess, + ["consentTimestamp"] = consentSuccess ? DateTime.UtcNow.ToString("O") : null, + ["scopes"] = new JsonArray(applicationScopes.Select(s => JsonValue.Create(s)).ToArray()) + }); - var appScopesFromConfig = setupConfig.AgentApplicationScopes; - if (appScopesFromConfig != null && appScopesFromConfig.Count > 0) - { - logger.LogInformation(" Found 'agentApplicationScopes' in typed config"); - applicationScopes.AddRange(appScopesFromConfig); - } - else - { - logger.LogInformation(" 'agentApplicationScopes' not found in config, using hardcoded defaults"); - applicationScopes.AddRange(ConfigConstants.DefaultAgentApplicationScopes); - } + generatedConfig["resourceConsents"] = resourceConsents; - // Final fallback (should not happen with proper defaults) - if (applicationScopes.Count == 0) - { - logger.LogWarning("No application scopes available, falling back to User.Read"); - applicationScopes.Add("User.Read"); - } + if (!consentSuccess) + { + logger.LogWarning(""); + logger.LogWarning("Admin consent may not have been detected"); + logger.LogWarning("The setup will continue, but you may need to grant consent manually."); + logger.LogWarning("Consent URL: {Url}", consentUrlGraph); + } - logger.LogInformation(" - Application scopes: {Scopes}", string.Join(", ", applicationScopes)); + return (true, appId, objectId, servicePrincipalId, alreadyExisted); + } - // Generate consent URL for Graph API - var applicationScopesJoined = string.Join(' ', applicationScopes); - var consentUrlGraph = $"https://login.microsoftonline.com/{tenantId}/v2.0/adminconsent?client_id={appId}&scope={Uri.EscapeDataString(applicationScopesJoined)}&redirect_uri=https://entra.microsoft.com/TokenAuthorize&state=xyz123"; + /// + /// Gets application scopes from config with fallback to defaults. + /// + private static List GetApplicationScopes(Models.Agent365Config setupConfig, ILogger logger) + { + var applicationScopes = new List(); - logger.LogInformation("Opening browser for Graph API admin consent..."); - TryOpenBrowser(consentUrlGraph); + var appScopesFromConfig = setupConfig.AgentApplicationScopes; + if (appScopesFromConfig != null && appScopesFromConfig.Count > 0) + { + logger.LogDebug(" Found 'agentApplicationScopes' in typed config"); + applicationScopes.AddRange(appScopesFromConfig); + } + else + { + logger.LogDebug(" 'agentApplicationScopes' not found in config, using hardcoded defaults"); + applicationScopes.AddRange(ConfigConstants.DefaultAgentApplicationScopes); + } - var consentSuccess = await AdminConsentHelper.PollAdminConsentAsync(executor, logger, appId, "Graph API Scopes", 180, 5, ct); + // Final fallback (should not happen with proper defaults) + if (applicationScopes.Count == 0) + { + logger.LogWarning("No application scopes available, falling back to User.Read"); + applicationScopes.Add("User.Read"); + } - if (consentSuccess) - { - logger.LogInformation("Graph API admin consent granted successfully!"); - } - else + return applicationScopes; + } + + /// + /// Ensures admin consent for the blueprint application. + /// For existing blueprints, checks if consent already exists before requesting browser interaction. + /// For new blueprints, skips verification and directly requests consent. + /// Returns: (consentSuccess, consentUrl) + /// + private static async Task<(bool consentSuccess, string consentUrl)> EnsureAdminConsentAsync( + ILogger logger, + CommandExecutor executor, + GraphApiService graphApiService, + AgentBlueprintService blueprintService, + BlueprintLookupService blueprintLookupService, + string tenantId, + string appId, + string objectId, + string? servicePrincipalId, + Models.Agent365Config setupConfig, + bool alreadyExisted, + CancellationToken ct) + { + var applicationScopes = GetApplicationScopes(setupConfig, logger); + bool consentAlreadyExists = false; + + // Only check for existing consent if blueprint already existed + // New blueprints cannot have consent yet, so skip the verification + if (alreadyExisted) + { + logger.LogInformation("Verifying admin consent for application"); + logger.LogDebug(" - Application scopes: {Scopes}", string.Join(", ", applicationScopes)); + + // Check if consent already exists with required scopes + var blueprintSpId = servicePrincipalId; + if (string.IsNullOrWhiteSpace(blueprintSpId)) { - logger.LogWarning("Graph API admin consent may not have completed"); + logger.LogDebug("Looking up service principal for blueprint to check consent..."); + var spLookup = await blueprintLookupService.GetServicePrincipalByAppIdAsync(tenantId, appId, ct); + blueprintSpId = spLookup.ObjectId; } - // Set inheritable permissions for Microsoft Graph so agent instances can access Graph on behalf of users - if (consentSuccess) + if (!string.IsNullOrWhiteSpace(blueprintSpId)) { - logger.LogInformation("Configuring inheritable permissions for Microsoft Graph..."); - try - { - // Update config with blueprint ID so EnsureResourcePermissionsAsync can use it - setupConfig.AgentBlueprintId = appId; - - await SetupHelpers.EnsureResourcePermissionsAsync( - graph: graphApiService, - blueprintService: blueprintService, - config: setupConfig, - resourceAppId: AuthenticationConstants.MicrosoftGraphResourceAppId, - resourceName: "Microsoft Graph", - scopes: applicationScopes.ToArray(), - logger: logger, - addToRequiredResourceAccess: false, - setInheritablePermissions: true, - setupResults: null, - ct: ct); + // Get Microsoft Graph service principal ID + var graphSpId = await graphApiService.LookupServicePrincipalByAppIdAsync( + tenantId, + AuthenticationConstants.MicrosoftGraphResourceAppId, + ct); - logger.LogInformation("Microsoft Graph inheritable permissions configured successfully"); - } - catch (Exception ex) + if (!string.IsNullOrWhiteSpace(graphSpId)) { - logger.LogWarning("Failed to configure Microsoft Graph inheritable permissions: {Message}", ex.Message); - logger.LogWarning("Agent instances may not be able to access Microsoft Graph resources"); - logger.LogWarning("You can configure these manually later with: a365 setup permissions"); + // Use shared helper to check existing consent + consentAlreadyExists = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, + tenantId, + blueprintSpId, + graphSpId, + applicationScopes, + logger, + ct); } } - // Add Graph API consent to the resource consents collection - var resourceConsents = new JsonArray(); - resourceConsents.Add(new JsonObject + if (consentAlreadyExists) { - ["resourceName"] = "Microsoft Graph", - ["resourceAppId"] = "00000003-0000-0000-c000-000000000000", - ["consentUrl"] = consentUrlGraph, - ["consentGranted"] = consentSuccess, - ["consentTimestamp"] = consentSuccess ? DateTime.UtcNow.ToString("O") : null, - ["scopes"] = new JsonArray(applicationScopes.Select(s => JsonValue.Create(s)).ToArray()) - }); + logger.LogInformation("Admin consent already granted for all required scopes"); + logger.LogDebug(" - Scopes: {Scopes}", string.Join(", ", applicationScopes)); + } + } + + var applicationScopesJoined = string.Join(' ', applicationScopes); + var consentUrlGraph = $"https://login.microsoftonline.com/{tenantId}/v2.0/adminconsent?client_id={appId}&scope={Uri.EscapeDataString(applicationScopesJoined)}&redirect_uri=https://entra.microsoft.com/TokenAuthorize&state=xyz123"; - generatedConfig["resourceConsents"] = resourceConsents; + if (consentAlreadyExists) + { + return (true, consentUrlGraph); + } + + // Request consent via browser + logger.LogInformation("Requesting admin consent for application"); + logger.LogInformation(" - Application scopes: {Scopes}", string.Join(", ", applicationScopes)); + logger.LogInformation("Opening browser for Graph API admin consent..."); + TryOpenBrowser(consentUrlGraph); + + var consentSuccess = await AdminConsentHelper.PollAdminConsentAsync(executor, logger, appId, "Graph API Scopes", 180, 5, ct); + + if (consentSuccess) + { + logger.LogInformation("Graph API admin consent granted successfully!"); - if (!consentSuccess) + // Set inheritable permissions for Microsoft Graph + logger.LogInformation("Configuring inheritable permissions for Microsoft Graph..."); + try { - logger.LogWarning(""); - logger.LogWarning("Admin consent may not have been detected"); - logger.LogWarning("The setup will continue, but you may need to grant consent manually."); - logger.LogWarning("Consent URL: {Url}", consentUrlGraph); - } + setupConfig.AgentBlueprintId = appId; + + await SetupHelpers.EnsureResourcePermissionsAsync( + graph: graphApiService, + blueprintService: blueprintService, + config: setupConfig, + resourceAppId: AuthenticationConstants.MicrosoftGraphResourceAppId, + resourceName: "Microsoft Graph", + scopes: applicationScopes.ToArray(), + logger: logger, + addToRequiredResourceAccess: false, + setInheritablePermissions: true, + setupResults: null, + ct: ct); - return (true, appId, objectId, servicePrincipalId, alreadyExisted: false); + logger.LogInformation("Microsoft Graph inheritable permissions configured successfully"); + } + catch (Exception ex) + { + logger.LogWarning("Failed to configure Microsoft Graph inheritable permissions: {Message}", ex.Message); + logger.LogWarning("Agent instances may not be able to access Microsoft Graph resources"); + logger.LogWarning("You can configure these manually later with: a365 setup permissions"); + } } - catch (Exception ex) + else { - logger.LogError(ex, "Failed to create agent blueprint: {Message}", ex.Message); - return (false, null, null, null, alreadyExisted: false); + logger.LogWarning("Graph API admin consent may not have completed"); } + + return (consentSuccess, consentUrlGraph); } /// @@ -1115,8 +1356,6 @@ private static void TryOpenBrowser(string url) public static async Task CreateBlueprintClientSecretAsync( string blueprintObjectId, string blueprintAppId, - JsonObject generatedConfig, - string generatedConfigPath, GraphApiService graphService, Models.Agent365Config setupConfig, IConfigService configService, @@ -1128,7 +1367,7 @@ public static async Task CreateBlueprintClientSecretAsync( logger.LogInformation("Creating client secret for Agent Blueprint using Graph API..."); var graphToken = await graphService.GetGraphAccessTokenAsync( - generatedConfig["tenantId"]?.GetValue() ?? string.Empty, ct); + setupConfig.TenantId ?? string.Empty, ct); if (string.IsNullOrWhiteSpace(graphToken)) { @@ -1173,17 +1412,16 @@ public static async Task CreateBlueprintClientSecretAsync( var protectedSecret = Microsoft.Agents.A365.DevTools.Cli.Helpers.SecretProtectionHelper.ProtectSecret(secretTextNode.GetValue(), logger); var isProtected = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); - generatedConfig["agentBlueprintClientSecret"] = protectedSecret; - generatedConfig["agentBlueprintClientSecretProtected"] = isProtected; setupConfig.AgentBlueprintClientSecret = protectedSecret; setupConfig.AgentBlueprintClientSecretProtected = isProtected; - // Use SaveStateAsync to preserve all existing dynamic properties (especially agentBlueprintServicePrincipalObjectId) + // Single consolidated save: persists blueprint identifiers (objectId, servicePrincipalId, appId) + client secret + // This ensures all blueprint-related state is saved atomically await configService.SaveStateAsync(setupConfig); logger.LogInformation("Client secret created successfully!"); logger.LogInformation($" - Secret stored in generated config (encrypted: {isProtected})"); - logger.LogWarning("IMPORTANT: The client secret has been stored in {Path}", generatedConfigPath); + logger.LogWarning("IMPORTANT: The client secret has been stored in a365.generated.config.json"); logger.LogWarning("Keep this file secure and do not commit it to source control!"); if (!isProtected) @@ -1200,7 +1438,7 @@ public static async Task CreateBlueprintClientSecretAsync( logger.LogInformation(" 2. Find your Agent Blueprint: {AppId}", blueprintAppId); logger.LogInformation(" 3. Navigate to Certificates & secrets > Client secrets"); logger.LogInformation(" 4. Click 'New client secret' and save the value"); - logger.LogInformation(" 5. Add it to {Path} as 'agentBlueprintClientSecret'", generatedConfigPath); + logger.LogInformation(" 5. Add it to a365.generated.config.json as 'agentBlueprintClientSecret'"); } } 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 dced44d1..9a639582 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -93,11 +93,20 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) logger.LogInformation(" [OK] Agent blueprint {Status} (Blueprint ID: {BlueprintId})", status, results.BlueprintId ?? "unknown"); } if (results.McpPermissionsConfigured) - logger.LogInformation(" [OK] MCP server permissions configured"); + { + var status = results.McpPermissionsAlreadyExisted ? "verified (already configured)" : "configured"; + logger.LogInformation(" [OK] MCP server permissions {Status}", status); + } if (results.InheritablePermissionsConfigured) - logger.LogInformation(" [OK] Inheritable permissions configured"); + { + var status = results.InheritablePermissionsAlreadyExisted ? "verified (already configured)" : "configured"; + logger.LogInformation(" [OK] Inheritable permissions {Status}", status); + } if (results.BotApiPermissionsConfigured) - logger.LogInformation(" [OK] Messaging Bot API permissions configured"); + { + var status = results.BotApiPermissionsAlreadyExisted ? "verified (already configured)" : "configured"; + logger.LogInformation(" [OK] Messaging Bot API permissions {Status}", status); + } if (results.MessagingEndpointRegistered) { var status = results.EndpointAlreadyExisted ? "configured (already exists)" : "created"; @@ -271,7 +280,7 @@ public static async Task EnsureResourcePermissionsAsync( if (setInheritablePermissions) { - logger.LogInformation(" - Inheritable permissions: blueprint {Blueprint} to resourceAppId {ResourceAppId} scopes [{Scopes}]", + logger.LogInformation(" - Configuring inheritable permissions: blueprint {Blueprint} to resourceAppId {ResourceAppId} scopes [{Scopes}]", config.AgentBlueprintId, resourceAppId, string.Join(' ', scopes)); // Use custom client app auth for inheritable permissions - Azure CLI doesn't support this operation @@ -286,6 +295,15 @@ public static async Task EnsureResourcePermissionsAsync( "Ensure you have AgentIdentityBlueprint.UpdateAuthProperties.All and Application.ReadWrite.All permissions in your custom client app."); } + if (alreadyExists) + { + logger.LogInformation(" - Inheritable permissions already configured for {ResourceName}", resourceName); + } + else + { + logger.LogInformation(" - Inheritable permissions created for {ResourceName}", resourceName); + } + inheritanceConfigured = true; inheritanceAlreadyExisted = alreadyExists; @@ -355,6 +373,22 @@ public static async Task EnsureResourcePermissionsAsync( } } + // Track if permissions already existed for accurate summary logging + if (setupResults != null && inheritanceConfigured) + { + // Update flags based on resource type + if (resourceName.Contains("Tools", StringComparison.OrdinalIgnoreCase) || + resourceName.Contains("MCP", StringComparison.OrdinalIgnoreCase)) + { + setupResults.McpPermissionsAlreadyExisted = inheritanceAlreadyExisted; + setupResults.InheritablePermissionsAlreadyExisted = inheritanceAlreadyExisted; + } + else if (resourceName.Contains("Bot", StringComparison.OrdinalIgnoreCase)) + { + setupResults.BotApiPermissionsAlreadyExisted = inheritanceAlreadyExisted; + } + } + // 4. Update resource consents collection var existingConsent = config.ResourceConsents.FirstOrDefault(rc => rc.ResourceAppId.Equals(resourceAppId, StringComparison.OrdinalIgnoreCase)); 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 16a6e435..ff0d280d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -16,14 +16,13 @@ public class SetupResults public bool MessagingEndpointRegistered { get; set; } public bool InheritablePermissionsConfigured { get; set; } - // Idempotency tracking flags + // Idempotency tracking flags - track whether resources already existed (vs newly created) public bool InfrastructureAlreadyExisted { get; set; } public bool BlueprintAlreadyExisted { get; set; } - public bool BlueprintDiscoveredAndPersisted { get; set; } - public bool ServicePrincipalDiscovered { get; set; } - public bool FicAlreadyExisted { get; set; } - public bool SecretReused { get; set; } public bool EndpointAlreadyExisted { get; set; } + public bool McpPermissionsAlreadyExisted { get; set; } + public bool InheritablePermissionsAlreadyExisted { get; set; } + public bool BotApiPermissionsAlreadyExisted { get; set; } public List Errors { get; } = new(); public List Warnings { get; } = new(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index b8b606b9..235e0eb1 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -97,7 +97,7 @@ static async Task Main(string[] args) var confirmationProvider = serviceProvider.GetRequiredService(); rootCommand.AddCommand(ConfigCommand.CreateCommand(configLogger, wizardService: wizardService, clientAppValidator: clientAppValidator)); rootCommand.AddCommand(QueryEntraCommand.CreateCommand(queryEntraLogger, configService, executor, graphApiService, agentBlueprintService)); - rootCommand.AddCommand(CleanupCommand.CreateCommand(cleanupLogger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider)); + rootCommand.AddCommand(CleanupCommand.CreateCommand(cleanupLogger, configService, botConfigurator, executor, agentBlueprintService, confirmationProvider, federatedCredentialService)); rootCommand.AddCommand(PublishCommand.CreateCommand(publishLogger, configService, agentPublishService, graphApiService, agentBlueprintService, manifestTemplateService)); // Wrap all command handlers with exception handling diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs index d7f9b875..cedb137d 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BotConfigurator.cs @@ -131,13 +131,18 @@ public async Task CreateEndpointWithAgentBlueprintAs { var errorContent = await response.Content.ReadAsStringAsync(); - // Check for "already exists" condition in multiple forms: - // 1. HTTP 409 Conflict (standard REST pattern) - // 2. HTTP 500 InternalServerError with "already exists" in message body (Azure Bot Service pattern) - bool isAlreadyExists = response.StatusCode == System.Net.HttpStatusCode.Conflict || - (errorContent.Contains(AlreadyExistsErrorMessage, StringComparison.OrdinalIgnoreCase)); + // Check for "already exists" condition - must be bot/endpoint-specific to avoid false positives + // Valid patterns: + // 1. HTTP 409 Conflict (standard REST pattern for resource conflicts) + // 2. HTTP 500 with bot-specific "already exists" message (Azure Bot Service pattern) + // - Must contain "already exists" AND at least one bot-specific keyword + bool isBotAlreadyExists = response.StatusCode == System.Net.HttpStatusCode.Conflict || + (errorContent.Contains(AlreadyExistsErrorMessage, StringComparison.OrdinalIgnoreCase) && + (errorContent.Contains("bot", StringComparison.OrdinalIgnoreCase) || + errorContent.Contains("endpoint", StringComparison.OrdinalIgnoreCase) || + errorContent.Contains(endpointName, StringComparison.OrdinalIgnoreCase))); - if (isAlreadyExists) + if (isBotAlreadyExists) { _logger.LogWarning("Endpoint '{EndpointName}' {AlreadyExistsMessage} in the resource group", endpointName, AlreadyExistsErrorMessage); _logger.LogInformation("Endpoint registration completed ({AlreadyExistsMessage})", AlreadyExistsErrorMessage); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs index 6a1a63cd..72e91c05 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs @@ -46,11 +46,27 @@ public async Task> GetFederatedCredentialsAsync( { _logger.LogDebug("Retrieving federated credentials for blueprint: {ObjectId}", blueprintObjectId); + // Try standard endpoint first var doc = await _graphApiService.GraphGetAsync( tenantId, $"/beta/applications/{blueprintObjectId}/federatedIdentityCredentials", cancellationToken); + // If standard endpoint returns data with credentials, use it + if (doc != null && doc.RootElement.TryGetProperty("value", out var valueCheck) && valueCheck.GetArrayLength() > 0) + { + _logger.LogDebug("Standard endpoint returned {Count} credential(s)", valueCheck.GetArrayLength()); + } + // If standard endpoint returns empty or null, try Agent Blueprint-specific endpoint + else + { + _logger.LogDebug("Standard endpoint returned no credentials or failed, trying Agent Blueprint fallback endpoint"); + doc = await _graphApiService.GraphGetAsync( + tenantId, + $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{blueprintObjectId}/federatedIdentityCredentials", + cancellationToken); + } + if (doc == null) { _logger.LogDebug("No federated credentials found for blueprint: {ObjectId}", blueprintObjectId); @@ -66,28 +82,65 @@ public async Task> GetFederatedCredentialsAsync( var credentials = new List(); foreach (var item in valueElement.EnumerateArray()) { - var id = item.GetProperty("id").GetString(); - var name = item.GetProperty("name").GetString(); - var issuer = item.GetProperty("issuer").GetString(); - var subject = item.GetProperty("subject").GetString(); - - var audiences = new List(); - if (item.TryGetProperty("audiences", out var audiencesElement)) + try { - foreach (var audience in audiencesElement.EnumerateArray()) + // Use TryGetProperty to handle missing fields gracefully + if (!item.TryGetProperty("id", out var idElement) || string.IsNullOrWhiteSpace(idElement.GetString())) { - audiences.Add(audience.GetString() ?? string.Empty); + _logger.LogWarning("Skipping federated credential with missing or empty 'id' field"); + continue; + } + + if (!item.TryGetProperty("name", out var nameElement) || string.IsNullOrWhiteSpace(nameElement.GetString())) + { + _logger.LogWarning("Skipping federated credential with missing or empty 'name' field"); + continue; } - } - credentials.Add(new FederatedCredentialInfo + if (!item.TryGetProperty("issuer", out var issuerElement) || string.IsNullOrWhiteSpace(issuerElement.GetString())) + { + _logger.LogWarning("Skipping federated credential with missing or empty 'issuer' field"); + continue; + } + + if (!item.TryGetProperty("subject", out var subjectElement) || string.IsNullOrWhiteSpace(subjectElement.GetString())) + { + _logger.LogWarning("Skipping federated credential with missing or empty 'subject' field"); + continue; + } + + var id = idElement.GetString(); + var name = nameElement.GetString(); + var issuer = issuerElement.GetString(); + var subject = subjectElement.GetString(); + + var audiences = new List(); + if (item.TryGetProperty("audiences", out var audiencesElement)) + { + foreach (var audience in audiencesElement.EnumerateArray()) + { + var audienceValue = audience.GetString(); + if (!string.IsNullOrWhiteSpace(audienceValue)) + { + audiences.Add(audienceValue); + } + } + } + + credentials.Add(new FederatedCredentialInfo + { + Id = id, + Name = name, + Issuer = issuer, + Subject = subject, + Audiences = audiences + }); + } + catch (Exception itemEx) { - Id = id, - Name = name, - Issuer = issuer, - Subject = subject, - Audiences = audiences - }); + // Log individual credential parsing errors but continue processing remaining credentials + _logger.LogWarning(itemEx, "Failed to parse federated credential entry, skipping"); + } } _logger.LogDebug("Found {Count} federated credential(s) for blueprint: {ObjectId}", @@ -146,7 +199,8 @@ public async Task CheckFederatedCredentialExists } catch (Exception ex) { - _logger.LogError(ex, "Failed to check federated credential existence"); + _logger.LogWarning(ex, "Failed to retrieve federated credentials for existence check. Assuming credential does not exist."); + _logger.LogDebug("Error details: {Message}", ex.Message); return new FederatedCredentialCheckResult { Exists = false, @@ -189,98 +243,231 @@ public async Task CreateFederatedCredentialAsyn audiences }; - var payloadJson = JsonSerializer.Serialize(payload); - - // Try the standard endpoint first - var endpoint = $"/beta/applications/{blueprintObjectId}/federatedIdentityCredentials"; - - var response = await _graphApiService.GraphPostWithResponseAsync( - tenantId, - endpoint, - payloadJson, - cancellationToken); + // Try both standard and Agent Blueprint-specific endpoints + var endpoints = new[] + { + $"/beta/applications/{blueprintObjectId}/federatedIdentityCredentials", + $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{blueprintObjectId}/federatedIdentityCredentials" + }; - if (response.IsSuccess) + foreach (var endpoint in endpoints) { - _logger.LogInformation("Successfully created federated credential: {Name}", name); - return new FederatedCredentialCreateResult + _logger.LogDebug("Attempting federated credential creation with endpoint: {Endpoint}", endpoint); + + var response = await _graphApiService.GraphPostWithResponseAsync( + tenantId, + endpoint, + payload, + cancellationToken); + + if (response.IsSuccess) { - Success = true, - AlreadyExisted = false - }; - } + _logger.LogInformation("Successfully created federated credential: {Name}", name); + return new FederatedCredentialCreateResult + { + Success = true, + AlreadyExisted = false + }; + } - // Check for HTTP 409 (Conflict) - credential already exists - if (response.StatusCode == 409) - { - _logger.LogDebug("Federated credential already exists (HTTP 409): {Name}", name); + // Check for HTTP 409 (Conflict) - credential already exists + if (response.StatusCode == 409) + { + _logger.LogInformation("Federated credential already exists: {Name}", name); + + return new FederatedCredentialCreateResult + { + Success = true, + AlreadyExisted = true + }; + } + + // Check if we should try the alternative endpoint + if (response.StatusCode == 403 && response.Body?.Contains("Agent Blueprints are not supported") == true) + { + _logger.LogDebug("Standard endpoint not supported for Agent Blueprints, trying specialized endpoint..."); + continue; + } + + // Check if we should try the alternative endpoint due to calling identity type + if (response.StatusCode == 403 && response.Body?.Contains("This operation cannot be performed for the specified calling identity type") == true) + { + _logger.LogDebug("Endpoint rejected calling identity type, trying alternative endpoint..."); + continue; + } + + // Check for HTTP 404 - resource not found (propagation delay) + if (response.StatusCode == 404) + { + _logger.LogDebug("Application not yet ready (HTTP 404), will retry..."); + return new FederatedCredentialCreateResult + { + Success = false, + ErrorMessage = $"HTTP {response.StatusCode}: {response.ReasonPhrase}", + ShouldRetry = true + }; + } + + // For other errors on first endpoint, try second endpoint + if (endpoint == endpoints[0]) + { + _logger.LogDebug("First endpoint failed with HTTP {StatusCode}, trying second endpoint...", response.StatusCode); + continue; + } + + // Both endpoints failed + _logger.LogError("Failed to create federated credential: HTTP {StatusCode} {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); + if (!string.IsNullOrWhiteSpace(response.Body)) + { + _logger.LogError("Error details: {Body}", response.Body); + } + + _logger.LogError("Failed to create federated credential: {Name}", name); return new FederatedCredentialCreateResult { - Success = true, - AlreadyExisted = true + Success = false, + ErrorMessage = $"HTTP {response.StatusCode}: {response.ReasonPhrase}" }; } - // Log error details from standard endpoint - _logger.LogWarning("Standard endpoint failed: HTTP {StatusCode} {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); - if (!string.IsNullOrWhiteSpace(response.Body)) + // Should not reach here, but handle it + return new FederatedCredentialCreateResult { - _logger.LogDebug("Response body: {Body}", response.Body); + Success = false, + ErrorMessage = "Failed after trying all endpoints" + }; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to create federated credential: {Name}", name); + return new FederatedCredentialCreateResult + { + Success = false, + ErrorMessage = ex.Message + }; + } + } + + /// + /// Delete a federated credential from a blueprint application. + /// + /// The tenant ID for authentication + /// The blueprint application object ID + /// The federated credential ID to delete + /// Cancellation token + /// True if deleted successfully or not found, false otherwise + public async Task DeleteFederatedCredentialAsync( + string tenantId, + string blueprintObjectId, + string credentialId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogDebug("Deleting federated credential: {CredentialId} from blueprint: {ObjectId}", + credentialId, blueprintObjectId); + + // Try the standard endpoint first + var endpoint = $"/beta/applications/{blueprintObjectId}/federatedIdentityCredentials/{credentialId}"; + + var success = await _graphApiService.GraphDeleteAsync( + tenantId, + endpoint, + cancellationToken, + treatNotFoundAsSuccess: true); + + if (success) + { + _logger.LogDebug("Successfully deleted federated credential using standard endpoint: {CredentialId}", credentialId); + return true; } // Try fallback endpoint for agent blueprint - _logger.LogDebug("Trying fallback endpoint for agent blueprint federated credential"); - endpoint = $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{blueprintObjectId}/federatedIdentityCredentials"; + _logger.LogDebug("Standard endpoint failed, trying fallback endpoint for agent blueprint"); + endpoint = $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{blueprintObjectId}/federatedIdentityCredentials/{credentialId}"; - response = await _graphApiService.GraphPostWithResponseAsync( + success = await _graphApiService.GraphDeleteAsync( tenantId, endpoint, - payloadJson, - cancellationToken); + cancellationToken, + treatNotFoundAsSuccess: true); - if (response.IsSuccess) + if (success) { - _logger.LogInformation("Successfully created federated credential using fallback endpoint: {Name}", name); - return new FederatedCredentialCreateResult - { - Success = true, - AlreadyExisted = false - }; + _logger.LogDebug("Successfully deleted federated credential using fallback endpoint: {CredentialId}", credentialId); + return true; } - // Check for HTTP 409 (Conflict) - credential already exists - if (response.StatusCode == 409) + _logger.LogWarning("Failed to delete federated credential using both endpoints: {CredentialId}", credentialId); + return false; + } + catch (Exception ex) + { + _logger.LogError(ex, "Exception deleting federated credential: {CredentialId}", credentialId); + return false; + } + } + + /// + /// Delete all federated credentials from a blueprint application. + /// + /// The tenant ID for authentication + /// The blueprint application object ID + /// Cancellation token + /// True if all credentials deleted successfully, false otherwise + public async Task DeleteAllFederatedCredentialsAsync( + string tenantId, + string blueprintObjectId, + CancellationToken cancellationToken = default) + { + try + { + _logger.LogInformation("Retrieving federated credentials for deletion from blueprint: {ObjectId}", blueprintObjectId); + + var credentials = await GetFederatedCredentialsAsync(tenantId, blueprintObjectId, cancellationToken); + + if (credentials.Count == 0) { - _logger.LogDebug("Federated credential already exists (HTTP 409) on fallback endpoint: {Name}", name); - return new FederatedCredentialCreateResult - { - Success = true, - AlreadyExisted = true - }; + _logger.LogInformation("No federated credentials found to delete"); + return true; } - // Log error details from fallback endpoint - _logger.LogError("Fallback endpoint also failed: HTTP {StatusCode} {ReasonPhrase}", response.StatusCode, response.ReasonPhrase); - if (!string.IsNullOrWhiteSpace(response.Body)) + _logger.LogInformation("Found {Count} federated credential(s) to delete", credentials.Count); + + bool allSuccess = true; + foreach (var credential in credentials) { - _logger.LogDebug("Response body: {Body}", response.Body); + if (string.IsNullOrWhiteSpace(credential.Id)) + { + _logger.LogWarning("Skipping credential with missing ID"); + continue; + } + + _logger.LogInformation("Deleting federated credential: {Name}", credential.Name ?? credential.Id); + + var deleted = await DeleteFederatedCredentialAsync( + tenantId, + blueprintObjectId, + credential.Id, + cancellationToken); + + if (deleted) + { + _logger.LogInformation("Federated credential deleted: {Name}", credential.Name ?? credential.Id); + } + else + { + _logger.LogWarning("Failed to delete federated credential: {Name}", credential.Name ?? credential.Id); + allSuccess = false; + } } - _logger.LogError("Failed to create federated credential: {Name}. Both standard and fallback endpoints returned errors.", name); - return new FederatedCredentialCreateResult - { - Success = false, - ErrorMessage = $"HTTP {response.StatusCode}: {response.ReasonPhrase}" - }; + return allSuccess; } catch (Exception ex) { - _logger.LogError(ex, "Failed to create federated credential: {Name}", name); - return new FederatedCredentialCreateResult - { - Success = false, - ErrorMessage = ex.Message - }; + _logger.LogError(ex, "Exception deleting federated credentials from blueprint: {ObjectId}", blueprintObjectId); + return false; } } } @@ -315,4 +502,5 @@ public class FederatedCredentialCreateResult public bool Success { get; set; } public bool AlreadyExisted { get; set; } public string? ErrorMessage { get; set; } + public bool ShouldRetry { get; set; } } 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 423847c3..7b1824cf 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs @@ -92,4 +92,85 @@ public static async Task PollAdminConsentAsync( return false; } } + + /// + /// 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. + /// + /// Graph API service for querying grants + /// Tenant ID + /// Client service principal object ID + /// Resource service principal object ID + /// List of required scope names (case-insensitive) + /// Logger for diagnostics + /// Cancellation token + /// True if all required scopes are already granted, false otherwise + public static async Task CheckConsentExistsAsync( + Services.GraphApiService graphApiService, + string tenantId, + string clientSpId, + string resourceSpId, + System.Collections.Generic.IEnumerable requiredScopes, + ILogger logger, + CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(clientSpId) || string.IsNullOrWhiteSpace(resourceSpId)) + { + logger.LogDebug("Cannot check consent: missing service principal IDs (Client: {ClientSpId}, Resource: {ResourceSpId})", + clientSpId ?? "(null)", resourceSpId ?? "(null)"); + return false; + } + + try + { + // Query existing grants + var grantDoc = await graphApiService.GraphGetAsync( + tenantId, + $"/v1.0/oauth2PermissionGrants?$filter=clientId eq '{clientSpId}' and resourceId eq '{resourceSpId}'", + ct); + + if (grantDoc == null || !grantDoc.RootElement.TryGetProperty("value", out var grants) || grants.GetArrayLength() == 0) + { + logger.LogDebug("No oauth2PermissionGrants found between client {ClientSpId} and resource {ResourceSpId}", + clientSpId, resourceSpId); + return false; + } + + // Check first grant for scopes + var grant = grants[0]; + if (!grant.TryGetProperty("scope", out var grantedScopes)) + { + logger.LogDebug("oauth2PermissionGrant missing 'scope' property"); + return false; + } + + var scopesString = grantedScopes.GetString() ?? ""; + var grantedScopeSet = new System.Collections.Generic.HashSet( + scopesString.Split(' ', StringSplitOptions.RemoveEmptyEntries), + StringComparer.OrdinalIgnoreCase); + + var requiredScopeSet = new System.Collections.Generic.HashSet(requiredScopes, StringComparer.OrdinalIgnoreCase); + + // Check if all required scopes are already granted + bool allScopesPresent = requiredScopeSet.IsSubsetOf(grantedScopeSet); + + if (allScopesPresent) + { + logger.LogDebug("All required scopes already granted: {Scopes}", string.Join(", ", requiredScopes)); + } + else + { + var missing = requiredScopeSet.Except(grantedScopeSet); + logger.LogDebug("Missing scopes in existing grant: {MissingScopes}", string.Join(", ", missing)); + } + + return allScopesPresent; + } + catch (Exception ex) + { + logger.LogDebug(ex, "Error checking existing consent between {ClientSpId} and {ResourceSpId}", + clientSpId, resourceSpId); + return false; + } + } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs index 17994cee..3ee26492 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandBotEndpointTests.cs @@ -18,6 +18,7 @@ public class CleanupCommandBotEndpointTests private readonly CommandExecutor _mockExecutor; private readonly GraphApiService _graphApiService; private readonly AgentBlueprintService _agentBlueprintService; + private readonly FederatedCredentialService _federatedCredentialService; private readonly IMicrosoftGraphTokenProvider _mockTokenProvider; private readonly IConfirmationProvider _mockConfirmationProvider; @@ -66,6 +67,10 @@ public CleanupCommandBotEndpointTests() var mockBlueprintLogger = Substitute.For>(); _agentBlueprintService = new AgentBlueprintService(mockBlueprintLogger, _graphApiService); + // Create FederatedCredentialService wrapping GraphApiService + var mockFicLogger = Substitute.For>(); + _federatedCredentialService = new FederatedCredentialService(mockFicLogger, _graphApiService); + // Setup mock confirmation provider to return true by default _mockConfirmationProvider = Substitute.For(); _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); @@ -101,7 +106,8 @@ public void BotConfigurator_DeleteEndpoint_ShouldBeCalledIndependentlyOfWebApp() _mockBotConfigurator, _mockExecutor, _agentBlueprintService, - _mockConfirmationProvider); + _mockConfirmationProvider, + _federatedCredentialService); Assert.NotNull(command); Assert.Equal("cleanup", command.Name); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs index 08cea7e9..0be3e50d 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CleanupCommandTests.cs @@ -12,6 +12,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands; +[Collection("ConsoleOutput")] public class CleanupCommandTests { private readonly ILogger _mockLogger; @@ -20,6 +21,7 @@ public class CleanupCommandTests private readonly CommandExecutor _mockExecutor; private readonly GraphApiService _graphApiService; private readonly AgentBlueprintService _agentBlueprintService; + private readonly FederatedCredentialService _federatedCredentialService; private readonly IMicrosoftGraphTokenProvider _mockTokenProvider; private readonly IConfirmationProvider _mockConfirmationProvider; @@ -56,6 +58,10 @@ public CleanupCommandTests() var mockBlueprintLogger = Substitute.For>(); _agentBlueprintService = new AgentBlueprintService(mockBlueprintLogger, _graphApiService); + // Create FederatedCredentialService wrapping GraphApiService + var mockFicLogger = Substitute.For>(); + _federatedCredentialService = new FederatedCredentialService(mockFicLogger, _graphApiService); + // Mock confirmation provider - default to confirming (for most tests) _mockConfirmationProvider = Substitute.For(); _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); @@ -69,7 +75,7 @@ public async Task CleanupAzure_WithValidConfig_ShouldExecuteResourceDeleteComman var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "azure", "--config", "test.json" }; // Act @@ -98,7 +104,7 @@ public async Task CleanupInstance_WithValidConfig_ShouldReturnSuccess() _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "instance", "--config", "test.json" }; var originalIn = Console.In; @@ -129,7 +135,7 @@ public async Task Cleanup_WithoutSubcommand_ShouldExecuteCompleteCleanup() var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -159,7 +165,7 @@ public async Task CleanupAzure_WithMissingWebAppName_ShouldStillExecuteCommand() var config = CreateConfigWithMissingWebApp(); // Create config without web app name _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "azure", "--config", "test.json" }; // Act @@ -186,7 +192,7 @@ public async Task CleanupCommand_WithInvalidConfigFile_ShouldReturnError() _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(false)); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "azure", "--config", "invalid.json" }; // Act @@ -206,7 +212,7 @@ await _mockExecutor.DidNotReceive().ExecuteAsync( public void CleanupCommand_ShouldHaveCorrectSubcommands() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); // Assert - Verify command structure (what users see) Assert.Equal("cleanup", command.Name); @@ -225,7 +231,7 @@ public void CleanupCommand_ShouldHaveCorrectSubcommands() public void CleanupCommand_ShouldHaveDefaultHandlerOptions() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); // Assert - Verify parent command has options for default handler var optionNames = command.Options.Select(opt => opt.Name).ToList(); @@ -238,7 +244,7 @@ public void CleanupCommand_ShouldHaveDefaultHandlerOptions() public void CleanupSubcommands_ShouldHaveRequiredOptions() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var blueprintCommand = command.Subcommands.First(sc => sc.Name == "blueprint"); // Assert - Verify user-facing options @@ -255,7 +261,7 @@ public async Task CleanupBlueprint_WithValidConfig_ShouldReturnSuccess() var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--config", "test.json" }; // Act @@ -313,7 +319,7 @@ public async Task Cleanup_WhenUserDeclinesInitialConfirmation_ShouldAbortWithout // User declines the initial "Are you sure?" confirmation _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(false); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -341,7 +347,7 @@ public async Task Cleanup_WhenUserConfirmsButDoesNotTypeDelete_ShouldAbortWithou _mockConfirmationProvider.ConfirmAsync(Arg.Any()).Returns(true); _mockConfirmationProvider.ConfirmWithTypedResponseAsync(Arg.Any(), "DELETE").Returns(false); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -365,7 +371,7 @@ public async Task Cleanup_ShouldCallConfirmationProviderWithCorrectPrompts() var config = CreateValidConfig(); _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "--config", "test.json" }; // Act @@ -390,7 +396,8 @@ public void CleanupCommand_ShouldAcceptConfirmationProviderParameter() _mockBotConfigurator, _mockExecutor, _agentBlueprintService, - _mockConfirmationProvider); + _mockConfirmationProvider, + _federatedCredentialService); command.Should().NotBeNull(); command.Name.Should().Be("cleanup"); @@ -403,7 +410,7 @@ public void CleanupCommand_ShouldAcceptConfirmationProviderParameter() public void CleanupBlueprint_ShouldHaveEndpointOnlyOption() { // Arrange & Act - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var blueprintCommand = command.Subcommands.First(sc => sc.Name == "blueprint"); // Assert @@ -424,7 +431,7 @@ public async Task CleanupBlueprint_WithEndpointOnly_ShouldOnlyDeleteMessagingEnd _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Simulate user confirmation with y @@ -482,7 +489,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoBlueprintId_ShouldLogErr }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -520,7 +527,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoBotName_ShouldLogInfo() }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -559,7 +566,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndInvalidLocation_ShouldPass _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(false); // API will likely fail with invalid location - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -599,7 +606,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndApiException_ShouldHandleG _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromException(new InvalidOperationException("API connection failed"))); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -652,7 +659,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndWhitespaceBlueprint_Should }; _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(config); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; // Act @@ -679,7 +686,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndInvalidInput_ShouldCancelC _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -718,7 +725,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndNoResponse_ShouldCancelCle _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; @@ -757,7 +764,7 @@ public async Task CleanupBlueprint_WithEndpointOnlyAndEmptyInput_ShouldCancelCle _mockBotConfigurator.DeleteEndpointWithAgentBlueprintAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(true); - var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider); + var command = CleanupCommand.CreateCommand(_mockLogger, _mockConfigService, _mockBotConfigurator, _mockExecutor, _agentBlueprintService, _mockConfirmationProvider, _federatedCredentialService); var args = new[] { "cleanup", "blueprint", "--endpoint-only", "--config", "test.json" }; var originalIn = Console.In; 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 ad25a34b..f5685b5d 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 @@ -50,6 +50,173 @@ public async Task PollAdminConsentAsync_ReturnsFalse_WhenNoGrant() result.Should().BeFalse(); } + + [Fact] + public async Task CheckConsentExistsAsync_ReturnsTrue_WhenAllScopesGranted() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + // Mock grant with multiple scopes + var grantJson = """ + { + "value": [ + { + "id": "grant-123", + "scope": "User.Read Mail.Send Calendars.Read" + } + ] + } + """; + var grantDoc = JsonDocument.Parse(grantJson); + graphApiService.GraphGetAsync("tenant-1", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(grantDoc)); + + var requiredScopes = new[] { "User.Read", "Mail.Send" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "client-sp-123", "resource-sp-456", requiredScopes, logger, CancellationToken.None); + + result.Should().BeTrue(); + } + + [Fact] + public async Task CheckConsentExistsAsync_ReturnsFalse_WhenScopeMissing() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + // Mock grant with fewer scopes than required + var grantJson = """ + { + "value": [ + { + "id": "grant-123", + "scope": "User.Read" + } + ] + } + """; + var grantDoc = JsonDocument.Parse(grantJson); + graphApiService.GraphGetAsync("tenant-1", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(grantDoc)); + + var requiredScopes = new[] { "User.Read", "Mail.Send" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "client-sp-123", "resource-sp-456", requiredScopes, logger, CancellationToken.None); + + result.Should().BeFalse(); + } + + [Fact] + public async Task CheckConsentExistsAsync_IsCaseInsensitive() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + // Mock grant with different casing + var grantJson = """ + { + "value": [ + { + "id": "grant-123", + "scope": "user.read MAIL.SEND" + } + ] + } + """; + var grantDoc = JsonDocument.Parse(grantJson); + graphApiService.GraphGetAsync("tenant-1", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(grantDoc)); + + var requiredScopes = new[] { "User.Read", "Mail.Send" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "client-sp-123", "resource-sp-456", requiredScopes, logger, CancellationToken.None); + + result.Should().BeTrue(); + } + + [Fact] + public async Task CheckConsentExistsAsync_ReturnsFalse_WhenNoGrantsExist() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + // Mock empty grants response + var grantJson = """ + { + "value": [] + } + """; + var grantDoc = JsonDocument.Parse(grantJson); + graphApiService.GraphGetAsync("tenant-1", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(grantDoc)); + + var requiredScopes = new[] { "User.Read" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "client-sp-123", "resource-sp-456", requiredScopes, logger, CancellationToken.None); + + result.Should().BeFalse(); + } + + [Fact] + public async Task CheckConsentExistsAsync_ReturnsFalse_WhenClientSpIdMissing() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + var requiredScopes = new[] { "User.Read" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "", "resource-sp-456", requiredScopes, logger, CancellationToken.None); + + result.Should().BeFalse(); + } + + [Fact] + public async Task CheckConsentExistsAsync_ReturnsFalse_WhenResourceSpIdMissing() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + var requiredScopes = new[] { "User.Read" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "client-sp-123", string.Empty, requiredScopes, logger, CancellationToken.None); + + result.Should().BeFalse(); + } + + [Fact] + public async Task CheckConsentExistsAsync_ReturnsFalse_WhenGrantMissingScopeProperty() + { + var graphApiService = Substitute.For(Substitute.For>(), Substitute.For(Substitute.For>())); + var logger = Substitute.For(); + + // Mock grant without scope property + var grantJson = """ + { + "value": [ + { + "id": "grant-123" + } + ] + } + """; + var grantDoc = JsonDocument.Parse(grantJson); + graphApiService.GraphGetAsync("tenant-1", Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(grantDoc)); + + var requiredScopes = new[] { "User.Read" }; + + var result = await AdminConsentHelper.CheckConsentExistsAsync( + graphApiService, "tenant-1", "client-sp-123", "resource-sp-456", requiredScopes, logger, CancellationToken.None); + + result.Should().BeFalse(); + } } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs index e3afc6e9..2eb3233b 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/BlueprintLookupServiceTests.cs @@ -150,7 +150,7 @@ public async Task GetApplicationByDisplayNameAsync_EscapesSingleQuotes() .Returns(jsonDoc); // Act - var result = await _service.GetApplicationByDisplayNameAsync(TestTenantId, displayNameWithQuotes); + await _service.GetApplicationByDisplayNameAsync(TestTenantId, displayNameWithQuotes); // Assert await _graphApiService.Received(1).GraphGetAsync( diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs index b4167b23..ea729d71 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/FederatedCredentialServiceTests.cs @@ -25,7 +25,13 @@ public class FederatedCredentialServiceTests public FederatedCredentialServiceTests() { _logger = Substitute.For>(); - _graphApiService = Substitute.For(); + + // Use ForPartsOf to create a partial mock of the concrete GraphApiService class + // This allows mocking of virtual methods (GraphGetAsync, GraphPostWithResponseAsync) + var mockLogger = Substitute.For>(); + var mockExecutor = Substitute.ForPartsOf(Substitute.For>()); + _graphApiService = Substitute.ForPartsOf(mockLogger, mockExecutor); + _service = new FederatedCredentialService(_logger, _graphApiService); } @@ -223,7 +229,7 @@ public async Task CreateFederatedCredentialAsync_WhenSuccessful_ReturnsSuccess() _graphApiService.GraphPostWithResponseAsync( TestTenantId, $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", - Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any?>()) .Returns(successResponse); @@ -260,7 +266,7 @@ public async Task CreateFederatedCredentialAsync_WhenHttp409Conflict_ReturnsSucc _graphApiService.GraphPostWithResponseAsync( TestTenantId, Arg.Any(), - Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any?>()) .Returns(conflictResponse); @@ -390,7 +396,7 @@ public async Task GetFederatedCredentialsAsync_OnException_ReturnsEmptyList() // Arrange _graphApiService.GraphGetAsync( TestTenantId, - $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any(), Arg.Any()) .Throws(new Exception("Network error")); @@ -401,4 +407,112 @@ public async Task GetFederatedCredentialsAsync_OnException_ReturnsEmptyList() result.Should().NotBeNull(); result.Should().BeEmpty(); } + + [Fact] + public async Task GetFederatedCredentialsAsync_WhenStandardEndpointReturnsEmpty_TriesFallbackEndpoint() + { + // Arrange + var standardEndpoint = $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials"; + var fallbackEndpoint = $"/beta/applications/microsoft.graph.agentIdentityBlueprint/{TestBlueprintObjectId}/federatedIdentityCredentials"; + + // Standard endpoint returns empty array + var emptyResponse = @"{""value"": []}"; + var emptyJsonDoc = JsonDocument.Parse(emptyResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + standardEndpoint, + Arg.Any()) + .Returns(emptyJsonDoc); + + // Fallback endpoint returns credentials + var fallbackResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""cred-id-1"", + ""name"": ""AgentBlueprintCredential"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""{TestMsiPrincipalId}"", + ""audiences"": [""api://AzureADTokenExchange""] + }} + ] + }}"; + var fallbackJsonDoc = JsonDocument.Parse(fallbackResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + fallbackEndpoint, + Arg.Any()) + .Returns(fallbackJsonDoc); + + // Act + var result = await _service.GetFederatedCredentialsAsync(TestTenantId, TestBlueprintObjectId); + + // Assert + result.Should().NotBeNull(); + result.Should().HaveCount(1); + result[0].Name.Should().Be("AgentBlueprintCredential"); + result[0].Subject.Should().Be(TestMsiPrincipalId); + + // Verify both endpoints were called + await _graphApiService.Received(1).GraphGetAsync( + TestTenantId, + standardEndpoint, + Arg.Any()); + + await _graphApiService.Received(1).GraphGetAsync( + TestTenantId, + fallbackEndpoint, + Arg.Any()); + } + + [Fact] + public async Task GetFederatedCredentialsAsync_WithMalformedCredentials_ReturnsOnlyValidOnes() + { + // Arrange - JSON response with mixed valid and malformed credentials + var jsonResponse = $@"{{ + ""value"": [ + {{ + ""id"": ""cred-id-1"", + ""name"": ""ValidCredential1"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""{TestMsiPrincipalId}"", + ""audiences"": [""api://AzureADTokenExchange""] + }}, + {{ + ""id"": ""cred-id-2"", + ""name"": ""MissingSubject"" + }}, + {{ + ""id"": ""cred-id-3"", + ""name"": ""ValidCredential2"", + ""issuer"": ""{TestIssuer}"", + ""subject"": ""different-principal"", + ""audiences"": [""api://AzureADTokenExchange""] + }}, + {{ + ""issuer"": ""{TestIssuer}"", + ""subject"": ""another-principal"" + }} + ] + }}"; + var jsonDoc = JsonDocument.Parse(jsonResponse); + + _graphApiService.GraphGetAsync( + TestTenantId, + $"/beta/applications/{TestBlueprintObjectId}/federatedIdentityCredentials", + Arg.Any()) + .Returns(jsonDoc); + + // Act + var result = await _service.GetFederatedCredentialsAsync(TestTenantId, TestBlueprintObjectId); + + // Assert + result.Should().NotBeNull(); + result.Should().HaveCount(2); // Only the 2 valid credentials + result[0].Name.Should().Be("ValidCredential1"); + result[0].Subject.Should().Be(TestMsiPrincipalId); + result[1].Name.Should().Be("ValidCredential2"); + result[1].Subject.Should().Be("different-principal"); + } } From 95e9b1d286f3be4ad859f50ebb24571586bf5d99 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 7 Jan 2026 10:46:52 -0800 Subject: [PATCH 3/4] Refactor: move model classes to dedicated files Moved blueprint, federated credential, and password credential model classes from service classes to new files in the Models namespace for better organization. Updated services to use the new models. Improved logging for messaging endpoint setup and Azure CLI Python bitness detection. --- .../SetupSubcommands/BlueprintSubcommand.cs | 1 + .../Commands/SetupSubcommands/SetupHelpers.cs | 2 +- .../Models/BlueprintLookupModels.cs | 32 ++++++++++++++++ .../Models/FederatedCredentialModels.cs | 37 +++++++++++++++++++ .../Models/PasswordCredentialInfo.cs | 19 ++++++++++ .../Services/AgentBlueprintService.cs | 15 -------- .../Services/AzureEnvironmentValidator.cs | 6 ++- .../Services/BlueprintLookupService.cs | 29 +-------------- .../Services/FederatedCredentialService.cs | 34 +---------------- 9 files changed, 96 insertions(+), 79 deletions(-) create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Models/BlueprintLookupModels.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Models/FederatedCredentialModels.cs create mode 100644 src/Microsoft.Agents.A365.DevTools.Cli/Models/PasswordCredentialInfo.cs diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index c2f0d209..b802cd3b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -6,6 +6,7 @@ using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Exceptions; using Microsoft.Agents.A365.DevTools.Cli.Helpers; +using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Agents.A365.DevTools.Cli.Services; using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; 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 9a639582..1bbffdb4 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -162,7 +162,7 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) if (!results.MessagingEndpointRegistered) { logger.LogInformation(" - Messaging Endpoint: Run 'a365 setup blueprint --endpoint-only' to retry"); - logger.LogInformation(" Or delete conflicting endpoint first: a365 cleanup blueprint"); + logger.LogInformation(" If there's a conflicting endpoint, delete it first: a365 cleanup blueprint --endpoint-only"); } } else if (results.HasWarnings) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/BlueprintLookupModels.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/BlueprintLookupModels.cs new file mode 100644 index 00000000..a421183a --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/BlueprintLookupModels.cs @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.DevTools.Cli.Models; + +/// +/// Result of blueprint application lookup operation. +/// +public class BlueprintLookupResult +{ + public bool Found { get; set; } + public string? ObjectId { get; set; } + public string? AppId { get; set; } + public string? DisplayName { get; set; } + public string? LookupMethod { get; set; } + public bool RequiresPersistence { get; set; } + public string? ErrorMessage { get; set; } +} + +/// +/// Result of service principal lookup operation. +/// +public class ServicePrincipalLookupResult +{ + public bool Found { get; set; } + public string? ObjectId { get; set; } + public string? AppId { get; set; } + public string? DisplayName { get; set; } + public string? LookupMethod { get; set; } + public bool RequiresPersistence { get; set; } + public string? ErrorMessage { get; set; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/FederatedCredentialModels.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/FederatedCredentialModels.cs new file mode 100644 index 00000000..9d7f2be1 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/FederatedCredentialModels.cs @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.DevTools.Cli.Models; + +/// +/// Information about a federated identity credential. +/// +public class FederatedCredentialInfo +{ + public string? Id { get; set; } + public string? Name { get; set; } + public string? Issuer { get; set; } + public string? Subject { get; set; } + public List Audiences { get; set; } = new(); +} + +/// +/// Result of checking if a federated credential exists. +/// +public class FederatedCredentialCheckResult +{ + public bool Exists { get; set; } + public FederatedCredentialInfo? ExistingCredential { get; set; } + public string? ErrorMessage { get; set; } +} + +/// +/// Result of creating a federated credential. +/// +public class FederatedCredentialCreateResult +{ + public bool Success { get; set; } + public bool AlreadyExisted { get; set; } + public string? ErrorMessage { get; set; } + public bool ShouldRetry { get; set; } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Models/PasswordCredentialInfo.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PasswordCredentialInfo.cs new file mode 100644 index 00000000..56f7c720 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Models/PasswordCredentialInfo.cs @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.DevTools.Cli.Models; + +/// +/// Information about a password credential (client secret). +/// Note: The actual secret value cannot be retrieved from Graph API. +/// +public class PasswordCredentialInfo +{ + public string? DisplayName { get; set; } + public string? Hint { get; set; } + public string? KeyId { get; set; } + public DateTime? EndDateTime { get; set; } + + public bool IsExpired => EndDateTime.HasValue && EndDateTime.Value < DateTime.UtcNow; + public bool IsExpiringSoon => EndDateTime.HasValue && EndDateTime.Value < DateTime.UtcNow.AddDays(30); +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs index f2bd8fce..fa973f54 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AgentBlueprintService.cs @@ -729,18 +729,3 @@ private static List ParseInheritableScopesFromJson(JsonElement entry) return scopesList; } } - -/// -/// Information about a password credential (client secret). -/// Note: The actual secret value cannot be retrieved from Graph API. -/// -public class PasswordCredentialInfo -{ - public string? DisplayName { get; set; } - public string? Hint { get; set; } - public string? KeyId { get; set; } - public DateTime? EndDateTime { get; set; } - - public bool IsExpired => EndDateTime.HasValue && EndDateTime.Value < DateTime.UtcNow; - public bool IsExpiringSoon => EndDateTime.HasValue && EndDateTime.Value < DateTime.UtcNow.AddDays(30); -} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs index 2a15a48a..4c9d273e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureEnvironmentValidator.cs @@ -62,11 +62,13 @@ private async Task ValidateAzureCliArchitectureAsync() // Check if Azure CLI is using 32-bit Python on 64-bit Windows if (result.StandardOutput.Contains("32 bit", StringComparison.OrdinalIgnoreCase)) { - _logger.LogDebug("Azure CLI is using 32-bit Python (performance may be affected)"); + _logger.LogWarning("Azure CLI is using 32-bit Python on 64-bit Windows (performance may be affected)"); + _logger.LogWarning("For optimal performance, consider reinstalling Azure CLI with 64-bit Python"); + _logger.LogWarning("For more information, see: https://learn.microsoft.com/cli/azure/install-azure-cli-windows"); } else if (result.StandardOutput.Contains("64 bit", StringComparison.OrdinalIgnoreCase)) { - _logger.LogDebug("Azure CLI is using 64-bit Python (optimal)"); + _logger.LogInformation("Azure CLI is using 64-bit Python (optimal)"); } } } \ No newline at end of file diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs index bd13ffc5..8d40d2ea 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/BlueprintLookupService.cs @@ -3,6 +3,7 @@ using System.Text.Json; using Microsoft.Extensions.Logging; +using Microsoft.Agents.A365.DevTools.Cli.Models; namespace Microsoft.Agents.A365.DevTools.Cli.Services; @@ -305,31 +306,3 @@ public async Task GetServicePrincipalByAppIdAsync( } } } - -/// -/// Result of blueprint application lookup operation. -/// -public class BlueprintLookupResult -{ - public bool Found { get; set; } - public string? ObjectId { get; set; } - public string? AppId { get; set; } - public string? DisplayName { get; set; } - public string? LookupMethod { get; set; } - public bool RequiresPersistence { get; set; } - public string? ErrorMessage { get; set; } -} - -/// -/// Result of service principal lookup operation. -/// -public class ServicePrincipalLookupResult -{ - public bool Found { get; set; } - public string? ObjectId { get; set; } - public string? AppId { get; set; } - public string? DisplayName { get; set; } - public string? LookupMethod { get; set; } - public bool RequiresPersistence { get; set; } - public string? ErrorMessage { get; set; } -} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs index 72e91c05..928072d0 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/FederatedCredentialService.cs @@ -3,6 +3,7 @@ using System.Text.Json; using Microsoft.Extensions.Logging; +using Microsoft.Agents.A365.DevTools.Cli.Models; namespace Microsoft.Agents.A365.DevTools.Cli.Services; @@ -471,36 +472,3 @@ public async Task DeleteAllFederatedCredentialsAsync( } } } - -/// -/// Information about a federated identity credential. -/// -public class FederatedCredentialInfo -{ - public string? Id { get; set; } - public string? Name { get; set; } - public string? Issuer { get; set; } - public string? Subject { get; set; } - public List Audiences { get; set; } = new(); -} - -/// -/// Result of checking if a federated credential exists. -/// -public class FederatedCredentialCheckResult -{ - public bool Exists { get; set; } - public FederatedCredentialInfo? ExistingCredential { get; set; } - public string? ErrorMessage { get; set; } -} - -/// -/// Result of creating a federated credential. -/// -public class FederatedCredentialCreateResult -{ - public bool Success { get; set; } - public bool AlreadyExisted { get; set; } - public string? ErrorMessage { get; set; } - public bool ShouldRetry { get; set; } -} From c9893465768483cab940d45e3f309c7a1dd33c63 Mon Sep 17 00:00:00 2001 From: Sellakumaran Kanagarathnam Date: Wed, 7 Jan 2026 11:02:18 -0800 Subject: [PATCH 4/4] Refactor FIC naming and logging; clean up retry logic - Update FIC naming to use display name only (no objectId), as uniqueness is per app, not tenant-wide. - Improve error logging for missing blueprint identifiers by removing null-coalescing fallback. - Remove unused variable assignment from federated credential retry logic. --- .../SetupSubcommands/BlueprintSubcommand.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs index b802cd3b..a7833a34 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -626,7 +626,7 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( if (string.IsNullOrWhiteSpace(existingAppId) || string.IsNullOrWhiteSpace(existingObjectId)) { logger.LogError("Existing blueprint found but required identifiers are missing (AppId: {AppId}, ObjectId: {ObjectId})", - existingAppId ?? "(null)", existingObjectId ?? "(null)"); + existingAppId, existingObjectId); return (false, null, null, null, alreadyExisted: false); } @@ -932,10 +932,9 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( if (useManagedIdentity && !string.IsNullOrWhiteSpace(managedIdentityPrincipalId)) { logger.LogInformation("Configuring Federated Identity Credential for Managed Identity..."); - // Include full blueprint object ID to ensure absolute uniqueness across tenant - // FIC names have tenant-wide uniqueness constraints, not scoped to individual blueprints - // Using full objectId guarantees no collisions even with rapid create/delete cycles - var credentialName = $"{displayName.Replace(" ", "")}-{objectId}-MSI"; + // Federated credential names are scoped to the application and only need to be unique per app. + // Use a readable name based on the display name, with whitespace removed and "-MSI" suffix. + var credentialName = $"{displayName.Replace(" ", "")}-MSI"; // For existing blueprints, check if FIC already exists to provide better UX // For new blueprints, we skip this and go straight to create (avoiding race conditions) @@ -969,7 +968,7 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( var retryHelper = new RetryHelper(logger); FederatedCredentialCreateResult? ficCreateResult = null; - var ficReady = await retryHelper.ExecuteWithRetryAsync( + await retryHelper.ExecuteWithRetryAsync( async ct => { ficCreateResult = await federatedCredentialService.CreateFederatedCredentialAsync( @@ -1018,7 +1017,7 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( var retryHelper = new RetryHelper(logger); FederatedCredentialCreateResult? ficCreateResult = null; - var ficReady = await retryHelper.ExecuteWithRetryAsync( + await retryHelper.ExecuteWithRetryAsync( async ct => { ficCreateResult = await federatedCredentialService.CreateFederatedCredentialAsync(