Skip to content

Commit 35cd754

Browse files
Josina20JosinaJoy
andauthored
Fix graph consent Issue (#216)
* Fix graph consent Issue * code review --------- Co-authored-by: Josina Joy <josjoy@microsoft.com>
1 parent 294877a commit 35cd754

7 files changed

Lines changed: 549 additions & 32 deletions

File tree

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,19 @@ public static Command CreateCommand(
308308
setupResults.Errors.Add("Messaging endpoint registration failed");
309309
}
310310

311+
// Track Graph permissions status - critical for agent token exchange
312+
setupResults.GraphPermissionsConfigured = result.GraphPermissionsConfigured;
313+
if (result.GraphInheritablePermissionsFailed)
314+
{
315+
setupResults.GraphInheritablePermissionsError = result.GraphInheritablePermissionsError
316+
?? "Microsoft Graph inheritable permissions failed to configure";
317+
setupResults.Warnings.Add($"Microsoft Graph inheritable permissions: {setupResults.GraphInheritablePermissionsError}");
318+
}
319+
else
320+
{
321+
setupResults.GraphInheritablePermissionsConfigured = true;
322+
}
323+
311324
if (!result.BlueprintCreated)
312325
{
313326
throw new GraphApiException(
@@ -393,6 +406,10 @@ public static Command CreateCommand(
393406
setupResults);
394407

395408
setupResults.BotApiPermissionsConfigured = botPermissionSetup;
409+
if (botPermissionSetup)
410+
{
411+
setupResults.BotInheritablePermissionsConfigured = setupConfig.IsBotInheritanceConfigured();
412+
}
396413
}
397414
catch (Exception botPermEx)
398415
{

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

Lines changed: 73 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,20 @@ internal class BlueprintCreationResult
3333
/// Indicates whether endpoint registration was attempted (vs. skipped via --no-endpoint or missing config)
3434
/// </summary>
3535
public bool EndpointRegistrationAttempted { get; set; }
36+
37+
/// <summary>
38+
/// Indicates whether Graph admin consent (OAuth2 permissions) was granted.
39+
/// </summary>
40+
public bool GraphPermissionsConfigured { get; set; }
41+
/// <summary>
42+
/// Indicates whether Graph inheritable permissions failed to be configured.
43+
/// This is critical for agent token exchange functionality.
44+
/// </summary>
45+
public bool GraphInheritablePermissionsFailed { get; set; }
46+
/// <summary>
47+
/// Error message when Graph inheritable permissions fail.
48+
/// </summary>
49+
public string? GraphInheritablePermissionsError { get; set; }
3650
}
3751

3852
/// <summary>
@@ -589,7 +603,10 @@ await CreateBlueprintClientSecretAsync(
589603
BlueprintAlreadyExisted = blueprintAlreadyExisted,
590604
EndpointRegistered = endpointRegistered,
591605
EndpointAlreadyExisted = endpointAlreadyExisted,
592-
EndpointRegistrationAttempted = !skipEndpointRegistration
606+
EndpointRegistrationAttempted = !skipEndpointRegistration,
607+
GraphPermissionsConfigured = blueprintResult.graphPermissionsConfigured,
608+
GraphInheritablePermissionsFailed = blueprintResult.graphInheritablePermissionsFailed,
609+
GraphInheritablePermissionsError = blueprintResult.graphInheritablePermissionsError
593610
};
594611
}
595612

@@ -649,9 +666,9 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
649666
/// Implements displayName-first discovery for idempotency: always searches by displayName from a365.config.json (the source of truth).
650667
/// Cached objectIds are only used for dependent resources (FIC, etc.) after blueprint existence is confirmed.
651668
/// Used by: BlueprintSubcommand and A365SetupRunner Phase 2.2
652-
/// Returns: (success, appId, objectId, servicePrincipalId, alreadyExisted)
669+
/// Returns: (success, appId, objectId, servicePrincipalId, alreadyExisted, graphPermissionsConfigured, graphInheritablePermissionsFailed, graphInheritablePermissionsError)
653670
/// </summary>
654-
public static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId, bool alreadyExisted)> CreateAgentBlueprintAsync(
671+
public static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId, bool alreadyExisted, bool graphPermissionsConfigured, bool graphInheritablePermissionsFailed, string? graphInheritablePermissionsError)> CreateAgentBlueprintAsync(
655672
ILogger logger,
656673
CommandExecutor executor,
657674
GraphApiService graphApiService,
@@ -736,7 +753,7 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
736753
{
737754
logger.LogError("Existing blueprint found but required identifiers are missing (AppId: {AppId}, ObjectId: {ObjectId})",
738755
existingAppId, existingObjectId);
739-
return (false, null, null, null, alreadyExisted: false);
756+
return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
740757
}
741758

742759
return await CompleteBlueprintConfigurationAsync(
@@ -812,7 +829,7 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
812829
if (string.IsNullOrEmpty(graphToken))
813830
{
814831
logger.LogError("Failed to extract access token from Graph client");
815-
return (false, null, null, null, alreadyExisted: false);
832+
return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
816833
}
817834

818835
// Create the application using Microsoft Graph SDK
@@ -857,13 +874,13 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
857874
{
858875
errorContent = await appResponse.Content.ReadAsStringAsync(ct);
859876
logger.LogError("Failed to create application (fallback): {Status} - {Error}", appResponse.StatusCode, errorContent);
860-
return (false, null, null, null, alreadyExisted: false);
877+
return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
861878
}
862879
}
863880
else
864881
{
865882
logger.LogError("Failed to create application: {Status} - {Error}", appResponse.StatusCode, errorContent);
866-
return (false, null, null, null, alreadyExisted: false);
883+
return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
867884
}
868885
}
869886

