diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index efaf3aa..10b3f0c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -36,6 +36,11 @@ - Focus on quality over quantity of tests - Add regression tests for bug fixes - Tests should verify CLI reliability +- **Dispose IDisposable objects properly**: + - `HttpResponseMessage` objects created in tests must be disposed + - Even in mock/test handlers, follow proper disposal patterns + - Consider using `using` statements or ensure test handlers dispose responses + - This applies to all `IDisposable` test objects to avoid analyzer warnings ### Output and Logging - No emojis or special characters in logs, output, or comments diff --git a/docs/guides/custom-client-app-registration.md b/docs/guides/custom-client-app-registration.md deleted file mode 100644 index 581644b..0000000 --- a/docs/guides/custom-client-app-registration.md +++ /dev/null @@ -1,336 +0,0 @@ -# Custom Client App Registration Guide - -## Overview - -The Agent365 CLI requires a custom client app registration in your Entra ID tenant to authenticate and manage Agent Identity Blueprints. - -## Quick Setup - -### Prerequisites - -**To register the app** (Steps 1-2): -- Any developer with basic Entra ID access can register an application - -**To add permissions and grant consent** (Steps 3-4): -- **One of these admin roles** is required: - - **Application Administrator** (recommended - can manage app registrations and grant consent) - - **Cloud Application Administrator** (can manage app registrations and grant consent) - - **Global Administrator** (has all permissions, but not required) - -> **Don't have admin access?** You can complete Steps 1-2 yourself, then ask your tenant administrator to complete Steps 3-4. Provide them: -> - Your **Application (client) ID** from Step 2 -> - A link to this guide: [Custom Client App Registration](#3-configure-api-permissions) - -### 1. Register Application - -Follow [Microsoft's quickstart guide](https://learn.microsoft.com/en-us/entra/identity-platform/quickstart-register-app) to create an app registration: - -1. Go to **Azure Portal** → **Entra ID** → **App registrations** → **New registration** -2. Enter: - - **Name**: `Agent365 CLI` (or your preferred name) - - **Supported account types**: **Single tenant** (Accounts in this organizational directory only) - - **Redirect URI**: Select **Public client/native (mobile & desktop)** → Enter `http://localhost:8400/` -3. Click **Register** - -> **Note**: The CLI uses port 8400 for the OAuth callback. Ensure this port is not blocked by your firewall. - -### 2. Copy Application (client) ID - -From the app's **Overview** page, copy the **Application (client) ID** (GUID format). You'll enter this during `a365 config init`. - -> **Tip**: Don't confuse this with **Object ID** - you need the **Application (client) ID**. - -### 3. Configure API Permissions - -> **⚠️ Admin privileges required for this step and Step 4.** If you're a developer without admin access, send your **Application (client) ID** from Step 2 to your tenant administrator and have them complete Steps 3-4. - -**Choose Your Method**: The two `AgentIdentityBlueprint.*` permissions are beta APIs and may not be visible in the Azure Portal UI. You can either: -- **Option A**: Use Azure Portal for all permissions (if beta permissions are visible) -- **Option B**: Use Microsoft Graph API to add all permissions (recommended if beta permissions not visible) - -#### Option A: Azure Portal (Standard Method) - -**If beta permissions are visible in your tenant**: - -1. In your app registration, go to **API permissions** -2. Click **Add a permission** → **Microsoft Graph** → **Delegated permissions** -3. Add these 5 permissions one by one: - -> **Important**: You MUST use **Delegated permissions** (NOT Application permissions). The CLI authenticates interactively - you sign in, and it acts on your behalf. See [Troubleshooting](#wrong-permission-type-delegated-vs-application) if you accidentally add Application permissions. - -| Permission | Purpose | -|-----------|---------| -| `AgentIdentityBlueprint.ReadWrite.All` | Manage Agent Blueprint configurations (beta API) | -| `AgentIdentityBlueprint.UpdateAuthProperties.All` | Update Agent Blueprint inheritable permissions (beta API) | -| `Application.ReadWrite.All` | Create and manage applications and Agent Blueprints | -| `DelegatedPermissionGrant.ReadWrite.All` | Grant permissions for agent blueprints | -| `Directory.Read.All` | Read directory data for validation | - - **For each permission above**: - - In the search box, type the permission name (e.g., `AgentIdentityBlueprint.ReadWrite.All`) - - Check the checkbox next to the permission - - Click **Add permissions** button - - Repeat for all 5 permissions - -4. Click **Grant admin consent for [Your Tenant]** - - **Why is this required?** Agent Identity Blueprints are tenant-wide resources that multiple users and applications can reference. Without tenant-wide consent, the CLI will fail during authentication. - - **What if it fails?** You need Application Administrator, Cloud Application Administrator, or Global Administrator role. Ask your tenant admin for help. -5. Verify all permissions show green checkmarks under "Status" - -If the beta permissions (`AgentIdentityBlueprint.*`) are **not visible**, proceed to **Option B** below. - -#### Option B: Microsoft Graph API (For Beta Permissions) - -**Use this method if `AgentIdentityBlueprint.*` permissions are not visible in Azure Portal**. - -> **⚠️ WARNING**: If you use this API method, **do NOT use Azure Portal's "Grant admin consent" button** afterward. The API method grants admin consent automatically, and using the Portal button will **delete your beta permissions**. See [troubleshooting section](#beta-permissions-disappear-after-portal-admin-consent) for details. - -1. **Open Graph Explorer**: Go to https://developer.microsoft.com/graph/graph-explorer -2. **Sign in** with your admin account (Application Administrator or Cloud Application Administrator) -3. **Grant admin consent using Graph API**: - - **Step 1**: Get your service principal ID and Graph resource ID: - - > **What's a service principal?** It's your app's identity in your tenant, required before granting permissions via API. - - Set method to **GET** and use this URL (replace YOUR_CLIENT_APP_ID with your actual Application client ID from Step 2): - ``` - https://graph.microsoft.com/v1.0/servicePrincipals?$filter=appId eq 'YOUR_CLIENT_APP_ID'&$select=id - ``` - - Click **Run query**. If the query fails with a permissions error, click the **Modify permissions** tab, consent to the required permissions, then click **Run query** again. - - **If the query returns empty results** (`"value": []`), create the service principal: - - Set method to **POST** and use this URL: - ``` - https://graph.microsoft.com/v1.0/servicePrincipals - ``` - - **Request Body** (replace YOUR_CLIENT_APP_ID with your actual Application client ID): - ```json - { - "appId": "YOUR_CLIENT_APP_ID" - } - ``` - - Click **Run query**. You should get a `201 Created` response. - - **Copy the `id` value from whichever query succeeded** (GET or POST) - this is your `SP_OBJECT_ID`. - - Set method to **GET** and use this URL: - ``` - https://graph.microsoft.com/v1.0/servicePrincipals?$filter=appId eq '00000003-0000-0000-c000-000000000000'&$select=id - ``` - - Click **Run query**. If the query fails with a permissions error, click the **Modify permissions** tab, consent to the required permissions, then click **Run query** again. Copy the `id` value (this is your `GRAPH_RESOURCE_ID`) - - **Step 2**: Grant admin consent with all 5 permissions (including beta): - - This API call grants tenant-wide admin consent for all 5 permissions, including the 2 beta permissions that aren't visible in Portal. - - Set method to **POST** and use this URL: - ``` - https://graph.microsoft.com/v1.0/oauth2PermissionGrants - ``` - - **Request Body**: - ```json - { - "clientId": "SP_OBJECT_ID_FROM_STEP1", - "consentType": "AllPrincipals", - "principalId": null, - "resourceId": "GRAPH_RESOURCE_ID_FROM_STEP1", - "scope": "Application.ReadWrite.All Directory.Read.All DelegatedPermissionGrant.ReadWrite.All AgentIdentityBlueprint.ReadWrite.All AgentIdentityBlueprint.UpdateAuthProperties.All" - } - ``` - - Click **Run query**. If the query fails with a permissions error (likely DelegatedPermissionGrant.ReadWrite.All), click the **Modify permissions** tab, consent to DelegatedPermissionGrant.ReadWrite.All, then click **Run query** again. - - **If you get `201 Created` response** - Success! The `scope` field in the response shows all 5 permission names. You're done. - - **If you get error `Request_MultipleObjectsWithSameKeyValue`** - A grant already exists (you may have added permissions in Portal earlier). Update it instead: - - Set method to **GET** and use this URL: - ``` - https://graph.microsoft.com/v1.0/oauth2PermissionGrants?$filter=clientId eq 'SP_OBJECT_ID_FROM_STEP1' - ``` - - Click **Run query**. Copy the `id` value from the response. - - Set method to **PATCH** and use this URL (replace YOUR_GRANT_ID with the ID you just copied): - ``` - https://graph.microsoft.com/v1.0/oauth2PermissionGrants/YOUR_GRANT_ID - ``` - - **Request Body**: - ```json - { - "scope": "Application.ReadWrite.All Directory.Read.All DelegatedPermissionGrant.ReadWrite.All AgentIdentityBlueprint.ReadWrite.All AgentIdentityBlueprint.UpdateAuthProperties.All" - } - ``` - - Click **Run query**. You should get a `200 OK` response with all 5 permissions in the `scope` field. - -> **⚠️ CRITICAL WARNING**: The `consentType: "AllPrincipals"` in the POST request above **already grants tenant-wide admin consent**. **DO NOT click "Grant admin consent" in Azure Portal** after using this API method - doing so will **delete your beta permissions** because the Portal UI cannot see beta permissions and will overwrite your API-granted consent with only the visible permissions. - -### 4. Use in Agent365 CLI - -Run the configuration wizard and enter your Application (client) ID when prompted: - -```powershell -a365 config init -``` - -The CLI automatically validates: -- App exists in your tenant -- Required permissions are configured -- Admin consent has been granted - -## Troubleshooting - -### Wrong Permission Type (Delegated vs Application) - -**Symptom**: CLI fails with authentication errors or permission denied errors. - -**Root cause**: You added **Application permissions** instead of **Delegated permissions**. - -| Permission Type | When to Use | How Agent365 Uses It | -|----------------|-------------|---------------------| -| **Delegated** ("Scope") | User signs in interactively | **Agent365 CLI uses this** - You sign in, CLI acts on your behalf | -| **Application** ("Role") | Service runs without user | **Don't use** - For background services/daemons only | - -**Why Delegated?** -- You sign in interactively (browser authentication) -- CLI performs actions **as you** (audit trails show your identity) -- More secure - limited by your actual permissions -- Ensures accountability and compliance - -**Solution**: -1. Go to Azure Portal → App registrations → Your app → API permissions -2. **Remove** any Application permissions (these show as "Admin" in the Type column) -3. **Add** the same permissions as **Delegated** permissions -4. Grant admin consent again - -**Common mistake**: Adding `Directory.Read.All` as **Application** instead of **Delegated**. - -### Beta Permissions Disappear After Portal Admin Consent - -**Symptom**: You used the API method (Option B) to add beta permissions, but they disappeared after clicking "Grant admin consent" in Azure Portal. - -**Root cause**: Azure Portal doesn't show beta permissions in the UI, so when you click "Grant admin consent" in Portal, it only grants the *visible* permissions and overwrites the API-granted consent. - -**Why this happens**: -1. You use the Graph API (Option B) to add all 5 permissions including beta permissions -2. The API call with `consentType: "AllPrincipals"` **already grants tenant-wide admin consent** -3. You go to Azure Portal and see only 3 permissions (the beta permissions are invisible) -4. You click "Grant admin consent" in Portal thinking you need to -5. Portal overwrites your API-granted consent with **only the 3 visible permissions** -6. Your 2 beta permissions are now deleted - -**Solution**: -- **Never use Portal admin consent after API method** - the API method already grants admin consent -- If you accidentally deleted beta permissions, re-run the Option B Step 2 to restore them - - You'll get a `Request_MultipleObjectsWithSameKeyValue` error - follow the PATCH instructions in Step 2 -- Check the `scope` field in the POST or PATCH response to verify all 5 permissions are listed - -### Validation Errors - -The CLI automatically validates your client app when running `a365 setup all` or `a365 config init`. - -Common issues: -- **App not found**: Verify you copied the **Application (client) ID** (not Object ID) -- **Missing permissions**: Add all five required permissions -- **Admin consent not granted**: - - If you used **Option A** (Portal only): Click "Grant admin consent" in Azure Portal - - If you used **Option B** (Graph API): Re-run the POST or PATCH request - do NOT use Portal's consent button -- **Wrong permission type**: Use Delegated permissions, not Application permissions - -For detailed troubleshooting, see [Microsoft's app registration documentation](https://learn.microsoft.com/en-us/entra/identity-platform/quickstart-register-app). - -## MOS API Permissions (Auto-Configured) - -The `a365 publish` command requires access to Microsoft Office Store (MOS) APIs to publish agent packages. These permissions are **automatically configured** the first time you run `a365 publish`. - -### What Gets Configured Automatically - -When you run `a365 publish` for the first time, the CLI will: -1. Create service principals for MOS resource applications in your tenant -2. Configure your custom client app to request MOS API permissions -3. Prompt you to grant admin consent if not already granted - -### MOS Resource Applications - -The following MOS resource applications need service principals created in your tenant: - -| Resource App ID | Purpose | -|----------------|---------| -| `6ec511af-06dc-4fe2-b493-63a37bc397b1` | TPS AppServices 3p App (Server) - Required for MOS publishing | -| `8578e004-a5c6-46e7-913e-12f58912df43` | Power Platform API - MOS token acquisition | -| `e8be65d6-d430-4289-a665-51bf2a194bda` | MOS Titles API - Access to titles.prod.mos.microsoft.com | - -### Redirect URI Requirement - -Ensure your custom client app registration includes the redirect URI `http://localhost:8400/` (already configured in Step 1 above). This URI is used for both Microsoft Graph and MOS token acquisition. - -### Admin Consent - -After the CLI automatically configures MOS permissions, you'll need to grant admin consent: - -1. The CLI will display a message with a direct link to the Azure Portal permissions page -2. Go to the provided URL or navigate to: **Azure Portal** → **App registrations** → **Your app** → **API permissions** -3. Click **Grant admin consent for [Your Tenant]** -4. Verify all permissions show green checkmarks under "Status" - -Alternatively, you can consent interactively when the authentication prompt appears during `a365 publish`. - -### Manual Configuration (If Needed) - -If automatic configuration fails due to insufficient privileges, you can manually create the service principals: - -```powershell -az ad sp create --id 6ec511af-06dc-4fe2-b493-63a37bc397b1 -az ad sp create --id 8578e004-a5c6-46e7-913e-12f58912df43 -az ad sp create --id e8be65d6-d430-4289-a665-51bf2a194bda -``` - -After creating service principals manually, run `a365 publish` again. - -### Troubleshooting MOS Permissions - -**Error: AADSTS650052 when running publish** -- **Cause**: Service principals don't exist or admin consent not granted -- **Solution**: Run `a365 publish` again (it will create service principals) and grant admin consent when prompted - -**Error: AADSTS650057 - Invalid resource** -- **Cause**: MOS Titles API resource app not added to your client app's API permissions -- **Solution**: Run `a365 publish` again (it will automatically add the missing resource) and grant admin consent when prompted - -**Error: Insufficient privileges to create service principal** -- **Cause**: You don't have Application Administrator, Cloud Application Administrator, or Global Administrator role -- **Solution**: Use the manual configuration commands above, or ask your tenant administrator to run `a365 publish` or execute the `az ad sp create` commands - -**Admin consent not showing up** -- **Cause**: OAuth2 permission grant hasn't propagated yet -- **Solution**: Wait a few minutes and run `a365 publish` again, or manually grant consent via Azure Portal - -## Security Best Practices - -**Do**: -- Use single-tenant registration -- Grant only the required delegated permissions (Microsoft Graph + MOS APIs) -- Audit permissions regularly -- Remove the app when no longer needed - -**Don't**: -- Grant Application permissions (use Delegated only) -- Share the Client ID publicly -- Grant additional unnecessary permissions -- Use the app for other purposes - -## Additional Resources - -- [Microsoft Graph Permissions Reference](https://learn.microsoft.com/en-us/graph/permissions-reference) -- [Entra ID App Registration](https://learn.microsoft.com/en-us/entra/identity-platform/quickstart-register-app) -- [Grant Admin Consent](https://learn.microsoft.com/en-us/entra/identity/enterprise-apps/grant-admin-consent) -- [Agent365 CLI Documentation](../README.md) 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 f99469c..b9da953 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs @@ -892,8 +892,8 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( setupConfig.AgentBlueprintObjectId = objectId; setupConfig.AgentBlueprintServicePrincipalObjectId = servicePrincipalId; setupConfig.AgentBlueprintId = appId; - - logger.LogDebug("Blueprint identifiers staged for persistence: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", + + logger.LogDebug("Blueprint identifiers staged for persistence: ObjectId={ObjectId}, SPObjectId={SPObjectId}, AppId={AppId}", objectId, servicePrincipalId, appId); // Complete configuration (FIC validation + admin consent) @@ -946,6 +946,31 @@ public static async Task EnsureDelegatedConsentWithRetriesAsync( bool alreadyExisted, CancellationToken ct) { + // ======================================================================== + // Application Owner Assignment + // ======================================================================== + + // Add current user as owner to the application (for both new and existing blueprints) + // This ensures the creator can set callback URLs and bot IDs via the Developer Portal + // Requires Application.ReadWrite.All or Directory.ReadWrite.All permissions + logger.LogInformation("Ensuring current user is owner of application..."); + var ownerScopes = new[] { GraphApiConstants.Scopes.ApplicationReadWriteAll }; + var ownerAdded = await graphApiService.AddApplicationOwnerAsync( + tenantId, + objectId, + userObjectId: null, + ct, + scopes: ownerScopes); + if (ownerAdded) + { + logger.LogInformation("Current user is an owner of the application"); + } + else + { + logger.LogWarning("Could not verify or add current user as application owner"); + logger.LogWarning("See detailed error above or refer to: https://learn.microsoft.com/en-us/graph/api/application-post-owners?view=graph-rest-beta"); + } + // ======================================================================== // Federated Identity Credential Validation/Creation // ======================================================================== diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Constants/GraphApiConstants.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/GraphApiConstants.cs new file mode 100644 index 0000000..7d49a22 --- /dev/null +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Constants/GraphApiConstants.cs @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace Microsoft.Agents.A365.DevTools.Cli.Constants; + +/// +/// Constants for Microsoft Graph API endpoints and resources +/// +public static class GraphApiConstants +{ + /// + /// Base URL for Microsoft Graph API + /// + public const string BaseUrl = "https://graph.microsoft.com"; + + /// + /// Resource identifier for Microsoft Graph API (used in Azure CLI token acquisition) + /// + public const string Resource = "https://graph.microsoft.com/"; + + /// + /// Endpoint versions + /// + public static class Versions + { + /// + /// Stable v1.0 endpoint for production workloads + /// + public const string V1 = "v1.0"; + + /// + /// Beta endpoint for preview features + /// + public const string Beta = "beta"; + } + + /// + /// Common Microsoft Graph permission scopes + /// + public static class Scopes + { + /// + /// Application.ReadWrite.All permission scope - required for managing application registrations + /// + public const string ApplicationReadWriteAll = "https://graph.microsoft.com/Application.ReadWrite.All"; + + /// + /// Directory.ReadWrite.All permission scope - required for directory-level operations + /// + public const string DirectoryReadWriteAll = "https://graph.microsoft.com/Directory.ReadWrite.All"; + } +} diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs index f336660..a4c844a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs @@ -6,9 +6,11 @@ using Microsoft.Agents.A365.DevTools.Cli.Services.Internal; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using System.Net; using System.Net.Http.Headers; using System.Text; using System.Text.Json; +using System.Text.Json.Nodes; namespace Microsoft.Agents.A365.DevTools.Cli.Services; @@ -413,13 +415,13 @@ public async Task CreateOrUpdateOauth2PermissionGrantAsync( /// Cancellation token /// True if user has required roles, false otherwise public virtual async Task<(bool hasPrivileges, List roles)> CheckServicePrincipalCreationPrivilegesAsync( - string tenantId, + string tenantId, CancellationToken ct = default) { try { _logger.LogDebug("Checking user's directory roles for service principal creation privileges"); - + var token = await GetGraphAccessTokenAsync(tenantId, ct); if (token == null) { @@ -427,7 +429,7 @@ public async Task CreateOrUpdateOauth2PermissionGrantAsync( return (false, new List()); } - using var request = new HttpRequestMessage(HttpMethod.Get, + using var request = new HttpRequestMessage(HttpMethod.Get, "https://graph.microsoft.com/v1.0/me/memberOf/microsoft.graph.directoryRole"); request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); @@ -454,22 +456,22 @@ public async Task CreateOrUpdateOauth2PermissionGrantAsync( _logger.LogDebug("User has {Count} directory roles", roles.Count); // Check for required roles - var requiredRoles = new[] - { - "Application Administrator", - "Cloud Application Administrator", - "Global Administrator" + var requiredRoles = new[] + { + "Application Administrator", + "Cloud Application Administrator", + "Global Administrator" }; var hasRequiredRole = roles.Any(r => requiredRoles.Contains(r, StringComparer.OrdinalIgnoreCase)); - + if (hasRequiredRole) { _logger.LogDebug("User has sufficient privileges for service principal creation"); } else { - _logger.LogDebug("User does not have required roles for service principal creation. Roles: {Roles}", + _logger.LogDebug("User does not have required roles for service principal creation. Roles: {Roles}", string.Join(", ", roles)); } @@ -481,4 +483,159 @@ public async Task CreateOrUpdateOauth2PermissionGrantAsync( return (false, new List()); } } + + /// + /// Ensures the current user is an owner of an application (idempotent operation). + /// First checks if the user is already an owner, and only adds if not present. + /// This ensures the creator has ownership permissions for setting callback URLs and bot IDs via the Developer Portal. + /// Requires Application.ReadWrite.All or Directory.ReadWrite.All permissions. + /// See: https://learn.microsoft.com/en-us/graph/api/application-post-owners?view=graph-rest-beta + /// + /// The tenant ID + /// The application object ID (not the client/app ID) + /// The user's object ID to add as owner. If null, uses the current authenticated user. + /// Cancellation token + /// OAuth2 scopes for elevated permissions (e.g., Application.ReadWrite.All, Directory.ReadWrite.All) + /// True if the user is an owner (either already was or was successfully added), false otherwise + public virtual async Task AddApplicationOwnerAsync( + string tenantId, + string applicationObjectId, + string? userObjectId = null, + CancellationToken ct = default, + IEnumerable? scopes = null) + { + try + { + // Get current user's object ID if not provided + if (string.IsNullOrWhiteSpace(userObjectId)) + { + if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) + { + _logger.LogWarning("Could not acquire Graph token to add application owner"); + return false; + } + + using var meRequest = new HttpRequestMessage(HttpMethod.Get, + "https://graph.microsoft.com/v1.0/me?$select=id"); + meRequest.Headers.Authorization = _httpClient.DefaultRequestHeaders.Authorization; + + using var meResponse = await _httpClient.SendAsync(meRequest, ct); + if (!meResponse.IsSuccessStatusCode) + { + _logger.LogWarning("Could not retrieve current user's ID: {Status}", meResponse.StatusCode); + return false; + } + + var meJson = await meResponse.Content.ReadAsStringAsync(ct); + using var meDoc = JsonDocument.Parse(meJson); + + if (!meDoc.RootElement.TryGetProperty("id", out var idElement)) + { + _logger.LogWarning("Could not extract user ID from Graph response"); + return false; + } + + userObjectId = idElement.GetString(); + _logger.LogDebug("Retrieved current user's object ID: {UserId}", userObjectId); + } + + if (string.IsNullOrWhiteSpace(userObjectId)) + { + _logger.LogWarning("User object ID is empty, cannot add as owner"); + return false; + } + + // Check if user is already an owner (idempotency check) + _logger.LogDebug("Checking if user {UserId} is already an owner of application {AppObjectId}", userObjectId, applicationObjectId); + + var ownersDoc = await GraphGetAsync(tenantId, $"/v1.0/applications/{applicationObjectId}/owners?$select=id", ct, scopes); + if (ownersDoc != null && ownersDoc.RootElement.TryGetProperty("value", out var ownersArray)) + { + var isAlreadyOwner = ownersArray.EnumerateArray() + .Where(owner => owner.TryGetProperty("id", out var ownerId)) + .Any(owner => string.Equals(owner.GetProperty("id").GetString(), userObjectId, StringComparison.OrdinalIgnoreCase)); + + if (isAlreadyOwner) + { + _logger.LogDebug("User is already an owner of the application"); + return true; + } + } + + // User is not an owner, add them + // https://learn.microsoft.com/en-us/graph/api/application-post-owners?view=graph-rest-beta + _logger.LogDebug("Adding user {UserId} as owner to application {AppObjectId}", userObjectId, applicationObjectId); + + var payload = new JsonObject + { + ["@odata.id"] = $"{GraphApiConstants.BaseUrl}/{GraphApiConstants.Versions.Beta}/directoryObjects/{userObjectId}" + }; + + // Use beta endpoint as recommended in the documentation + var relativePath = $"/beta/applications/{applicationObjectId}/owners/$ref"; + + if (!await EnsureGraphHeadersAsync(tenantId, ct, scopes)) + { + _logger.LogWarning("Could not authenticate to Graph API to add application owner"); + return false; + } + + var url = $"{GraphApiConstants.BaseUrl}{relativePath}"; + using var content = new StringContent( + payload.ToJsonString(), + Encoding.UTF8, + "application/json"); + + using var response = await _httpClient.PostAsync(url, content, ct); + + if (response.IsSuccessStatusCode) + { + _logger.LogInformation("Successfully added user as owner to application"); + return true; + } + + var errorBody = await response.Content.ReadAsStringAsync(ct); + + // Check if the user is already an owner (409 Conflict or specific error message) + // This handles race conditions where the user was added between our check and the POST + if ((int)response.StatusCode == 409 || + errorBody.Contains("already exist", StringComparison.OrdinalIgnoreCase) || + errorBody.Contains("One or more added object references already exist", StringComparison.OrdinalIgnoreCase)) + { + _logger.LogDebug("User is already an owner of the application (detected during add)"); + return true; + } + + // Log specific error guidance based on status code + _logger.LogWarning("Failed to add user as owner to application. Status: {Status}, URL: {Url}", + response.StatusCode, url); + + if (response.StatusCode == HttpStatusCode.Forbidden) + { + _logger.LogWarning("Access denied. Ensure the authenticated user has Application.ReadWrite.All or Directory.ReadWrite.All permissions"); + _logger.LogWarning("To manually add yourself as an owner, make this Graph API call:"); + _logger.LogWarning(" POST {Url}", url); + _logger.LogWarning(" Content-Type: application/json"); + _logger.LogWarning(" Body: {{\"@odata.id\": \"{ODataId}\"}}", $"{GraphApiConstants.BaseUrl}/{GraphApiConstants.Versions.Beta}/directoryObjects/{userObjectId}"); + } + else if (response.StatusCode == HttpStatusCode.NotFound) + { + _logger.LogWarning("Application or user not found. Verify ObjectId: {AppObjectId}, UserId: {UserId}", + applicationObjectId, userObjectId); + } + else if (response.StatusCode == HttpStatusCode.BadRequest) + { + _logger.LogWarning("Bad request. Verify the payload format and user object ID"); + _logger.LogWarning("Attempted payload: {{\"@odata.id\": \"{ODataId}\"}}", $"{GraphApiConstants.BaseUrl}/{GraphApiConstants.Versions.Beta}/directoryObjects/{userObjectId}"); + } + + _logger.LogDebug("Graph API error response: {Error}", errorBody); + return false; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error adding user as owner to application: {Message}", ex.Message); + return false; + } + } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddApplicationOwnerTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddApplicationOwnerTests.cs new file mode 100644 index 0000000..1a71d2a --- /dev/null +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddApplicationOwnerTests.cs @@ -0,0 +1,372 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Net; +using System.Net.Http; +using System.Text.Json; +using FluentAssertions; +using Microsoft.Agents.A365.DevTools.Cli.Services; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Services; + +/// +/// Tests for GraphApiService.AddApplicationOwnerAsync method. +/// Verifies idempotency, owner checking, and error handling. +/// +/// +/// HttpResponseMessage objects are created inline and queued to test handlers. +/// The test handlers (TestHttpMessageHandler and CapturingHttpMessageHandler) properly dispose +/// all queued responses in their Dispose methods. Suppressing CA2000 for this pattern. +/// +#pragma warning disable CA2000 // Dispose objects before losing scope +public class GraphApiServiceAddApplicationOwnerTests +{ + private readonly ILogger _mockLogger; + private readonly CommandExecutor _mockExecutor; + + public GraphApiServiceAddApplicationOwnerTests() + { + _mockLogger = Substitute.For>(); + var mockExecutorLogger = Substitute.For>(); + _mockExecutor = Substitute.ForPartsOf(mockExecutorLogger); + + // Mock Azure CLI authentication + _mockExecutor.ExecuteAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(callInfo => + { + var cmd = callInfo.ArgAt(0); + var args = callInfo.ArgAt(1); + if (cmd == "az" && args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase)) + return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "{}", StandardError = string.Empty }); + if (cmd == "az" && args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase)) + return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = "fake-token", StandardError = string.Empty }); + return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty }); + }); + } + + [Fact] + public async Task AddApplicationOwnerAsync_WhenUserNotOwner_AddsOwnerSuccessfully() + { + // Arrange + using var handler = new TestHttpMessageHandler(); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-789"; + + // Queue response for checking existing owners (empty list) + var emptyOwnersResponse = new { value = Array.Empty() }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(emptyOwnersResponse)) + }); + + // Queue response for adding owner (success) + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeTrue("owner should be added successfully"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_WhenUserAlreadyOwner_ReturnsTrue() + { + // Arrange + using var handler = new TestHttpMessageHandler(); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-789"; + + // Queue response for checking existing owners (user is already an owner) + var existingOwnersResponse = new + { + value = new[] + { + new { id = userObjectId }, + new { id = "other-user-999" } + } + }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(existingOwnersResponse)) + }); + + // No POST should be made since user is already an owner + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeTrue("user is already an owner"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_WhenConflictError_ReturnsTrue() + { + // Arrange + using var handler = new TestHttpMessageHandler(); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-789"; + + // Queue response for checking existing owners (empty list) + var emptyOwnersResponse = new { value = Array.Empty() }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(emptyOwnersResponse)) + }); + + // Queue response for adding owner (409 Conflict - race condition) + var conflictError = new + { + error = new + { + code = "Request_BadRequest", + message = "One or more added object references already exist" + } + }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Conflict) + { + Content = new StringContent(JsonSerializer.Serialize(conflictError)) + }); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeTrue("409 conflict means user is already an owner"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_WithoutUserObjectId_RetrievesCurrentUser() + { + // Arrange + HttpRequestMessage? capturedMeRequest = null; + using var handler = new CapturingHttpMessageHandler((req) => + { + if (req.RequestUri?.AbsolutePath.Contains("/me") == true) + { + capturedMeRequest = req; + } + }); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + + // Queue response for /me request + var meResponse = new { id = "current-user-999" }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(meResponse)) + }); + + // Queue response for checking existing owners (empty list) + var emptyOwnersResponse = new { value = Array.Empty() }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(emptyOwnersResponse)) + }); + + // Queue response for adding owner (success) + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId: null); + + // Assert + result.Should().BeTrue("owner should be added successfully"); + capturedMeRequest.Should().NotBeNull("should have called /me endpoint to get current user"); + capturedMeRequest!.RequestUri!.Query.Should().Contain("$select=id"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_WhenGetCurrentUserFails_ReturnsFalse() + { + // Arrange + using var handler = new TestHttpMessageHandler(); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + + // Queue response for /me request (failure) + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Unauthorized) + { + Content = new StringContent("Authentication failed") + }); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId: null); + + // Assert + result.Should().BeFalse("should fail when unable to get current user"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_WhenAddFails_ReturnsFalse() + { + // Arrange + using var handler = new TestHttpMessageHandler(); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-789"; + + // Queue response for checking existing owners (empty list) + var emptyOwnersResponse = new { value = Array.Empty() }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(emptyOwnersResponse)) + }); + + // Queue response for adding owner (failure - forbidden) + var forbiddenError = new + { + error = new + { + code = "Authorization_RequestDenied", + message = "Insufficient privileges to complete the operation" + } + }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.Forbidden) + { + Content = new StringContent(JsonSerializer.Serialize(forbiddenError)) + }); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeFalse("should fail when insufficient privileges"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_UsesBetaEndpoint() + { + // Arrange + HttpRequestMessage? capturedAddRequest = null; + using var handler = new CapturingHttpMessageHandler((req) => + { + if (req.Method == HttpMethod.Post && req.RequestUri?.AbsolutePath.Contains("/owners/") == true) + { + capturedAddRequest = req; + } + }); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-789"; + + // Queue response for checking existing owners (empty list) + var emptyOwnersResponse = new { value = Array.Empty() }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(emptyOwnersResponse)) + }); + + // Queue response for adding owner (success) + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeTrue(); + capturedAddRequest.Should().NotBeNull("should have made POST request to add owner"); + capturedAddRequest!.RequestUri!.AbsolutePath.Should().Contain("/beta/applications/", + "should use beta endpoint as recommended in documentation"); + capturedAddRequest.RequestUri.AbsolutePath.Should().Contain($"/{appObjectId}/owners/$ref"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_SendsCorrectPayload() + { + // Arrange + string? capturedPayload = null; + using var handler = new CapturingHttpMessageHandler((req) => + { + if (req.Method == HttpMethod.Post && req.RequestUri?.AbsolutePath.Contains("/owners/") == true) + { + capturedPayload = req.Content?.ReadAsStringAsync().Result; + } + }); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-789"; + + // Queue response for checking existing owners (empty list) + var emptyOwnersResponse = new { value = Array.Empty() }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(emptyOwnersResponse)) + }); + + // Queue response for adding owner (success) + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.NoContent)); + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeTrue(); + capturedPayload.Should().NotBeNullOrWhiteSpace("should have sent payload"); + + // Verify payload structure (uses JsonObject with bracket notation for @odata.id) + var payload = JsonSerializer.Deserialize(capturedPayload!); + payload.TryGetProperty("@odata.id", out var odataId).Should().BeTrue("payload should have @odata.id property"); + odataId.GetString().Should().Be($"{Cli.Constants.GraphApiConstants.BaseUrl}/{Cli.Constants.GraphApiConstants.Versions.Beta}/directoryObjects/{userObjectId}", + "payload should use beta endpoint and reference the user object"); + } + + [Fact] + public async Task AddApplicationOwnerAsync_IsCaseInsensitiveForUserIdComparison() + { + // Arrange + using var handler = new TestHttpMessageHandler(); + var service = new GraphApiService(_mockLogger, _mockExecutor, handler); + + var tenantId = "tenant-123"; + var appObjectId = "app-obj-456"; + var userObjectId = "user-abc-123"; + + // Queue response for checking existing owners (user ID with different casing) + var existingOwnersResponse = new + { + value = new[] + { + new { id = "USER-ABC-123" } // Different case + } + }; + handler.QueueResponse(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(JsonSerializer.Serialize(existingOwnersResponse)) + }); + + // No POST should be made since user is already an owner + + // Act + var result = await service.AddApplicationOwnerAsync(tenantId, appObjectId, userObjectId); + + // Assert + result.Should().BeTrue("should match user ID case-insensitively"); + } +} +#pragma warning restore CA2000 // Dispose objects before losing scope + +// Test helper classes are defined in GraphApiServiceTests.cs diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs index 5e871d0..038fd06 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs @@ -209,6 +209,18 @@ protected override Task SendAsync(HttpRequestMessage reques var resp = _responses.Dequeue(); return Task.FromResult(resp); } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + while (_responses.Count > 0) + { + _responses.Dequeue().Dispose(); + } + } + base.Dispose(disposing); + } } // Capturing handler that captures requests AFTER headers are applied @@ -236,5 +248,17 @@ protected override Task SendAsync(HttpRequestMessage reques var resp = _responses.Dequeue(); return Task.FromResult(resp); } + + protected override void Dispose(bool disposing) + { + if (disposing) + { + while (_responses.Count > 0) + { + _responses.Dequeue().Dispose(); + } + } + base.Dispose(disposing); + } }