Skip to content

Commit 96f1f34

Browse files
author
Jesus Terrazas
committed
Add ResolveResource for duplicate code
1 parent c7d4966 commit 96f1f34

5 files changed

Lines changed: 267 additions & 98 deletions

File tree

‎src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/AddPermissionsSubcommand.cs‎

Lines changed: 74 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
using Microsoft.Agents.A365.DevTools.Cli.Constants;
54
using Microsoft.Agents.A365.DevTools.Cli.Helpers;
65
using Microsoft.Agents.A365.DevTools.Cli.Services;
76
using Microsoft.Extensions.Logging;
@@ -51,10 +50,21 @@ public static Command CreateCommand(
5150
["--verbose", "-v"],
5251
description: "Show detailed output");
5352

54-
var resourceOption = new Option<string>(
53+
var resourceOption = new Option<string?>(
5554
["--resource", "-r"],
56-
getDefaultValue: () => "mcp",
57-
description: "Target resource API: 'mcp' (default), 'powerplatform'");
55+
description: "Target resource API: 'mcp' (default), 'powerplatform'. " +
56+
"When specified, --scopes is required for non-mcp resources.")
57+
{
58+
IsRequired = false
59+
};
60+
61+
var resourceIdOption = new Option<string?>(
62+
["--resource-id"],
63+
description: "Resource application ID (GUID) to add permissions for. " +
64+
"When specified, --scopes is required.")
65+
{
66+
IsRequired = false
67+
};
5868