@@ -893,7 +910,7 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
893910
if (!appAvailable)
894911
{
895912
logger.LogError("Application object not available after creation and retries. Aborting setup.");
896-
return (false, null, null, null, alreadyExisted: false);
913+
return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
897914
}
898915

899916
logger.LogInformation("Application object verified in directory");
@@ -1013,15 +1030,15 @@ public static async Task<bool> EnsureDelegatedConsentWithRetriesAsync(
10131030
catch (Exception ex)
10141031
{
10151032
logger.LogError(ex, "Failed to create agent blueprint: {Message}", ex.Message);
1016-
return (false, null, null, null, alreadyExisted: false);
1033+
return (false, null, null, null, alreadyExisted: false, graphPermissionsConfigured: false, graphInheritablePermissionsFailed: false, graphInheritablePermissionsError: null);
10171034
}
10181035
}
10191036

10201037
/// <summary>
10211038
/// Completes blueprint configuration by validating/creating federated credentials and requesting admin consent.
10221039
/// Called by both existing blueprint and new blueprint paths to ensure consistent configuration.
10231040
/// </summary>
1024-
private static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId, bool alreadyExisted)> CompleteBlueprintConfigurationAsync(
1041+
private static async Task<(bool success, string? appId, string? objectId, string? servicePrincipalId, bool alreadyExisted, bool graphPermissionsConfigured, bool graphInheritablePermissionsFailed, string? graphInheritablePermissionsError)> CompleteBlueprintConfigurationAsync(
10251042
ILogger logger,
10261043
CommandExecutor executor,
10271044
GraphApiService graphApiService,
@@ -1152,7 +1169,7 @@ await retryHelper.ExecuteWithRetryAsync(
11521169
// Admin Consent
11531170
// ========================================================================
11541171

1155-
var (consentSuccess, consentUrlGraph) = await EnsureAdminConsentAsync(
1172+
var (consentSuccess, consentUrlGraph, graphInheritablePermissionsConfigured, graphInheritablePermissionsError) = await EnsureAdminConsentAsync(
11561173
logger,
11571174
executor,
11581175
graphApiService,
@@ -1189,7 +1206,9 @@ await retryHelper.ExecuteWithRetryAsync(
11891206
logger.LogWarning("Consent URL: {Url}", consentUrlGraph);
11901207
}
11911208

1192-
return (true, appId, objectId, servicePrincipalId, alreadyExisted);
1209+
// Track Graph permissions status - this is critical for agent token exchange
1210+
bool graphPermissionsFailed = !graphInheritablePermissionsConfigured;
1211+
return (true, appId, objectId, servicePrincipalId, alreadyExisted, consentSuccess, graphPermissionsFailed, graphInheritablePermissionsError);
11931212
}
11941213

11951214
/// <summary>
@@ -1225,9 +1244,9 @@ private static List<string> GetApplicationScopes(Models.Agent365Config setupConf
12251244
/// Ensures admin consent for the blueprint application.
12261245
/// For existing blueprints, checks if consent already exists before requesting browser interaction.
12271246
/// For new blueprints, skips verification and directly requests consent.
1228-
/// Returns: (consentSuccess, consentUrl)
1247+
/// Returns: (consentSuccess, consentUrl, graphInheritablePermissionsConfigured, graphInheritablePermissionsError)
12291248
/// </summary>
1230-
private static async Task<(bool consentSuccess, string consentUrl)> EnsureAdminConsentAsync(
1249+
private static async Task<(bool consentSuccess, string consentUrl, bool graphInheritablePermissionsConfigured, string? graphInheritablePermissionsError)> EnsureAdminConsentAsync(
12311250
ILogger logger,
12321251
CommandExecutor executor,
12331252
GraphApiService graphApiService,
@@ -1294,7 +1313,39 @@ private static List<string> GetApplicationScopes(Models.Agent365Config setupConf
12941313

12951314
if (consentAlreadyExists)
12961315
{
1297-
return (true, consentUrlGraph);
1316+
// For existing consent, we still need to verify/configure inheritable permissions
1317+
logger.LogInformation("Configuring inheritable permissions for Microsoft Graph...");
1318+
bool graphInheritableConfigured = false;
1319+
string? graphInheritableError = null;
1320+
try
1321+
{
1322+
setupConfig.AgentBlueprintId = appId;
1323+
1324+
await SetupHelpers.EnsureResourcePermissionsAsync(
1325+
graph: graphApiService,
1326+
blueprintService: blueprintService,
1327+
config: setupConfig,
1328+
resourceAppId: AuthenticationConstants.MicrosoftGraphResourceAppId,
1329+
resourceName: "Microsoft Graph",
1330+
scopes: applicationScopes.ToArray(),
1331+
logger: logger,
1332+
addToRequiredResourceAccess: false,
1333+
setInheritablePermissions: true,
1334+
setupResults: null,
1335+
ct: ct);
1336+
1337+
logger.LogInformation("Microsoft Graph inheritable permissions configured successfully");
1338+
graphInheritableConfigured = true;
1339+
}
1340+
catch (Exception ex)
1341+
{
1342+
graphInheritableError = ex.Message;
1343+
logger.LogWarning("Failed to configure Microsoft Graph inheritable permissions: {Message}", ex.Message);
1344+
logger.LogWarning("Agent instances may not be able to access Microsoft Graph resources");
1345+
logger.LogWarning("You can configure these manually later with: a365 setup blueprint");
1346+
}
1347+
1348+
return (true, consentUrlGraph, graphInheritableConfigured, graphInheritableError);
12981349
}
12991350

13001351
// Request consent via browser
@@ -1305,6 +1356,9 @@ private static List<string> GetApplicationScopes(Models.Agent365Config setupConf
13051356

13061357
var consentSuccess = await AdminConsentHelper.PollAdminConsentAsync(executor, logger, appId, "Graph API Scopes", 180, 5, ct);
13071358

1359+
bool graphInheritablePermissionsConfigured = false;
1360+
string? graphInheritablePermissionsError = null;
1361+
13081362
if (consentSuccess)
13091363
{
13101364
logger.LogInformation("Graph API admin consent granted successfully!");
@@ -1329,20 +1383,22 @@ await SetupHelpers.EnsureResourcePermissionsAsync(
13291383
ct: ct);
13301384

13311385
logger.LogInformation("Microsoft Graph inheritable permissions configured successfully");
1386+
graphInheritablePermissionsConfigured = true;
13321387
}
13331388
catch (Exception ex)
13341389
{
1390+
graphInheritablePermissionsError = ex.Message;
13351391
logger.LogWarning("Failed to configure Microsoft Graph inheritable permissions: {Message}", ex.Message);
13361392
logger.LogWarning("Agent instances may not be able to access Microsoft Graph resources");
1337-
logger.LogWarning("You can configure these manually later with: a365 setup permissions");
1393+
logger.LogWarning("You can configure these manually later with: a365 setup blueprint");
13381394
}
13391395
}
13401396
else
13411397
{
13421398
logger.LogWarning("Graph API admin consent may not have completed");
13431399
}
13441400

1345-
return (consentSuccess, consentUrlGraph);
1401+
return (consentSuccess, consentUrlGraph, graphInheritablePermissionsConfigured, graphInheritablePermissionsError);
13461402
}
13471403

13481404
/// <summary>

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

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -92,20 +92,23 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger)
9292
var status = results.BlueprintAlreadyExisted ? "configured (already exists)" : "created";
9393
logger.LogInformation(" [OK] Agent blueprint {Status} (Blueprint ID: {BlueprintId})", status, results.BlueprintId ?? "unknown");
9494
}
95-
if (results.McpPermissionsConfigured)
95+
if (results.McpPermissionsConfigured && results.InheritablePermissionsConfigured)
9696
{
97-
var status = results.McpPermissionsAlreadyExisted ? "verified (already configured)" : "configured";
98-
logger.LogInformation(" [OK] MCP server permissions {Status}", status);
97+
var permStatus = results.McpPermissionsAlreadyExisted ? "verified" : "configured";
98+
var inheritStatus = results.InheritablePermissionsAlreadyExisted ? "verified" : "configured";
99+
logger.LogInformation(" [OK] MCP Tools permissions {PermStatus}, inheritable permissions {InheritStatus}", permStatus, inheritStatus);
99100
}
100-
if (results.InheritablePermissionsConfigured)
101+
if (results.BotApiPermissionsConfigured && results.BotInheritablePermissionsConfigured)
101102
{
102-
var status = results.InheritablePermissionsAlreadyExisted ? "verified (already configured)" : "configured";
103-
logger.LogInformation(" [OK] Inheritable permissions {Status}", status);
103+
var permStatus = results.BotApiPermissionsAlreadyExisted ? "verified" : "configured";
104+
var inheritStatus = results.BotInheritablePermissionsAlreadyExisted ? "verified" : "configured";
105+
logger.LogInformation(" [OK] Messaging Bot API permissions {PermStatus}, inheritable permissions {InheritStatus}", permStatus, inheritStatus);
104106
}
105-
if (results.BotApiPermissionsConfigured)
107+
if (results.GraphPermissionsConfigured && results.GraphInheritablePermissionsConfigured)
106108
{
107-
var status = results.BotApiPermissionsAlreadyExisted ? "verified (already configured)" : "configured";
108-
logger.LogInformation(" [OK] Messaging Bot API permissions {Status}", status);
109+
var permStatus = results.GraphPermissionsAlreadyExisted ? "verified" : "configured";
110+
var inheritStatus = results.GraphInheritablePermissionsAlreadyExisted ? "verified" : "configured";
111+
logger.LogInformation(" [OK] Microsoft Graph permissions {PermStatus}, inheritable permissions {InheritStatus}", permStatus, inheritStatus);
109112
}
110113
if (results.MessagingEndpointRegistered)
111114
{
@@ -144,19 +147,19 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger)
144147
logger.LogInformation("");
145148
logger.LogInformation("Recovery Actions:");
146149

147-
if (!results.InheritablePermissionsConfigured)
150+
if (!results.McpPermissionsConfigured || !results.InheritablePermissionsConfigured)
148151
{
149-
logger.LogInformation(" - Inheritable Permissions: Run 'a365 setup permissions mcp' to retry");
152+
logger.LogInformation(" - MCP Tools Permissions: Run 'a365 setup permissions mcp' to retry");
150153
}
151154

152-
if (!results.McpPermissionsConfigured)
155+
if (!results.BotApiPermissionsConfigured || !results.BotInheritablePermissionsConfigured)
153156
{
154-
logger.LogInformation(" - MCP Permissions: Run 'a365 setup permissions mcp' to retry");
157+
logger.LogInformation(" - Messaging Bot API Permissions: Run 'a365 setup permissions bot' to retry");
155158
}
156159

157-
if (!results.BotApiPermissionsConfigured)
160+
if (!results.GraphPermissionsConfigured || !results.GraphInheritablePermissionsConfigured)
158161
{
159-
logger.LogInformation(" - Bot API Permissions: Run 'a365 setup permissions bot' to retry");
162+
logger.LogInformation(" - Microsoft Graph Permissions: Run 'a365 setup blueprint' to retry");
160163
}
161164

162165
if (!results.MessagingEndpointRegistered)
@@ -168,6 +171,15 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger)
168171
else if (results.HasWarnings)
169172
{
170173
logger.LogInformation("Setup completed successfully with warnings");
174+
logger.LogInformation("");
175+
logger.LogInformation("Recovery Actions:");
176+
177+
if (!string.IsNullOrEmpty(results.GraphInheritablePermissionsError))
178+
{
179+
logger.LogInformation(" - Graph Inheritable Permissions: Run 'a365 setup blueprint' to retry");
180+
}
181+
182+
logger.LogInformation("");
171183
logger.LogInformation("Review warnings above and take action if needed");
172184
}
173185
else
@@ -386,6 +398,7 @@ public static async Task EnsureResourcePermissionsAsync(
386398
else if (resourceName.Contains("Bot", StringComparison.OrdinalIgnoreCase))
387399
{
388400
setupResults.BotApiPermissionsAlreadyExisted = inheritanceAlreadyExisted;
401+
setupResults.BotInheritablePermissionsAlreadyExisted = inheritanceAlreadyExisted;
389402
}
390403
}
391404

0 commit comments

Comments
 (0)