diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs index fce13d0a..20674962 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommand.cs @@ -9,7 +9,6 @@ using Microsoft.Agents.A365.DevTools.Cli.Services.Helpers; using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; using Microsoft.Extensions.Logging; -using Microsoft.Identity.Client; using System.CommandLine; using System.IO.Compression; using System.Net.Http.Headers; @@ -355,66 +354,6 @@ public static Command CreateCommand( mosToken = await mosTokenService.AcquireTokenAsync(mosEnv, mosPersonalToken); logger.LogDebug("MOS token acquired successfully"); } - catch (MsalServiceException ex) when (ex.ErrorCode == "invalid_client" && - ex.Message.Contains("AADSTS650052")) - { - logger.LogError("MOS token acquisition failed: Missing service principal or admin consent (Error: {ErrorCode})", ex.ErrorCode); - logger.LogInformation(""); - logger.LogInformation("The MOS service principals exist, but admin consent may not be granted."); - logger.LogInformation("Grant admin consent at:"); - logger.LogInformation(" {PortalUrl}", - MosConstants.GetApiPermissionsPortalUrl(config.ClientAppId)); - logger.LogInformation(""); - logger.LogInformation("Or authenticate interactively and consent when prompted."); - logger.LogInformation(""); - return; - } - catch (MsalServiceException ex) when (ex.ErrorCode == "unauthorized_client" && - ex.Message.Contains("AADSTS50194")) - { - logger.LogError("MOS token acquisition failed: Single-tenant app cannot use /common endpoint (Error: {ErrorCode})", ex.ErrorCode); - logger.LogInformation(""); - logger.LogInformation("AADSTS50194: The application is configured as single-tenant but is trying to use the /common authority."); - logger.LogInformation("This should be automatically handled by using tenant-specific authority URLs."); - logger.LogInformation(""); - logger.LogInformation("If this error persists:"); - logger.LogInformation("1. Verify your app registration is configured correctly in Azure Portal"); - logger.LogInformation("2. Check that tenantId in a365.config.json matches your app's home tenant"); - logger.LogInformation("3. Ensure the app's 'Supported account types' setting matches your use case"); - logger.LogInformation(""); - return; - } - catch (MsalServiceException ex) when (ex.ErrorCode == "invalid_grant") - { - logger.LogError("MOS token acquisition failed: Invalid or expired credentials (Error: {ErrorCode})", ex.ErrorCode); - logger.LogInformation(""); - logger.LogInformation("The authentication failed due to invalid credentials or expired tokens."); - logger.LogInformation("Try clearing the token cache and re-authenticating:"); - logger.LogInformation(" - Delete: ~/.a365/mos-token-cache.json"); - logger.LogInformation(" - Run: a365 publish"); - logger.LogInformation(""); - return; - } - catch (MsalServiceException ex) - { - // Log all MSAL-specific errors with full context for debugging - logger.LogError("MOS token acquisition failed with MSAL error"); - logger.LogError("Error Code: {ErrorCode}", ex.ErrorCode); - logger.LogError("Error Message: {Message}", ex.Message); - logger.LogDebug("Stack Trace: {StackTrace}", ex.StackTrace); - - logger.LogInformation(""); - logger.LogInformation("Authentication failed. Common issues:"); - logger.LogInformation("1. Missing admin consent - Grant at:"); - logger.LogInformation(" {PortalUrl}", - MosConstants.GetApiPermissionsPortalUrl(config.ClientAppId)); - logger.LogInformation("2. Insufficient permissions - Verify required API permissions are configured"); - logger.LogInformation("3. Tenant configuration - Ensure app registration matches your tenant setup"); - logger.LogInformation(""); - logger.LogInformation("For detailed troubleshooting, search for error code: {ErrorCode}", ex.ErrorCode); - logger.LogInformation(""); - return; - } catch (Exception ex) { logger.LogError(ex, "Failed to acquire MOS token: {Message}", ex.Message); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs index 08a913b1..7a6aef71 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs @@ -21,7 +21,6 @@ public static Command CreateCommand( DeploymentService deploymentService, IBotConfigurator botConfigurator, IAzureValidator azureValidator, - AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, @@ -46,16 +45,16 @@ public static Command CreateCommand( logger, configService, clientAppValidator)); command.AddCommand(InfrastructureSubcommand.CreateCommand( - logger, configService, azureValidator, webAppCreator, platformDetector, executor)); + logger, configService, azureValidator, platformDetector, executor)); command.AddCommand(BlueprintSubcommand.CreateCommand( - logger, configService, executor, azureValidator, webAppCreator, platformDetector, botConfigurator, graphApiService, blueprintService, clientAppValidator, blueprintLookupService, federatedCredentialService)); + logger, configService, executor, azureValidator, 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, blueprintLookupService, federatedCredentialService)); + logger, configService, executor, botConfigurator, azureValidator, 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 b67534c2..39f6a421 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -27,7 +27,6 @@ public static Command CreateCommand( CommandExecutor executor, IBotConfigurator botConfigurator, IAzureValidator azureValidator, - AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, GraphApiService graphApiService, AgentBlueprintService blueprintService, 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 c1317519..c6dc5cb6 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -116,7 +116,6 @@ public static Command CreateCommand( IConfigService configService, CommandExecutor executor, IAzureValidator azureValidator, - AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, IBotConfigurator botConfigurator, GraphApiService graphApiService, 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 dfe369c2..7731bf0e 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/InfrastructureSubcommand.cs @@ -102,7 +102,6 @@ public static Command CreateCommand( ILogger logger, IConfigService configService, IAzureValidator azureValidator, - AzureWebAppCreator webAppCreator, PlatformDetector platformDetector, CommandExecutor executor) { diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/MsalHelper.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/MsalHelper.cs new file mode 100644 index 00000000..1914965b --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Helpers/MsalHelper.cs @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using Microsoft.Identity.Client; + +namespace Microsoft.Agents.A365.DevTools.Cli.Helpers; + +/// +/// Shared helpers for MSAL-based authentication flows used across multiple services. +/// +public static class MsalHelper +{ + /// + /// Creates the standard MSAL device code callback that logs the verification URL and user code. + /// Shared across all interactive auth flows so the device code prompt is consistent. + /// + /// Optional logger. When null, falls back to . + public static Func CreateDeviceCodeCallback(ILogger? logger) + { + return deviceCode => + { + if (logger != null) + { + logger.LogInformation(""); + logger.LogInformation("=========================================================================="); + logger.LogInformation("To sign in, use a web browser to open the page:"); + logger.LogInformation(" {VerificationUrl}", deviceCode.VerificationUrl); + logger.LogInformation(""); + logger.LogInformation("And enter the code: {UserCode}", deviceCode.UserCode); + logger.LogInformation("=========================================================================="); + logger.LogInformation(""); + } + else + { + Console.Error.WriteLine(); + Console.Error.WriteLine("=========================================================================="); + Console.Error.WriteLine("To sign in, use a web browser to open the page:"); + Console.Error.WriteLine($" {deviceCode.VerificationUrl}"); + Console.Error.WriteLine(); + Console.Error.WriteLine($"And enter the code: {deviceCode.UserCode}"); + Console.Error.WriteLine("=========================================================================="); + Console.Error.WriteLine(); + } + return Task.CompletedTask; + }; + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs index fa64b85b..2b6e2087 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Program.cs @@ -102,7 +102,6 @@ static async Task Main(string[] args) var agentBlueprintService = serviceProvider.GetRequiredService(); var blueprintLookupService = serviceProvider.GetRequiredService(); var federatedCredentialService = serviceProvider.GetRequiredService(); - var webAppCreator = serviceProvider.GetRequiredService(); var platformDetector = serviceProvider.GetRequiredService(); var processService = serviceProvider.GetRequiredService(); var clientAppValidator = serviceProvider.GetRequiredService(); @@ -111,7 +110,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, blueprintLookupService, federatedCredentialService, clientAppValidator)); + deploymentService, botConfigurator, azureValidator, platformDetector, graphApiService, agentBlueprintService, blueprintLookupService, federatedCredentialService, clientAppValidator)); rootCommand.AddCommand(CreateInstanceCommand.CreateCommand(createInstanceLogger, configService, executor, botConfigurator, graphApiService, azureValidator)); rootCommand.AddCommand(DeployCommand.CreateCommand(deployLogger, configService, executor, @@ -253,9 +252,6 @@ private static void ConfigureServices(IServiceCollection services, LogLevel mini services.AddSingleton(); // For AgentApplication.Create permission services.AddSingleton(); // For publish command template extraction - // Register AzureWebAppCreator for SDK-based web app creation - services.AddSingleton(); - // Register ProcessService for cross-platform process launching services.AddSingleton(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureWebAppCreator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureWebAppCreator.cs deleted file mode 100644 index 41f284d0..00000000 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/AzureWebAppCreator.cs +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System; -using System.Threading.Tasks; -using Azure.Core; -using Azure.Identity; -using Azure.ResourceManager; -using Azure.ResourceManager.AppService; -using Azure.ResourceManager.AppService.Models; -using Azure.ResourceManager.Resources; -using Microsoft.Extensions.Logging; - -namespace Microsoft.Agents.A365.DevTools.Cli.Services -{ - public class AzureWebAppCreator - { - private readonly ILogger _logger; - - public AzureWebAppCreator(ILogger logger) - { - _logger = logger; - } - - public async Task CreateWebAppAsync( - string subscriptionId, - string resourceGroupName, - string appServicePlanName, - string webAppName, - string location, - string? tenantId = null) - { - try - { - ArmClient armClient; - // Use DefaultAzureCredential which tries credentials in this order: - // 1. Environment variables - // 2. Managed Identity - // 3. Visual Studio / VS Code - // 4. Azure CLI (az login) - // 5. Interactive Browser (if needed) - var credentialOptions = new DefaultAzureCredentialOptions - { - ExcludeInteractiveBrowserCredential = false - }; - - if (!string.IsNullOrWhiteSpace(tenantId)) - { - credentialOptions.TenantId = tenantId; - } - - armClient = new ArmClient(new DefaultAzureCredential(credentialOptions), subscriptionId); - - var subscription = armClient.GetSubscriptionResource(new ResourceIdentifier($"/subscriptions/{subscriptionId}")); - var resourceGroup = await subscription.GetResourceGroups().GetAsync(resourceGroupName); - - // Get the App Service plan - var appServicePlan = await resourceGroup.Value.GetAppServicePlans().GetAsync(appServicePlanName); - - // Prepare the web app data - var webAppData = new WebSiteData(location) - { - AppServicePlanId = appServicePlan.Value.Id, - SiteConfig = new SiteConfigProperties - { - LinuxFxVersion = "DOTNETCORE|8.0" - }, - Kind = "app,linux" - }; - - // Create the web app - var webAppLro = await resourceGroup.Value.GetWebSites().CreateOrUpdateAsync( - Azure.WaitUntil.Completed, - webAppName, - webAppData); - - _logger.LogInformation("Web app '{WebAppName}' created successfully.", webAppName); - return true; - } - catch (Exception ex) - { - _logger.LogError(ex, "Failed to create web app '{WebAppName}'.", webAppName); - return false; - } - } - } -} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs index fb04aee9..210ebd00 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/Internal/MicrosoftGraphTokenProvider.cs @@ -288,7 +288,7 @@ private async Task ExecuteWithFallbackAsync( { if (string.IsNullOrWhiteSpace(clientAppId)) { - _logger.LogDebug("No client app ID available for MSAL Graph fallback."); + _logger.LogWarning("MSAL Graph fallback skipped: no client app ID available. Ensure ClientAppId is set in a365.config.json."); return null; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs index ac82e18e..7a8336dd 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MosTokenService.cs @@ -1,9 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Globalization; +using Azure.Core; using Microsoft.Agents.A365.DevTools.Cli.Constants; -using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Extensions.Logging; using Microsoft.Identity.Client; @@ -11,51 +10,36 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Services; /// /// Native C# service for acquiring MOS (Microsoft Office Store) tokens. -/// Replaces GetToken.ps1 PowerShell script. +/// Delegates to for interactive authentication +/// with automatic device code fallback, leveraging MSAL's built-in token cache. /// public class MosTokenService { private readonly ILogger _logger; private readonly IConfigService _configService; - private readonly string _cacheFilePath; public MosTokenService(ILogger logger, IConfigService configService) { _logger = logger; _configService = configService; - - // Store token cache in user's home directory for security - // Avoid current directory which may have shared/inappropriate permissions - var cacheDir = FileHelper.GetSecureCrossOsDirectory(); - _cacheFilePath = Path.Combine(cacheDir, "mos-token-cache.json"); } /// /// Acquire MOS token for the specified environment. - /// Uses MSAL.NET for interactive authentication with caching. + /// Uses for interactive authentication with caching. /// public async Task AcquireTokenAsync(string environment, string? personalToken = null, CancellationToken cancellationToken = default) { environment = environment.ToLowerInvariant().Trim(); - // If personal token provided, use it directly (no caching) if (!string.IsNullOrWhiteSpace(personalToken)) { _logger.LogInformation("Using provided personal MOS token override"); return personalToken.Trim(); } - // Try cache first - var cached = TryGetCachedToken(environment); - if (cached.HasValue) - { - _logger.LogInformation("Using cached MOS token (valid until {Expiry:u})", cached.Value.Expiry); - return cached.Value.Token; - } - - // Load config to get tenant ID var setupConfig = await _configService.LoadAsync(); - if (setupConfig == null) + if (setupConfig is null) { _logger.LogError("Configuration not found. Run 'a365 config init' first."); return null; @@ -67,58 +51,45 @@ public MosTokenService(ILogger logger, IConfigService configSer return null; } - // Use Microsoft first-party client app for MOS token acquisition - // This is required because MOS APIs only accept tokens from first-party apps - var mosClientAppId = MosConstants.TpsAppServicesClientAppId; - _logger.LogDebug("Using Microsoft first-party client app for MOS tokens: {ClientAppId}", mosClientAppId); - - // Get environment-specific configuration - var config = GetEnvironmentConfig(environment, mosClientAppId, setupConfig.TenantId); - if (config == null) + var config = GetEnvironmentConfig(environment, MosConstants.TpsAppServicesClientAppId, setupConfig.TenantId); + if (config is null) { _logger.LogError("Unsupported MOS environment: {Environment}", environment); return null; } - // Acquire new token using MSAL.NET try { _logger.LogInformation("Acquiring MOS token for environment: {Environment}", environment); - _logger.LogInformation("A browser window will open for authentication..."); - - var app = PublicClientApplicationBuilder - .Create(config.ClientId) - .WithAuthority(config.Authority) - .WithRedirectUri(MosConstants.RedirectUri) - .Build(); - - var result = await app - .AcquireTokenInteractive(new[] { config.Scope }) - .WithPrompt(Prompt.SelectAccount) - .ExecuteAsync(cancellationToken); - - if (result?.AccessToken == null) - { - _logger.LogError("Failed to acquire MOS token"); - return null; - } - // Log the scopes in the token for debugging - if (result.Scopes != null && result.Scopes.Any()) + // useWam: false because TpsAppServicesClientAppId is a Microsoft first-party app. + // WAM would override the redirect URI to the WAM broker format, which is not + // registered for this app. The original flow used a system browser redirect. + var credential = new MsalBrowserCredential( + config.ClientId, + setupConfig.TenantId, + redirectUri: MosConstants.RedirectUri, + logger: _logger, + useWam: false, + authority: config.Authority); + + var tokenRequestContext = new TokenRequestContext(new[] { config.Scope }); + var token = await credential.GetTokenAsync(tokenRequestContext, cancellationToken); + + _logger.LogInformation("MOS token acquired successfully (expires {Expiry:u})", token.ExpiresOn.UtcDateTime); + return token.Token; + } + catch (MsalAuthenticationFailedException ex) + { + if (ex.InnerException is MsalServiceException msalEx) { - _logger.LogDebug("Token scopes: {Scopes}", string.Join(", ", result.Scopes)); + LogMsalServiceError(msalEx, config.ClientId); } else { - _logger.LogWarning("Token has no scopes property"); + _logger.LogError("Failed to acquire MOS token: {Message}", ex.Message); } - - // Cache the token - var expiry = result.ExpiresOn.UtcDateTime; - CacheToken(environment, result.AccessToken, expiry); - - _logger.LogInformation("MOS token acquired successfully (expires {Expiry:u})", expiry); - return result.AccessToken; + return null; } catch (Exception ex) { @@ -127,6 +98,57 @@ public MosTokenService(ILogger logger, IConfigService configSer } } + private void LogMsalServiceError(MsalServiceException ex, string clientAppId) + { + if (ex.ErrorCode == "invalid_client" && ex.Message.Contains("AADSTS650052")) + { + _logger.LogError("MOS token acquisition failed: Missing service principal or admin consent (Error: {ErrorCode})", ex.ErrorCode); + _logger.LogInformation(""); + _logger.LogInformation("The MOS service principals exist, but admin consent may not be granted."); + _logger.LogInformation("Grant admin consent at:"); + _logger.LogInformation(" {PortalUrl}", MosConstants.GetApiPermissionsPortalUrl(clientAppId)); + _logger.LogInformation(""); + _logger.LogInformation("Or authenticate interactively and consent when prompted."); + _logger.LogInformation(""); + } + else if (ex.ErrorCode == "unauthorized_client" && ex.Message.Contains("AADSTS50194")) + { + _logger.LogError("MOS token acquisition failed: Single-tenant app cannot use /common endpoint (Error: {ErrorCode})", ex.ErrorCode); + _logger.LogInformation(""); + _logger.LogInformation("AADSTS50194: The application is configured as single-tenant but is trying to use the /common authority."); + _logger.LogInformation("This should be automatically handled by using tenant-specific authority URLs."); + _logger.LogInformation(""); + _logger.LogInformation("If this error persists:"); + _logger.LogInformation("1. Verify your app registration is configured correctly in Azure Portal"); + _logger.LogInformation("2. Check that tenantId in a365.config.json matches your app's home tenant"); + _logger.LogInformation("3. Ensure the app's 'Supported account types' setting matches your use case"); + _logger.LogInformation(""); + } + else if (ex.ErrorCode == "invalid_grant") + { + _logger.LogError("MOS token acquisition failed: Invalid or expired credentials (Error: {ErrorCode})", ex.ErrorCode); + _logger.LogInformation(""); + _logger.LogInformation("The authentication failed due to invalid credentials or expired tokens."); + _logger.LogInformation("Re-run the command to re-authenticate."); + _logger.LogInformation(""); + } + else + { + _logger.LogError("MOS token acquisition failed with MSAL error"); + _logger.LogError("Error Code: {ErrorCode}", ex.ErrorCode); + _logger.LogError("Error Message: {Message}", ex.Message); + _logger.LogInformation(""); + _logger.LogInformation("Authentication failed. Common issues:"); + _logger.LogInformation("1. Missing admin consent - Grant at:"); + _logger.LogInformation(" {PortalUrl}", MosConstants.GetApiPermissionsPortalUrl(clientAppId)); + _logger.LogInformation("2. Insufficient permissions - Verify required API permissions are configured"); + _logger.LogInformation("3. Tenant configuration - Ensure app registration matches your tenant setup"); + _logger.LogInformation(""); + _logger.LogInformation("For detailed troubleshooting, search for error code: {ErrorCode}", ex.ErrorCode); + _logger.LogInformation(""); + } + } + private MosEnvironmentConfig? GetEnvironmentConfig(string environment, string clientAppId, string tenantId) { // Use tenant-specific authority to support single-tenant apps (AADSTS50194 fix) @@ -175,84 +197,6 @@ public MosTokenService(ILogger logger, IConfigService configSer }; } - private (string Token, DateTime Expiry)? TryGetCachedToken(string environment) - { - try - { - if (!File.Exists(_cacheFilePath)) - return null; - - var json = File.ReadAllText(_cacheFilePath); - using var doc = System.Text.Json.JsonDocument.Parse(json); - - if (doc.RootElement.TryGetProperty(environment, out var envElement)) - { - var token = envElement.TryGetProperty("token", out var t) ? t.GetString() : null; - var expiryStr = envElement.TryGetProperty("expiry", out var e) ? e.GetString() : null; - - if (!string.IsNullOrWhiteSpace(token) && DateTime.TryParse(expiryStr, CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal, out var expiry)) - { - // Return cached token if valid for at least 2 more minutes - if (DateTime.UtcNow < expiry.AddMinutes(-2)) - { - return (token, expiry); - } - } - } - - return null; - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Failed to read token cache"); - return null; - } - } - - private void CacheToken(string environment, string token, DateTime expiry) - { - try - { - var cache = new Dictionary(); - - if (File.Exists(_cacheFilePath)) - { - var json = File.ReadAllText(_cacheFilePath); - cache = System.Text.Json.JsonSerializer.Deserialize>(json) ?? new(); - } - - cache[environment] = new - { - token, - expiry = expiry.ToUniversalTime().ToString("o") - }; - - var updated = System.Text.Json.JsonSerializer.Serialize(cache, new System.Text.Json.JsonSerializerOptions { WriteIndented = true }); - File.WriteAllText(_cacheFilePath, updated); - - // Set file permissions to user-only on Unix systems - if (!OperatingSystem.IsWindows()) - { - try - { - var fileInfo = new FileInfo(_cacheFilePath); - fileInfo.UnixFileMode = UnixFileMode.UserRead | UnixFileMode.UserWrite; - _logger.LogDebug("Set secure permissions (0600) on token cache file"); - } - catch (Exception permEx) - { - _logger.LogWarning(permEx, "Failed to set Unix file permissions on token cache"); - } - } - - _logger.LogDebug("Token cached for environment: {Environment} at {Path}", environment, _cacheFilePath); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to cache token"); - } - } - private class MosEnvironmentConfig { public required string ClientId { get; init; } diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs index 913fee89..1beedb23 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs @@ -3,6 +3,7 @@ using Azure.Core; using Microsoft.Agents.A365.DevTools.Cli.Constants; +using Microsoft.Agents.A365.DevTools.Cli.Helpers; using Microsoft.Extensions.Logging; using Microsoft.Identity.Client; using Microsoft.Identity.Client.Broker; @@ -79,12 +80,15 @@ public sealed class MsalBrowserCredential : TokenCredential /// The redirect URI for authentication callbacks. /// Optional logger for diagnostic output. /// Whether to use WAM on Windows. Default is true. + /// Optional authority URL. When provided, overrides the default AzurePublic authority. + /// Use this for government clouds (e.g., "https://login.microsoftonline.us/{tenantId}"). public MsalBrowserCredential( string clientId, string tenantId, string? redirectUri = null, ILogger? logger = null, - bool useWam = true) + bool useWam = true, + string? authority = null) { if (string.IsNullOrWhiteSpace(clientId)) { @@ -118,9 +122,13 @@ public MsalBrowserCredential( } } - var builder = PublicClientApplicationBuilder - .Create(clientId) - .WithAuthority(AzureCloudInstance.AzurePublic, tenantId); + var builder = string.IsNullOrWhiteSpace(authority) + ? PublicClientApplicationBuilder + .Create(clientId) + .WithAuthority(AzureCloudInstance.AzurePublic, tenantId) + : PublicClientApplicationBuilder + .Create(clientId) + .WithAuthority(authority); if (_useWam) { @@ -414,18 +422,7 @@ private async Task AcquireTokenWithDeviceCodeFallbackAsync( try { var deviceCodeResult = await _publicClientApp - .AcquireTokenWithDeviceCode(scopes, deviceCode => - { - _logger?.LogInformation(""); - _logger?.LogInformation("=========================================================================="); - _logger?.LogInformation("To sign in, use a web browser to open the page:"); - _logger?.LogInformation(" {VerificationUrl}", deviceCode.VerificationUrl); - _logger?.LogInformation(""); - _logger?.LogInformation("And enter the code: {UserCode}", deviceCode.UserCode); - _logger?.LogInformation("=========================================================================="); - _logger?.LogInformation(""); - return Task.CompletedTask; - }) + .AcquireTokenWithDeviceCode(scopes, MsalHelper.CreateDeviceCodeCallback(_logger)) .ExecuteAsync(cancellationToken); _logger?.LogDebug("Successfully acquired token via device code authentication."); 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 048f45f2..5bfa5b45 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 @@ -27,7 +27,6 @@ public class BlueprintSubcommandTests private readonly IConfigService _mockConfigService; private readonly CommandExecutor _mockExecutor; private readonly IAzureValidator _mockAzureValidator; - private readonly AzureWebAppCreator _mockWebAppCreator; private readonly PlatformDetector _mockPlatformDetector; private readonly IBotConfigurator _mockBotConfigurator; private readonly GraphApiService _mockGraphApiService; @@ -43,7 +42,6 @@ public BlueprintSubcommandTests() var mockExecutorLogger = Substitute.For>(); _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); _mockAzureValidator = Substitute.For(); - _mockWebAppCreator = Substitute.ForPartsOf(Substitute.For>()); var mockPlatformDetectorLogger = Substitute.For>(); _mockPlatformDetector = Substitute.ForPartsOf(mockPlatformDetectorLogger); _mockBotConfigurator = Substitute.For(); @@ -63,7 +61,6 @@ public void CreateCommand_ShouldHaveCorrectName() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -81,7 +78,6 @@ public void CreateCommand_ShouldHaveDescription() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -100,7 +96,6 @@ public void CreateCommand_ShouldHaveConfigOption() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -121,7 +116,6 @@ public void CreateCommand_ShouldHaveVerboseOption() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -142,7 +136,6 @@ public void CreateCommand_ShouldHaveDryRunOption() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -162,7 +155,6 @@ public void CreateCommand_ShouldHaveSkipRequirementsOption() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -191,7 +183,6 @@ public async Task DryRun_ShouldLoadConfigAndNotExecute() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -226,7 +217,6 @@ public async Task DryRun_ShouldDisplayBlueprintInformation() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -338,7 +328,6 @@ public void CommandDescription_ShouldMentionRequiredPermissions() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -366,7 +355,6 @@ public async Task DryRun_WithCustomConfigPath_ShouldLoadCorrectFile() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -402,7 +390,6 @@ public async Task DryRun_ShouldNotCreateServicePrincipal() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -430,7 +417,6 @@ public void CreateCommand_ShouldHandleAllOptions() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -456,7 +442,6 @@ public async Task DryRun_WithMissingConfig_ShouldHandleGracefully() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -478,7 +463,6 @@ public void CreateCommand_DefaultConfigPath_ShouldBeA365ConfigJson() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -544,7 +528,6 @@ public void CommandDescription_ShouldBeInformativeAndActionable() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -572,7 +555,6 @@ public async Task DryRun_WithVerboseFlag_ShouldSucceed() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -606,7 +588,6 @@ public async Task DryRun_ShouldShowWhatWouldBeDone() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -638,7 +619,6 @@ public void CreateCommand_ShouldBeUsableInCommandPipeline() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1328,7 +1308,6 @@ public void CreateCommand_ShouldHaveUpdateEndpointOption() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1662,7 +1641,6 @@ public async Task SetHandler_WithClientAppId_ShouldConfigureGraphApiService() _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1697,7 +1675,6 @@ public async Task SetHandler_WithoutClientAppId_ShouldNotConfigureGraphApiServic _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); @@ -1732,7 +1709,6 @@ public async Task SetHandler_WithWhitespaceClientAppId_ShouldNotConfigureGraphAp _mockConfigService, _mockExecutor, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockBotConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService); 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 ee4fcd94..a5c86135 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 @@ -25,7 +25,6 @@ public class SetupCommandTests private readonly DeploymentService _mockDeploymentService; private readonly IBotConfigurator _mockBotConfigurator; private readonly IAzureValidator _mockAzureValidator; - private readonly AzureWebAppCreator _mockWebAppCreator; private readonly PlatformDetector _mockPlatformDetector; private readonly GraphApiService _mockGraphApiService; private readonly AgentBlueprintService _mockBlueprintService; @@ -54,7 +53,6 @@ public SetupCommandTests() mockPythonLogger); _mockBotConfigurator = Substitute.For(); _mockAzureValidator = Substitute.For(); - _mockWebAppCreator = Substitute.ForPartsOf(Substitute.For>()); _mockGraphApiService = Substitute.For(); _mockBlueprintService = Substitute.ForPartsOf(Substitute.For>(), _mockGraphApiService); _mockClientAppValidator = Substitute.For(); @@ -86,8 +84,7 @@ public async Task SetupAllCommand_DryRun_ValidConfig_OnlyValidatesConfig() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockAzureValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -133,8 +130,7 @@ public async Task SetupAllCommand_SkipInfrastructure_SkipsInfrastructureStep() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockAzureValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -161,8 +157,7 @@ public void SetupCommand_HasRequiredSubcommands() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockAzureValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -186,8 +181,7 @@ public void SetupCommand_PermissionsSubcommand_HasMcpAndBotSubcommands() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockAzureValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -214,8 +208,7 @@ public void SetupCommand_ErrorMessages_ShouldBeInformativeAndActionable() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockAzureValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -260,8 +253,7 @@ public async Task InfrastructureSubcommand_DryRun_CompletesSuccessfully() _mockExecutor, _mockDeploymentService, _mockBotConfigurator, - _mockAzureValidator, - _mockWebAppCreator, + _mockAzureValidator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -304,7 +296,6 @@ public async Task BlueprintSubcommand_DryRun_CompletesSuccessfully() _mockDeploymentService, _mockBotConfigurator, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, _mockBlueprintLookupService, _mockFederatedCredentialService, _mockClientAppValidator); @@ -346,7 +337,6 @@ public async Task RequirementsSubcommand_ValidConfig_CompletesSuccessfully() _mockDeploymentService, _mockBotConfigurator, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, @@ -390,7 +380,6 @@ public async Task RequirementsSubcommand_WithCategoryFilter_RunsFilteredChecks() _mockDeploymentService, _mockBotConfigurator, _mockAzureValidator, - _mockWebAppCreator, _mockPlatformDetector, _mockGraphApiService, _mockBlueprintService, diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/MsalHelperTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/MsalHelperTests.cs new file mode 100644 index 00000000..520d15f9 --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/MsalHelperTests.cs @@ -0,0 +1,172 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Microsoft.Agents.A365.DevTools.Cli.Helpers; +using Microsoft.Extensions.Logging; +using Microsoft.Identity.Client; +using NSubstitute; +using System.Reflection; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Helpers; + +public class MsalHelperTests +{ + private const string TestVerificationUrl = "https://microsoft.com/devicelogin"; + private const string TestUserCode = "ABCD1234"; + + /// + /// DeviceCodeResult has an internal constructor in MSAL 4.x. + /// Create one via reflection so we can invoke the callback in tests. + /// + private static DeviceCodeResult CreateDeviceCodeResult(string verificationUrl = TestVerificationUrl, string userCode = TestUserCode) + { + var ctor = typeof(DeviceCodeResult) + .GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance) + .FirstOrDefault() + ?? throw new InvalidOperationException("Cannot find DeviceCodeResult internal constructor."); + + // MSAL 4.x internal constructor parameter order: + // (string userCode, string deviceCode, string verificationUrl, + // DateTimeOffset expiresOn, long interval, string message, + // string clientId, ISet scopes) + return (DeviceCodeResult)ctor.Invoke(new object[] + { + userCode, + "device-code-value", + verificationUrl, + DateTimeOffset.UtcNow.AddMinutes(15), + 5L, + $"To sign in, use {verificationUrl} and enter {userCode}", + "test-client-id", + new HashSet { "test.scope" } + }); + } + + #region Delegate Creation Tests + + [Fact] + public void CreateDeviceCodeCallback_WithLogger_ReturnsNonNullDelegate() + { + var logger = Substitute.For(); + var callback = MsalHelper.CreateDeviceCodeCallback(logger); + Assert.NotNull(callback); + } + + [Fact] + public void CreateDeviceCodeCallback_WithNullLogger_ReturnsNonNullDelegate() + { + var callback = MsalHelper.CreateDeviceCodeCallback(null); + Assert.NotNull(callback); + } + + #endregion + + #region Logger Branch Tests + + [Fact] + public async Task CreateDeviceCodeCallback_WithLogger_CallsLogInformationWithVerificationUrl() + { + var logger = Substitute.For(); + logger.IsEnabled(Arg.Any()).Returns(true); + + var callback = MsalHelper.CreateDeviceCodeCallback(logger); + var result = CreateDeviceCodeResult(); + await callback(result); + + // Verify LogInformation was called at least once (for the verification URL line and user code line) + logger.Received().Log( + Microsoft.Extensions.Logging.LogLevel.Information, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public async Task CreateDeviceCodeCallback_WithLogger_CompletesSuccessfully() + { + var logger = Substitute.For(); + logger.IsEnabled(Arg.Any()).Returns(true); + + var callback = MsalHelper.CreateDeviceCodeCallback(logger); + var result = CreateDeviceCodeResult(); + + // Should not throw + var task = callback(result); + await task; + + Assert.True(task.IsCompletedSuccessfully); + } + + #endregion + + #region Console.Error Branch Tests + + [Fact] + public async Task CreateDeviceCodeCallback_WithNullLogger_WritesVerificationUrlToConsoleError() + { + var callback = MsalHelper.CreateDeviceCodeCallback(null); + var result = CreateDeviceCodeResult(TestVerificationUrl, TestUserCode); + + using var sw = new StringWriter(); + var original = Console.Error; + Console.SetError(sw); + try + { + await callback(result); + } + finally + { + Console.SetError(original); + } + + var output = sw.ToString(); + Assert.Contains(TestVerificationUrl, output); + } + + [Fact] + public async Task CreateDeviceCodeCallback_WithNullLogger_WritesUserCodeToConsoleError() + { + var callback = MsalHelper.CreateDeviceCodeCallback(null); + var result = CreateDeviceCodeResult(TestVerificationUrl, TestUserCode); + + using var sw = new StringWriter(); + var original = Console.Error; + Console.SetError(sw); + try + { + await callback(result); + } + finally + { + Console.SetError(original); + } + + var output = sw.ToString(); + Assert.Contains(TestUserCode, output); + } + + [Fact] + public async Task CreateDeviceCodeCallback_WithNullLogger_CompletesSuccessfully() + { + var callback = MsalHelper.CreateDeviceCodeCallback(null); + var result = CreateDeviceCodeResult(); + + using var sw = new StringWriter(); + var original = Console.Error; + Console.SetError(sw); + try + { + var task = callback(result); + await task; + Assert.True(task.IsCompletedSuccessfully); + } + finally + { + Console.SetError(original); + } + } + + #endregion +} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs deleted file mode 100644 index 22d0b970..00000000 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceCacheTests.cs +++ /dev/null @@ -1,213 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Globalization; -using FluentAssertions; -using Microsoft.Agents.A365.DevTools.Cli.Helpers; -using Microsoft.Agents.A365.DevTools.Cli.Models; -using Microsoft.Agents.A365.DevTools.Cli.Services; -using Microsoft.Extensions.Logging; -using NSubstitute; - -namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; - -/// -/// Tests must run sequentially because they modify a shared cache file -/// at ~/.a365/mos-token-cache.json. -/// -[CollectionDefinition("MosTokenCacheTests", DisableParallelization = true)] -public class MosTokenCacheTestCollection { } - -[Collection("MosTokenCacheTests")] -public class MosTokenServiceCacheTests : IDisposable -{ - private readonly ILogger _mockLogger; - private readonly IConfigService _mockConfigService; - private readonly MosTokenService _service; - private readonly string _cacheFilePath; - private readonly string? _originalCacheContent; - - public MosTokenServiceCacheTests() - { - _mockLogger = Substitute.For>(); - _mockConfigService = Substitute.For(); - _service = new MosTokenService(_mockLogger, _mockConfigService); - - var cacheDir = FileHelper.GetSecureCrossOsDirectory(); - _cacheFilePath = Path.Combine(cacheDir, "mos-token-cache.json"); - - // Backup any existing cache file to restore after tests - _originalCacheContent = File.Exists(_cacheFilePath) - ? File.ReadAllText(_cacheFilePath) - : null; - } - - public void Dispose() - { - // Restore original cache state - if (_originalCacheContent != null) - { - File.WriteAllText(_cacheFilePath, _originalCacheContent); - } - else if (File.Exists(_cacheFilePath)) - { - File.Delete(_cacheFilePath); - } - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenWithFutureUtcExpiry_ReturnsCachedToken() - { - // Arrange - cache a token that expires 1 hour from now (UTC) - var futureUtc = DateTime.UtcNow.AddHours(1); - WriteCacheFile("prod", "cached-valid-token", futureUtc); - - // Act - var result = await _service.AcquireTokenAsync("prod"); - - // Assert - cached token returned without loading config - result.Should().Be("cached-valid-token"); - await _mockConfigService.DidNotReceive().LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenWithPastUtcExpiry_DoesNotReturnCachedToken() - { - // Arrange - cache a token that expired 1 hour ago (UTC) - var pastUtc = DateTime.UtcNow.AddHours(-1); - WriteCacheFile("prod", "expired-token", pastUtc); - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - expired token not returned, config was loaded (cache miss) - result.Should().NotBe("expired-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenUtcZSuffix_ParsedAsUtcNotLocalTime() - { - // Regression test for #277. Without DateTimeStyles.AdjustToUniversal, - // DateTime.TryParse converts the "Z" suffix to local time. On IST (+5:30): - // Stored: "...12:00:00Z" Parsed: DateTime(17:30, Kind=Local) - // UtcNow(14:00) < 17:28 -> TRUE -> stale token returned (bug) - // Only catches the regression on UTC+ machines; see the Kind test below - // for the CI-reliable counterpart. - var expiredUtc = DateTime.UtcNow.AddHours(-3); - WriteCacheFile("prod", "stale-tz-token", expiredUtc); - SetupConfigForCacheMiss(); - - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - result.Should().NotBe("stale-tz-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public void TryParseUtcTimestamp_WithAdjustToUniversal_ParsedAsUtcKindNotLocalTime() - { - // CI-reliable regression test for #277. On a UTC machine the buggy code - // produces Kind=Local with the same tick value as Kind=Utc, so comparison - // passes anyway — the service-level test above misses it. Checking Kind - // directly catches the regression on every machine including UTC CI runners. - const string utcZTimestamp = "2026-01-01T12:00:00.0000000Z"; - - var parsed = DateTime.TryParse( - utcZTimestamp, - CultureInfo.InvariantCulture, - DateTimeStyles.AdjustToUniversal, - out var result); - - parsed.Should().BeTrue(); - result.Kind.Should().Be(DateTimeKind.Utc); - result.Hour.Should().Be(12); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenWithin2MinuteBuffer_DoesNotReturnCachedToken() - { - // Arrange - token expiring in 90 seconds (within 2-minute safety buffer) - var almostExpiredUtc = DateTime.UtcNow.AddSeconds(90); - WriteCacheFile("prod", "almost-expired-token", almostExpiredUtc); - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - token within buffer not returned - result.Should().NotBe("almost-expired-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_CachedTokenForDifferentEnvironment_DoesNotReturnToken() - { - // Arrange - cache a valid token for "sdf" but request "prod" - var futureUtc = DateTime.UtcNow.AddHours(1); - WriteCacheFile("sdf", "sdf-only-token", futureUtc); - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - token for wrong environment not returned - result.Should().NotBe("sdf-only-token"); - await _mockConfigService.Received(1).LoadAsync(); - } - - [Fact] - public async Task AcquireTokenAsync_NoCacheFile_LoadsConfig() - { - // Arrange - ensure no cache file exists - if (File.Exists(_cacheFilePath)) - { - File.Delete(_cacheFilePath); - } - - SetupConfigForCacheMiss(); - - // Act - var result = await _service.AcquireTokenAsync("prod", cancellationToken: CancelledToken()); - - // Assert - no cache, falls through to config loading - await _mockConfigService.Received(1).LoadAsync(); - } - - /// - /// Write a MOS token cache JSON file with an ISO 8601 UTC expiry timestamp, - /// matching the format produced by MosTokenService.CacheToken(). - /// - private void WriteCacheFile(string environment, string token, DateTime expiryUtc) - { - var cacheDir = Path.GetDirectoryName(_cacheFilePath)!; - Directory.CreateDirectory(cacheDir); - - // Use the same format as CacheToken: expiry.ToUniversalTime().ToString("o") - // This produces "2026-02-18T17:00:00.0000000Z" with the "Z" suffix - var isoExpiry = expiryUtc.ToUniversalTime().ToString("o"); - var json = $$""" - { - "{{environment}}": { - "token": "{{token}}", - "expiry": "{{isoExpiry}}" - } - } - """; - File.WriteAllText(_cacheFilePath, json); - } - - private void SetupConfigForCacheMiss() - { - var config = new Agent365Config { TenantId = "test-tenant", ClientAppId = "test-client" }; - _mockConfigService.LoadAsync().Returns(Task.FromResult(config)); - } - - private static CancellationToken CancelledToken() - { - var cts = new CancellationTokenSource(); - cts.Cancel(); - return cts.Token; - } -} diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs index c4bf957c..36126f03 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MosTokenServiceTests.cs @@ -13,7 +13,6 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; /// /// Unit tests for MosTokenService. /// -[Collection("MosTokenCacheTests")] public class MosTokenServiceTests { private readonly ILogger _mockLogger; diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MsalBrowserCredentialTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MsalBrowserCredentialTests.cs index d0e9b3ae..047dfbd6 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MsalBrowserCredentialTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/MsalBrowserCredentialTests.cs @@ -68,10 +68,36 @@ public void Constructor_WithNullOrEmptyClientId_ShouldThrowArgumentNullException public void Constructor_WithNullOrEmptyTenantId_ShouldThrowArgumentNullException(string? tenantId) { // Act & Assert - Assert.Throws(() => + Assert.Throws(() => new MsalBrowserCredential(ValidClientId, tenantId!, ValidRedirectUri)); } + [Theory] + [InlineData("https://login.microsoftonline.us/tenant-id")] + [InlineData("https://login.microsoftonline.com/tenant-id")] + public void Constructor_WithCustomAuthority_ShouldSucceed(string authority) + { + // Custom authority is used for government clouds (gcch/dod) where + // AzureCloudInstance.AzurePublic is not appropriate. + var credential = new MsalBrowserCredential( + ValidClientId, ValidTenantId, redirectUri: null, authority: authority); + + Assert.NotNull(credential); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Constructor_WithNullOrWhitespaceAuthority_UsesDefaultAzurePublicAuthority(string? authority) + { + // Null/whitespace authority falls back to AzureCloudInstance.AzurePublic + tenantId. + var credential = new MsalBrowserCredential( + ValidClientId, ValidTenantId, redirectUri: null, authority: authority); + + Assert.NotNull(credential); + } + #endregion #region WAM Configuration Tests