Ensure blueprint creator is app owner; add tests (#163)

* Ensure blueprint creator is app owner; add tests

Adds logic to make the current user an owner of the Agent Blueprint application during configuration, using a new AddApplicationOwnerAsync method in GraphApiService. This method checks for existing ownership, adds the user if needed via the Graph beta API, and handles errors with clear logging and remediation steps. Includes a comprehensive test suite covering all major scenarios. This improves usability by ensuring the creator can manage the app in Azure and the Developer Portal. No breaking changes.

* Ensure proper disposal in tests; update Graph API payload

- Update testing standards to require disposing IDisposable objects (e.g., HttpResponseMessage) in tests.
- Refactor GraphApiService to use LINQ for owner checks and use JsonObject with "@odata.id" for payloads.
- Use using statements for StringContent and test handlers to ensure disposal.
- Update tests to check for "@odata.id" and dispose handlers properly.
- Add Dispose overrides to test handlers to clean up HttpResponseMessage instances.

* Improve app owner assignment via Graph API, add constants

Refactored owner assignment to use new GraphApiConstants for URLs, versions, and scopes. Now explicitly requests Application.ReadWrite.All permissions and uses the beta endpoint with a fully qualified @odata.id payload. Enhanced error logging with actionable guidance. Updated tests for new payload format. Improves standards compliance and user experience.

Author: sellakumaran

.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

docs/guides/custom-client-app-registration.md

@@ -892,8 +892,8 @@ public static async Task<bool> 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<bool> 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
         // ========================================================================

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs

@@ -0,0 +1,52 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace Microsoft.Agents.A365.DevTools.Cli.Constants;
+
+/// <summary>
+/// Constants for Microsoft Graph API endpoints and resources
+/// </summary>
+public static class GraphApiConstants
+{
+    /// <summary>
+    /// Base URL for Microsoft Graph API
+    /// </summary>
+    public const string BaseUrl = "https://graph.microsoft.com";
+
+    /// <summary>
+    /// Resource identifier for Microsoft Graph API (used in Azure CLI token acquisition)
+    /// </summary>
+    public const string Resource = "https://graph.microsoft.com/";
+
+    /// <summary>
+    /// Endpoint versions
+    /// </summary>
+    public static class Versions
+    {
+        /// <summary>
+        /// Stable v1.0 endpoint for production workloads
+        /// </summary>
+        public const string V1 = "v1.0";
+
+        /// <summary>
+        /// Beta endpoint for preview features
+        /// </summary>
+        public const string Beta = "beta";
+    }
+
+    /// <summary>
+    /// Common Microsoft Graph permission scopes
+    /// </summary>
+    public static class Scopes
+    {
+        /// <summary>
+        /// Application.ReadWrite.All permission scope - required for managing application registrations
+        /// </summary>
+        public const string ApplicationReadWriteAll = "https://graph.microsoft.com/Application.ReadWrite.All";
+
+        /// <summary>
+        /// Directory.ReadWrite.All permission scope - required for directory-level operations
+        /// </summary>
+        public const string DirectoryReadWriteAll = "https://graph.microsoft.com/Directory.ReadWrite.All";
+    }
+}

src/Microsoft.Agents.A365.DevTools.Cli/Constants/GraphApiConstants.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,21 +415,21 @@ public async Task<bool> CreateOrUpdateOauth2PermissionGrantAsync(
     /// <param name="ct">Cancellation token</param>
     /// <returns>True if user has required roles, false otherwise</returns>
     public virtual async Task<(bool hasPrivileges, List<string> 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)
             {
                 _logger.LogWarning("Could not acquire Graph token to check privileges");
                 return (false, new List<string>());
             }
 
-            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<bool> 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<bool> CreateOrUpdateOauth2PermissionGrantAsync(
             return (false, new List<string>());
         }
     }
+
+    /// <summary>
+    /// 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
+    /// </summary>
+    /// <param name="tenantId">The tenant ID</param>
+    /// <param name="applicationObjectId">The application object ID (not the client/app ID)</param>
+    /// <param name="userObjectId">The user's object ID to add as owner. If null, uses the current authenticated user.</param>
+    /// <param name="ct">Cancellation token</param>
+    /// <param name="scopes">OAuth2 scopes for elevated permissions (e.g., Application.ReadWrite.All, Directory.ReadWrite.All)</param>
+    /// <returns>True if the user is an owner (either already was or was successfully added), false otherwise</returns>
+    public virtual async Task<bool> AddApplicationOwnerAsync(
+        string tenantId,
+        string applicationObjectId,
+        string? userObjectId = null,
+        CancellationToken ct = default,
+        IEnumerable<string>? 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;
+        }
+    }
 }