5969
var dryRunOption = new Option<bool>(
6070
["--dry-run"],
@@ -65,10 +75,11 @@ public static Command CreateCommand(
6575
command.AddOption(appIdOption);
6676
command.AddOption(scopesOption);
6777
command.AddOption(resourceOption);
78+
command.AddOption(resourceIdOption);
6879
command.AddOption(verboseOption);
6980
command.AddOption(dryRunOption);
7081

71-
command.SetHandler(async (config, manifest, appId, scopes, resource, verbose, dryRun) =>
82+
command.SetHandler(async (config, manifest, appId, scopes, resource, resourceId, verbose, dryRun) =>
7283
{
7384
try
7485
{
@@ -120,6 +131,32 @@ public static Command CreateCommand(
120131
var manifestPath = manifest?.FullName
121132
?? Path.Combine(setupConfig?.DeploymentProjectPath ?? Environment.CurrentDirectory, "ToolingManifest.json");
122133

134+
var environment = setupConfig?.Environment ?? "prod";
135+
136+
// Resolve resource app ID
137+
ResolvedResource resolvedResource;
138+
try
139+
{
140+
resolvedResource = ResourceResolutionHelper.ResolveResource(resourceId, resource, environment);
141+
}
142+
catch (ArgumentException ex)
143+
{
144+
logger.LogError("Resource resolution error: {ErrorMessage}", ex.Message);
145+
logger.LogInformation("");
146+
logger.LogInformation("Example: a365 develop add-permissions --resource-id 12345678-1234-1234-1234-123456789abc --scopes .default");
147+
Environment.Exit(1);
148+
return;
149+
}
150+
151+
var resourceAppId = resolvedResource.ResourceAppId;
152+
var resourceName = resolvedResource.DisplayName;
153+
154+
logger.LogInformation("Target resource: {ResourceName} ({ResourceAppId})", resourceName, resourceAppId);
155+
logger.LogInformation("");
156+
157+
// Determine if custom resource is being used
158+
bool isCustomResource = !string.IsNullOrWhiteSpace(resource) || !string.IsNullOrWhiteSpace(resourceId);
159+
123160
// Determine which scopes to add
124161
string[] requestedScopes;
125162

@@ -130,67 +167,48 @@ public static Command CreateCommand(
130167
logger.LogInformation("Using user-specified scopes: {Scopes}", string.Join(", ", requestedScopes));
131168
logger.LogInformation("");
132169
}
170+
else if (isCustomResource)
171+
{
172+
logger.LogError("The --scopes option is required when using --resource or --resource-id.");
173+
logger.LogInformation("");
174+
logger.LogInformation("Manifest-based scopes are only supported for the default flow.");
175+
logger.LogInformation("Please omit the --resource and --resource-id options if you'd like to use manifest-based scopes.");
176+
logger.LogInformation("");
177+
logger.LogInformation("Example: a365 develop add-permissions --resource powerplatform --scopes .default");
178+
Environment.Exit(1);
179+
return;
180+
}
133181
else
134182
{
135-
// Only read scopes from ToolingManifest.json for mcp resource
136-
if (resource.ToLowerInvariant() is "mcp")
183+
// Default MCP flow: read scopes from ToolingManifest.json
184+
if (!File.Exists(manifestPath))
137185
{
138-
// Read scopes from ToolingManifest.json
139-
if (!File.Exists(manifestPath))
140-
{
141-
logger.LogError("ToolingManifest.json not found at: {Path}", manifestPath);
142-
logger.LogInformation("");
143-
logger.LogInformation("Please ensure ToolingManifest.json exists in your project directory");
144-
logger.LogInformation("or specify scopes explicitly with --scopes option.");
145-
logger.LogInformation("");
146-
logger.LogInformation("Example: a365 develop add-permissions --scopes McpServers.Mail.All McpServers.Calendar.All");
147-
Environment.Exit(1);
148-
return;
149-
}
150-
151-
logger.LogInformation("Reading MCP server configuration from: {Path}", manifestPath);
152-
153-
// Use ManifestHelper to extract scopes (includes fallback to mappings and McpServersMetadata.Read.All)
154-
requestedScopes = await ManifestHelper.GetRequiredScopesAsync(manifestPath);
155-
156-
if (requestedScopes.Length == 0)
157-
{
158-
logger.LogError("No scopes found in ToolingManifest.json");
159-
logger.LogInformation("You can specify scopes explicitly with --scopes option.");
160-
Environment.Exit(1);
161-
return;
162-
}
163-
164-
logger.LogInformation("Collected {Count} unique scope(s) from manifest: {Scopes}",
165-
requestedScopes.Length, string.Join(", ", requestedScopes));
166-
}
167-
else
168-
{
169-
// For other resources (like powerplatform), scopes are required
170-
logger.LogError("--scopes is required when --resource {resource} is specified.", resource);
186+
logger.LogError("ToolingManifest.json not found at: {Path}", manifestPath);
187+
logger.LogInformation("");
188+
logger.LogInformation("Please ensure ToolingManifest.json exists in your project directory");
189+
logger.LogInformation("or specify scopes explicitly with --scopes option.");
171190
logger.LogInformation("");
172-
logger.LogInformation("Example: a365 develop add-permissions --resource {resource} --scopes ExampleScope.ReadWrite.All", resource);
191+
logger.LogInformation("Example: a365 develop add-permissions --scopes McpServers.Mail.All McpServers.Calendar.All");
173192
Environment.Exit(1);
174193
return;
175194
}
176-
}
177195

178-
var environment = setupConfig?.Environment ?? "prod";
196+
logger.LogInformation("Reading MCP server configuration from: {Path}", manifestPath);
179197

180-
// Resolve resource configuration based on --resource option
181-
var resolvedResource = ResourceResolutionHelper.ResolveByKeyword(resource, environment);
182-
if (resolvedResource is null)
183-
{
184-
logger.LogError(ErrorMessages.UnknownResourceKeyword, resource);
185-
Environment.Exit(1);
186-
return;
187-
}
198+
// Use ManifestHelper to extract scopes (includes fallback to mappings and McpServersMetadata.Read.All)
199+
requestedScopes = await ManifestHelper.GetRequiredScopesAsync(manifestPath);
188200

189-
var resourceAppId = resolvedResource.ResourceAppId;
190-
var resourceName = resolvedResource.DisplayName;
201+
if (requestedScopes.Length == 0)
202+
{
203+
logger.LogError("No scopes found in ToolingManifest.json");
204+
logger.LogInformation("You can specify scopes explicitly with --scopes option.");
205+
Environment.Exit(1);
206+
return;
207+
}
191208

192-
logger.LogInformation("Target resource: {ResourceName} ({ResourceAppId})", resourceName, resourceAppId);
193-
logger.LogInformation("");
209+
logger.LogInformation("Collected {Count} unique scope(s) from manifest: {Scopes}",
210+
requestedScopes.Length, string.Join(", ", requestedScopes));
211+
}
194212

195213
// Dry run mode
196214
if (dryRun)
@@ -262,7 +280,7 @@ public static Command CreateCommand(
262280
logger.LogError(ex, "Failed to add API permissions: {Message}", ex.Message);
263281
Environment.Exit(1);
264282
}
265-
}, configOption, manifestOption, appIdOption, scopesOption, resourceOption, verboseOption, dryRunOption);
283+
}, configOption, manifestOption, appIdOption, scopesOption, resourceOption, resourceIdOption, verboseOption, dryRunOption);
266284

267285
return command;
268286
}

‎src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetTokenSubcommand.cs‎

Lines changed: 17 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,6 @@ public static Command CreateCommand(
104104

105105
try
106106
{
107-
// Validate mutual exclusivity of --resource and --resource-id
108-
if (!string.IsNullOrWhiteSpace(resource) && !string.IsNullOrWhiteSpace(resourceId))
109-
{
110-
logger.LogError("Cannot specify both --resource and --resource-id. Use one or the other.");
111-
Environment.Exit(1);
112-
return;
113-
}
114-
115107
// Determine if custom resource is being used
116108
bool isCustomResource = !string.IsNullOrWhiteSpace(resource) || !string.IsNullOrWhiteSpace(resourceId);
117109

@@ -143,44 +135,28 @@ public static Command CreateCommand(
143135
var environment = setupConfig?.Environment ?? "prod";
144136

145137
// Resolve resource app ID
146-
string resourceAppId;
147-
string resourceDisplayName;
148-
string? resourceUrl = null;
149-
if (!string.IsNullOrWhiteSpace(resourceId))
138+
ResolvedResource resolvedResource;
139+
try
150140
{
151-
// Validate that resource ID is a valid GUID
152-
if (!Guid.TryParse(resourceId, out _))
153-
{
154-
logger.LogError("Invalid resource application ID: {ResourceId}. Expected a valid GUID.", resourceId);
155-
logger.LogInformation("");
156-
logger.LogInformation("Example: a365 develop get-token --resource-id 12345678-1234-1234-1234-123456789abc --scopes .default");
157-
Environment.Exit(1);
158-
return;
159-
}
160-
161-
// User provided explicit resource ID
162-
var customResolved = ResourceResolutionHelper.ResolveByCustomId(resourceId);
163-
resourceAppId = customResolved.ResourceAppId;
164-
resourceDisplayName = customResolved.DisplayName;
165-
logger.LogInformation("Using custom resource ID: {ResourceId}", resourceId);
141+
resolvedResource = ResourceResolutionHelper.ResolveResource(resourceId, resource, environment);
166142
}
167-
else
143+
catch (ArgumentException ex)
168144
{
169-
// Resolve resource keyword to GUID (default to "mcp" if null)
170-
var resolved = ResourceResolutionHelper.ResolveByKeyword(resource, environment);
171-
if (resolved is null)
172-
{
173-
logger.LogError(ErrorMessages.UnknownResourceKeyword, resource);
174-
Environment.Exit(1);
175-
return;
176-
}
177-
178-
resourceAppId = resolved.ResourceAppId;
179-
resourceDisplayName = resolved.DisplayName;
180-
resourceUrl = resolved.Url;
181-
logger.LogInformation("Using resource: {DisplayName}", resourceDisplayName);
145+
logger.LogError("Resource resolution error: {ErrorMessage}", ex.Message);
146+
logger.LogInformation("");
147+
logger.LogInformation("Example: a365 develop get-token --resource-id 12345678-1234-1234-1234-123456789abc --scopes .default");
148+
Environment.Exit(1);
149+
return;
182150
}
183151

152+
var resourceAppId = resolvedResource.ResourceAppId;
153+
var resourceDisplayName = resolvedResource.DisplayName;
154+
var resourceUrl = resolvedResource.Url;
155+
156+
// Log which resource was selected
157+
logger.LogInformation("Selected resource: {ResourceDisplayName} (App ID: {ResourceAppId})",
158+
resourceDisplayName, resourceAppId);
159+
184160
// Determine which scopes to request
185161
string[] requestedScopes;
186162

‎src/Microsoft.Agents.A365.DevTools.Cli/Helpers/ResourceResolutionHelper.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,44 @@ public static ResolvedResource ResolveByCustomId(string resourceId)
5858
{
5959
return new ResolvedResource(resourceId, $"Custom Resource ({resourceId})", null);
6060
}
61+
62+
/// <summary>
63+
/// Resolves a resource from either a custom resource ID (GUID) or a keyword.
64+
/// Handles mutual exclusivity validation and GUID validation.
65+
/// </summary>
66+
/// <param name="resourceId">The custom resource application ID (GUID), or null.</param>
67+
/// <param name="resource">The resource keyword (e.g., "mcp", "powerplatform"), or null.</param>
68+
/// <param name="environment">The environment to use for environment-aware resource resolution (e.g., "prod").</param>
69+
/// <returns>A <see cref="ResolvedResource"/> containing the app ID, display name, and optional URL.</returns>
70+
/// <exception cref="ArgumentException">Thrown when both resourceId and resource are provided, when resourceId is not a valid GUID, or when the resource keyword is unknown.</exception>
71+
public static ResolvedResource ResolveResource(string? resourceId, string? resource, string environment)
72+
{
73+
// Validate mutual exclusivity
74+
if (!string.IsNullOrWhiteSpace(resourceId) && !string.IsNullOrWhiteSpace(resource))
75+
{
76+
throw new ArgumentException("Cannot specify both resourceId and resource. Use one or the other.");
77+
}
78+
79+
if (!string.IsNullOrWhiteSpace(resourceId))
80+
{
81+
// Validate that resource ID is a valid GUID
82+
if (!Guid.TryParse(resourceId, out _))
83+
{
84+
throw new ArgumentException($"Invalid resource application ID: {resourceId}. Expected a valid GUID.");
85+
}
86+
87+
return ResolveByCustomId(resourceId);
88+
}
89+
else
90+
{
91+
// Resolve resource keyword to GUID (defaults to "mcp" if null)
92+
var resolved = ResolveByKeyword(resource, environment);
93+
if (resolved is null)
94+
{
95+
throw new ArgumentException($"Unknown resource keyword: {resource}");
96+
}
97+
98+
return resolved;
99+
}
100+
}
61101
}

‎src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AddPermissionsSubcommandTests.cs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,31 @@ public void CreateCommand_ShouldHaveVerboseOption()
114114
verboseOption.Aliases.Should().Contain("-v");
115115
}
116116

117+
[Fact]
118+
public void CreateCommand_ShouldHaveResourceOption()
119+
{
120+
// Act
121+
var command = AddPermissionsSubcommand.CreateCommand(_mockLogger, _mockConfigService, _mockGraphApiService, _mockBlueprintService);
122+
123+
// Assert
124+
var resourceOption = command.Options.FirstOrDefault(o => o.Name == "resource");
125+
resourceOption.Should().NotBeNull();
126+
resourceOption!.Aliases.Should().Contain("--resource");
127+
resourceOption.Aliases.Should().Contain("-r");
128+
}
129+
130+
[Fact]
131+
public void CreateCommand_ShouldHaveResourceIdOption()
132+
{
133+
// Act
134+
var command = AddPermissionsSubcommand.CreateCommand(_mockLogger, _mockConfigService, _mockGraphApiService, _mockBlueprintService);
135+
136+
// Assert
137+
var resourceIdOption = command.Options.FirstOrDefault(o => o.Name == "resource-id");
138+
resourceIdOption.Should().NotBeNull();
139+
resourceIdOption!.Aliases.Should().Contain("--resource-id");
140+
}
141+
117142
[Fact]
118143
public void CreateCommand_ShouldHaveDryRunOption()
119144
{
@@ -133,7 +158,7 @@ public void CreateCommand_ShouldHaveAllRequiredOptions()
133158
var command = AddPermissionsSubcommand.CreateCommand(_mockLogger, _mockConfigService, _mockGraphApiService, _mockBlueprintService);
134159

135160
// Assert
136-
command.Options.Should().HaveCount(7);
161+
command.Options.Should().HaveCount(8);
137162
var optionNames = command.Options.Select(opt => opt.Name).ToList();
138163
optionNames.Should().Contain(new[]
139164
{
@@ -142,6 +167,7 @@ public void CreateCommand_ShouldHaveAllRequiredOptions()
142167
"app-id",
143168
"scopes",
144169
"resource",
170+
"resource-id",
145171
"verbose",
146172
"dry-run"
147173
});

0 commit comments

Comments
 (0)