-
Notifications
You must be signed in to change notification settings - Fork 7
Enforce device code authentication across entire CLI workflow #156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
## Overview Removed all browser-based authentication (InteractiveBrowserCredential, AcquireTokenInteractive) and enforced device code flow for Graph API and Azure CLI operations across the entire a365 CLI. ## Changes Made ### Authentication Services - **MicrosoftGraphTokenProvider**: Always use `-UseDeviceCode` flag in PowerShell commands - **GraphApiService**: Add fallback to Azure CLI when token provider is null with scopes - **AuthenticationService**: Remove InteractiveBrowserCredential, only use DeviceCodeCredential - **MosTokenService**: Replace AcquireTokenInteractive with AcquireTokenWithDeviceCode - **AzureWebAppCreator**: Set ExcludeInteractiveBrowserCredential = true in DefaultAzureCredential - **InteractiveGraphAuthService**: Remove browser auth fallback, use device code only ### Commands & Helpers - **InfrastructureSubcommand**: Add --use-device-code flag to all az login commands - **BlueprintSubcommand**: Replace GraphServiceClient with REST-based GraphApiService calls - **DelegatedConsentService**: Simplify by removing browser-based HttpClient auth flows (~460 to ~80 lines) - **AdminConsentHelper**: Replace az rest polling with GraphApiService calls using delegated scopes ### Constants - **AuthenticationConstants**: Add PermissionGrantAuthScopes and AgentBlueprintAuthScopes ### All Graph API Methods - Add optional `scopes` parameter to GraphGetAsync, GraphPostAsync, GraphPatchAsync, GraphDeleteAsync - Pass appropriate auth scopes throughout AgentBlueprintService and FederatedCredentialService ## Test Updates - Fixed 14 existing tests to work with new scopes parameter - Added 4 new tests for device code enforcement and Azure CLI fallback behavior - All 774 tests passing (790 total including skipped) ## Impact - Zero browser popups throughout entire CLI - Consistent device code experience for all commands - Works in headless environments (SSH, containers, CI/CD) - Token caching minimizes repeated device code prompts (1-2 prompts per workflow) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request enforces device code authentication across the entire a365 CLI workflow by removing all browser-based authentication methods (InteractiveBrowserCredential, AcquireTokenInteractive) and replacing them with device code flow. This change ensures the CLI works reliably in headless environments like SSH sessions, containers, and CI/CD pipelines.
Changes:
- Removed browser-based authentication methods and enforced device code flow across all authentication services
- Added scopes parameter to Graph API methods for delegated permission support
- Refactored DelegatedConsentService from ~460 to ~80 lines by replacing custom HTTP logic with GraphApiService calls
- Updated 14 existing tests and added 4 new tests to verify device code enforcement and Azure CLI fallback behavior
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| MicrosoftGraphTokenProvider.cs | Always use -UseDeviceCode flag in PowerShell commands regardless of parameter value |
| GraphApiService.cs | Add scopes parameter to Graph methods; fallback to Azure CLI when token provider is null |
| AuthenticationService.cs | Replace InteractiveBrowserCredential with DeviceCodeCredential |
| MosTokenService.cs | Replace AcquireTokenInteractive with AcquireTokenWithDeviceCode |
| InteractiveGraphAuthService.cs | Remove browser auth fallback, use device code only |
| AzureWebAppCreator.cs | Exclude InteractiveBrowserCredential from DefaultAzureCredential |
| InfrastructureSubcommand.cs | Add --use-device-code flag to all az login commands |
| BlueprintSubcommand.cs | Replace GraphServiceClient with REST-based GraphApiService calls |
| DelegatedConsentService.cs | Simplify by removing browser-based HttpClient auth flows |
| AdminConsentHelper.cs | Replace az rest polling with GraphApiService calls using delegated scopes |
| AuthenticationConstants.cs | Add PermissionGrantAuthScopes and AgentBlueprintAuthScopes constants |
| AgentBlueprintService.cs | Pass appropriate auth scopes throughout the service |
| FederatedCredentialService.cs | Add scopes parameter to Graph API method calls |
| PublishHelpers.cs | Add scopes parameter to Graph API method calls |
| SetupHelpers.cs | Rename RequiredPermissionGrantScopes to PermissionGrantAuthScopes |
| DeployCommand.cs | Update method call with renamed parameter |
| Test files | Update tests to work with new scopes parameter and verify device code enforcement |
| // ALWAYS use -UseDeviceCode for CLI-friendly authentication (no browser popup/download) | ||
| // This enforces device code flow for the entire CLI workflow | ||
| var authMethod = "-UseDeviceCode"; |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable authMethod is always set to "-UseDeviceCode" regardless of the useDeviceCode parameter value. This makes the parameter misleading. Since the entire PR is about enforcing device code authentication, consider removing the useDeviceCode parameter from the public API method signature, or add a clear comment explaining that the parameter is deprecated but kept for backward compatibility.
| // Fall back to Azure CLI token (for tests or when token provider is not configured) | ||
| if (scopes != null && _tokenProvider == null) | ||
| { | ||
| _logger.LogWarning("Token provider is not configured, falling back to Azure CLI for scopes: {Scopes}", string.Join(", ", scopes)); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning message on line 196 says "falling back to Azure CLI for scopes" which could be misleading. When scopes are provided but the token provider is null, the code does fall back to Azure CLI, but Azure CLI doesn't actually support scopes in the same way. The Azure CLI will get a token with default Graph API permissions, which may not include the requested scopes. Consider clarifying this in the warning message, e.g., "Token provider is not configured, falling back to Azure CLI (note: specific scopes may not be honored)".
| _logger.LogWarning("Token provider is not configured, falling back to Azure CLI for scopes: {Scopes}", string.Join(", ", scopes)); | |
| _logger.LogWarning("Token provider is not configured, falling back to Azure CLI (note: specific scopes may not be honored). Requested scopes: {Scopes}", string.Join(", ", scopes)); |
| _logger.LogInformation("Attempting to authenticate to Microsoft Graph using device code flow..."); | ||
| _logger.LogInformation("This requires permissions defined in AuthenticationConstants.RequiredClientAppPermissions for Agent Blueprint operations."); | ||
| _logger.LogInformation(""); | ||
| _logger.LogInformation("IMPORTANT: A browser window will open for authentication."); | ||
| _logger.LogInformation("Please sign in with an account that has Global Administrator or similar privileges."); | ||
| _logger.LogInformation("IMPORTANT: Please follow the device code instructions."); | ||
| _logger.LogInformation("Sign in with an account that has Global Administrator or similar privileges."); | ||
| _logger.LogInformation(""); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation now exclusively uses device code flow, but there's no XML documentation comment explaining this behavior change. Consider adding a documentation comment above the method (or updating existing comments within the changed region) to clarify that this method now uses device code authentication exclusively and no longer uses interactive browser authentication.
| const string identityId = "identity-123"; | ||
|
|
||
| // Queue successful response for DELETE operation | ||
| handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
| handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty<object>() })) | ||
| }); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
| handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) | ||
| { | ||
| Content = new StringContent(JsonSerializer.Serialize(new { value = Array.Empty<object>() })) | ||
| }); |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'HttpResponseMessage' is created but not disposed.
| else | ||
| { | ||
| // Fall back to Azure CLI token (for tests or when token provider is not configured) | ||
| if (scopes != null && _tokenProvider == null) |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition is always true because of ... != ....
| if (scopes != null && _tokenProvider == null) | |
| if (scopes != null) |
Overview
Removed all browser-based authentication (InteractiveBrowserCredential, AcquireTokenInteractive) and enforced device code flow for Graph API and Azure CLI operations across the entire a365 CLI.
Changes Made
Authentication Services
-UseDeviceCodeflag in PowerShell commandsCommands & Helpers
Constants
All Graph API Methods
scopesparameter to GraphGetAsync, GraphPostAsync, GraphPatchAsync, GraphDeleteAsyncTest Updates
Impact