From 8f1ea46bd01265640ae2629887f5f3eba8c5ac9b Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 12:08:22 +0100 Subject: [PATCH 01/10] Add GraphMissingPermissionsGuidancePlugin. Closes #88 Adds a guidance plugin that inspects Microsoft Graph responses with 403 Forbidden (including 403s inside $batch responses), compares the permissions on the access token with the permissions required for the endpoint (retrieved from the Microsoft Graph DevX permissions API and cached in memory), and logs which permission to add and how. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 241 +++++++++++++++ .../GraphMissingPermissionsGuidancePlugin.cs | 292 ++++++++++++++++++ .../GraphMinimalPermissionsGuidancePlugin.cs | 46 +-- DevProxy.Plugins/Utils/GraphUtils.cs | 42 +++ DevProxy/config/m365.json | 15 + DevProxy/config/microsoft-graph.json | 5 + skills/dev-proxy/references/configuration.md | 1 + skills/dev-proxy/references/plugin-catalog.md | 1 + 8 files changed, 599 insertions(+), 44 deletions(-) create mode 100644 DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs create mode 100644 DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs new file mode 100644 index 000000000..4efdc6286 --- /dev/null +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -0,0 +1,241 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Net; +using System.Text; +using System.Text.Json; +using DevProxy.Abstractions.Proxy; +using DevProxy.Plugins.Guidance; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace DevProxy.Integration.Tests; + +/// +/// Coverage for . The Microsoft Graph +/// DevX permissions API is stubbed with so the tests +/// stay hermetic. +/// +public sealed class GraphMissingPermissionsGuidancePluginTests +{ + private const string MailPermissionsJson = """ + [ + { "value": "Mail.Read", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, + { "value": "Mail.ReadBasic", "scopeType": "DelegatedWork", "isLeastPrivilege": true, "isHidden": false }, + { "value": "Mail.ReadWrite", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false } + ] + """; + + private const string UserPermissionsJson = """ + [ + { "value": "User.Read.All", "scopeType": "Application", "isLeastPrivilege": false, "isHidden": false }, + { "value": "User.ReadBasic.All", "scopeType": "Application", "isLeastPrivilege": true, "isHidden": false } + ] + """; + + private static ISet GraphWatch => KestrelProxyHarness.BuildUrlsToWatch("graph.microsoft.com"); + + [Fact] + public async Task Forbidden_DelegatedTokenMissingPermission_LogsFailedWithLeastPrivilegedPermission() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages", [("Authorization", BearerToken(scp: "User.Read openid"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("least privileged permission: Mail.ReadBasic", log.Message, StringComparison.Ordinal); + Assert.Contains("Mail.Read, Mail.ReadWrite", log.Message, StringComparison.Ordinal); + Assert.Contains("openid, User.Read", log.Message, StringComparison.Ordinal); + Assert.Contains("consent", log.Message, StringComparison.OrdinalIgnoreCase); + + var requestUri = Assert.Single(handler.Requests); + Assert.Contains("scopeType=DelegatedWork", requestUri.Query, StringComparison.Ordinal); + Assert.Contains("requesturl=%2Fme%2Fmessages", requestUri.Query, StringComparison.Ordinal); + Assert.Contains("method=GET", requestUri.Query, StringComparison.Ordinal); + } + + [Fact] + public async Task Forbidden_ApplicationTokenMissingPermission_LogsFailedWithAdminConsentInstructions() + { + using var handler = new StubPermissionsHandler(UserPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/users/8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c", [("Authorization", BearerToken(roles: ["Mail.Read"]))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("User.ReadBasic.All", log.Message, StringComparison.Ordinal); + Assert.Contains("admin consent", log.Message, StringComparison.Ordinal); + Assert.Contains("scopeType=Application", Assert.Single(handler.Requests).Query, StringComparison.Ordinal); + } + + [Fact] + public async Task Forbidden_TokenHasSufficientPermission_LogsWarningAboutOtherCauses() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages", [("Authorization", BearerToken(scp: "mail.read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + Assert.Empty(factory.LogsOfType(MessageType.Failed)); + var log = Assert.Single(factory.LogsOfType(MessageType.Warning)); + Assert.Contains("Mail.Read", log.Message, StringComparison.Ordinal); + Assert.Contains("another reason", log.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Forbidden_NoToken_LogsWarningWithoutCallingApi() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages") + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Warning)); + Assert.Contains("access token", log.Message, StringComparison.Ordinal); + Assert.Empty(handler.Requests); + } + + [Fact] + public async Task Forbidden_PermissionsUnknown_LogsWarning() + { + using var handler = new StubPermissionsHandler(null, HttpStatusCode.NotFound); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/foo/bar", [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Warning)); + Assert.Contains("couldn't determine", log.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task NonForbiddenResponse_IsIgnored() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages", [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Unauthorized); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + Assert.Empty(factory.LogsOfType(MessageType.Failed)); + Assert.Empty(factory.LogsOfType(MessageType.Warning)); + Assert.Empty(handler.Requests); + } + + [Fact] + public async Task RepeatedForbiddenRequests_UseCachedPermissions() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + foreach (var id in new[] { "0b8a5d2e-1c3f-4e5a-9b7c-6d8e9f0a1b2c", "4f3e2d1c-0b9a-4876-a543-210fedcba987" }) + { + var exchange = TestExchange + .Request("GET", $"https://graph.microsoft.com/v1.0/me/messages/{id}", [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + } + + Assert.Equal(2, factory.LogsOfType(MessageType.Failed).Count); + _ = Assert.Single(handler.Requests); + } + + [Fact] + public async Task Batch_ForbiddenInnerResponse_LogsFailedForThatRequest() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var batchRequest = """ + { + "requests": [ + { "id": "1", "method": "GET", "url": "/me" }, + { "id": "2", "method": "GET", "url": "/me/messages" } + ] + } + """; + var batchResponse = """ + { + "responses": [ + { "id": "1", "status": 200, "body": {} }, + { "id": "2", "status": 403, "body": { "error": { "code": "ErrorAccessDenied", "message": "Access is denied." } } } + ] + } + """; + + var exchange = TestExchange + .Request("POST", "https://graph.microsoft.com/v1.0/$batch", [("Authorization", BearerToken(scp: "User.Read")), ("Content-Type", "application/json")], batchRequest) + .WithResponse(HttpStatusCode.OK, [("Content-Type", "application/json")], batchResponse); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("Batch request 2", log.Message, StringComparison.Ordinal); + Assert.Contains("Mail.ReadBasic", log.Message, StringComparison.Ordinal); + Assert.Contains("requesturl=%2Fme%2Fmessages", Assert.Single(handler.Requests).Query, StringComparison.Ordinal); + } + + private static (GraphMissingPermissionsGuidancePlugin Plugin, CapturingLoggerFactory Factory) CreatePlugin(HttpMessageHandler handler) + { + var factory = new CapturingLoggerFactory(); +#pragma warning disable CA2000 // HttpClient doesn't own the handler; the test disposes it + var httpClient = new HttpClient(handler, disposeHandler: false); +#pragma warning restore CA2000 + var plugin = new GraphMissingPermissionsGuidancePlugin( + httpClient, + new Logger(factory), + GraphWatch); + return (plugin, factory); + } + + private static string BearerToken(string? scp = null, string[]? roles = null) + { + var payload = new Dictionary { ["aud"] = "https://graph.microsoft.com" }; + if (scp is not null) + { + payload["scp"] = scp; + } + if (roles is not null) + { + payload["roles"] = roles; + } + + static string Encode(string value) => Convert.ToBase64String(Encoding.UTF8.GetBytes(value)) + .TrimEnd('=') + .Replace('+', '-') + .Replace('/', '_'); + + return $"Bearer {Encode("""{"alg":"none","typ":"JWT"}""")}.{Encode(JsonSerializer.Serialize(payload))}.signature"; + } + + private sealed class StubPermissionsHandler(string? responseJson, HttpStatusCode statusCode = HttpStatusCode.OK) : HttpMessageHandler + { + public List Requests { get; } = []; + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + Requests.Add(request.RequestUri!); + return Task.FromResult(new HttpResponseMessage(statusCode) + { + Content = new StringContent(responseJson ?? "{}", Encoding.UTF8, "application/json") + }); + } + } +} diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs new file mode 100644 index 000000000..eaf46dc5d --- /dev/null +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -0,0 +1,292 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using DevProxy.Abstractions.Models; +using DevProxy.Abstractions.Plugins; +using DevProxy.Abstractions.Proxy; +using DevProxy.Abstractions.Utils; +using DevProxy.Plugins.Models; +using DevProxy.Plugins.Utils; +using Microsoft.Extensions.Logging; +using System.Collections.Concurrent; +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; + +namespace DevProxy.Plugins.Guidance; + +public sealed class GraphMissingPermissionsGuidancePlugin( + HttpClient httpClient, + ILogger logger, + ISet urlsToWatch) : BasePlugin(logger, urlsToWatch) +{ + private static readonly TimeSpan PermissionsLookupTimeout = TimeSpan.FromSeconds(10); + +#pragma warning disable CA2213 // Disposable fields should be disposed - HttpClient is injected via DI + private readonly HttpClient _httpClient = httpClient; +#pragma warning restore CA2213 + private readonly ConcurrentDictionary _permissionsCache = new(StringComparer.OrdinalIgnoreCase); + + public override string Name => nameof(GraphMissingPermissionsGuidancePlugin); + + public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationToken cancellationToken) + { + Logger.LogTrace("{Method} called", nameof(AfterResponseAsync)); + + ArgumentNullException.ThrowIfNull(e); + + var loggingContext = new LoggingContext(e.ProxySession); + var request = e.ProxySession.Request; + var response = e.ProxySession.Response; + + if (!e.HasRequestUrlMatch(UrlsToWatch)) + { + Logger.LogRequest("URL not matched", MessageType.Skipped, loggingContext); + return; + } + if (string.Equals(request.Method, "OPTIONS", StringComparison.OrdinalIgnoreCase)) + { + Logger.LogRequest("Skipping OPTIONS request", MessageType.Skipped, loggingContext); + return; + } + if (!ProxyUtils.IsGraphRequest(request)) + { + Logger.LogRequest("Not a Microsoft Graph request", MessageType.Skipped, loggingContext); + return; + } + if (response is null) + { + Logger.LogRequest("No response", MessageType.Skipped, loggingContext); + return; + } + + var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader( + request.Headers.GetFirst("Authorization")?.Value); + + if (ProxyUtils.IsGraphBatchUrl(request.RequestUri)) + { + if (response.StatusCode != HttpStatusCode.OK) + { + Logger.LogRequest("Skipping non-successful batch response", MessageType.Skipped, loggingContext); + return; + } + + var failedRequests = GetForbiddenRequestsFromBatch(request.BodyString, response.BodyString); + if (failedRequests.Count == 0) + { + Logger.LogRequest("No forbidden requests in batch", MessageType.Skipped, loggingContext); + return; + } + + var graphVersion = ProxyUtils.IsGraphBetaUrl(request.RequestUri) ? "beta" : "v1.0"; + foreach (var failedRequest in failedRequests) + { + var relativeUrl = failedRequest.Url.StartsWith('/') ? failedRequest.Url : $"/{failedRequest.Url}"; + var absoluteUrl = $"https://{request.RequestUri.Host}/{graphVersion}{relativeUrl}"; + await AnalyzeForbiddenRequestAsync( + failedRequest.Method, + absoluteUrl, + $"Batch request {failedRequest.Id} ({failedRequest.Method} {failedRequest.Url})", + permissionsType, + tokenPermissions, + loggingContext, + cancellationToken); + } + } + else if (response.StatusCode == HttpStatusCode.Forbidden) + { + await AnalyzeForbiddenRequestAsync( + request.Method, + request.RequestUri.AbsoluteUri, + "Request", + permissionsType, + tokenPermissions, + loggingContext, + cancellationToken); + } + else + { + Logger.LogRequest("Skipping non-403 response", MessageType.Skipped, loggingContext); + } + + Logger.LogTrace("Left {Name}", nameof(AfterResponseAsync)); + } + + private async Task AnalyzeForbiddenRequestAsync( + string method, + string absoluteUrl, + string requestDescription, + GraphPermissionsType? permissionsType, + IEnumerable tokenPermissions, + LoggingContext loggingContext, + CancellationToken cancellationToken) + { + if (permissionsType is null) + { + Logger.LogRequest( + $"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't read permissions from the access token on the request so it can't tell if the request failed due to missing permissions. Make sure the request includes a valid bearer token (JWT) issued for Microsoft Graph.", + MessageType.Warning, + loggingContext); + return; + } + + string tokenizedUrl; + try + { + tokenizedUrl = GraphUtils.GetTokenizedUrl(absoluteUrl); + } + catch (Exception ex) + { + Logger.LogDebug(ex, "Failed to tokenize URL {Url}", absoluteUrl); + return; + } + + var requiredPermissions = await GetPermissionsAsync(method, tokenizedUrl, permissionsType.Value, cancellationToken); + if (requiredPermissions is null || requiredPermissions.Length == 0) + { + Logger.LogRequest( + $"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't determine which permissions are required for {method} {tokenizedUrl}. Check the Microsoft Graph API reference for the permissions required to call this API.", + MessageType.Warning, + loggingContext); + return; + } + + var tokenPermissionsSet = new HashSet(tokenPermissions, StringComparer.OrdinalIgnoreCase); + var grantingPermissions = requiredPermissions + .Select(p => p.Value) + .Where(tokenPermissionsSet.Contains) + .ToArray(); + + if (grantingPermissions.Length > 0) + { + Logger.LogRequest( + $"{requestDescription} failed with 403 Forbidden even though the access token has the {FormatList(grantingPermissions)} {GetPermissionsTypeString(permissionsType.Value)} permission(s) which allow calling {method} {tokenizedUrl}. The request likely failed for another reason, for example: the {(permissionsType == GraphPermissionsType.Delegated ? "signed-in user" : "app")} doesn't have access to the resource, a required license or administrative role is missing, or the resource is restricted by a policy. Check the error details in the response for more information.", + MessageType.Warning, + loggingContext); + return; + } + + var leastPrivileged = requiredPermissions + .Where(p => p.IsLeastPrivilege) + .Select(p => p.Value) + .ToArray(); + if (leastPrivileged.Length == 0) + { + leastPrivileged = [requiredPermissions[0].Value]; + } + var otherPermissions = requiredPermissions + .Select(p => p.Value) + .Except(leastPrivileged, StringComparer.OrdinalIgnoreCase) + .ToArray(); + + var tokenPermissionsString = tokenPermissionsSet.Count > 0 + ? FormatList(tokenPermissionsSet.Order(StringComparer.OrdinalIgnoreCase)) + : "none"; + + var message = $"{requestDescription} failed with 403 Forbidden because the access token doesn't have the permissions required to call {method} {tokenizedUrl}. " + + $"{CapitalizeFirst(GetPermissionsTypeString(permissionsType.Value))} permissions on the token: {tokenPermissionsString}. " + + $"Add the least privileged permission: {FormatList(leastPrivileged)}." + + (otherPermissions.Length > 0 ? $" Alternatively, use one of the following higher-privileged permissions: {FormatList(otherPermissions)}." : string.Empty) + + " " + GetInstructions(permissionsType.Value); + + Logger.LogRequest(message, MessageType.Failed, loggingContext); + } + + private async Task GetPermissionsAsync( + string method, + string tokenizedUrl, + GraphPermissionsType permissionsType, + CancellationToken cancellationToken) + { + var scopeType = GraphUtils.GetScopeTypeString(permissionsType); + var cacheKey = $"{scopeType}|{method}|{tokenizedUrl}"; + if (_permissionsCache.TryGetValue(cacheKey, out var cachedPermissions)) + { + Logger.LogDebug("Using cached permissions for {CacheKey}", cacheKey); + return cachedPermissions; + } + + var url = $"https://graph-devx-api.microsoft.com/permissions?scopeType={scopeType}&requesturl={Uri.EscapeDataString(tokenizedUrl)}&method={Uri.EscapeDataString(method.ToUpperInvariant())}"; + + try + { + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(PermissionsLookupTimeout); + + Logger.LogDebug("Retrieving permissions from {Url}", url); + using var response = await _httpClient.GetAsync(new Uri(url), cts.Token); + if (response.StatusCode == HttpStatusCode.NotFound) + { + Logger.LogDebug("No permissions found for {Method} {Url}", method, tokenizedUrl); + _permissionsCache[cacheKey] = []; + return []; + } + if (!response.IsSuccessStatusCode) + { + Logger.LogDebug("Failed to retrieve permissions for {Method} {Url}: {StatusCode}", method, tokenizedUrl, response.StatusCode); + return null; + } + + var permissions = (await response.Content.ReadFromJsonAsync(ProxyUtils.JsonSerializerOptions, cts.Token) ?? []) + .Where(p => !p.IsHidden && !string.IsNullOrEmpty(p.Value)) + .ToArray(); + _permissionsCache[cacheKey] = permissions; + return permissions; + } + catch (Exception ex) when ((ex is HttpRequestException or JsonException or OperationCanceledException) && !cancellationToken.IsCancellationRequested) + { + Logger.LogDebug(ex, "Failed to retrieve permissions for {Method} {Url}", method, tokenizedUrl); + return null; + } + } + + private List GetForbiddenRequestsFromBatch(string requestBody, string responseBody) + { + if (string.IsNullOrEmpty(requestBody) || string.IsNullOrEmpty(responseBody)) + { + return []; + } + + try + { + var batchRequest = JsonSerializer.Deserialize(requestBody, ProxyUtils.JsonSerializerOptions); + var batchResponse = JsonSerializer.Deserialize(responseBody, ProxyUtils.JsonSerializerOptions); + if (batchRequest is null || batchResponse is null) + { + return []; + } + + var forbiddenIds = batchResponse.Responses + .Where(r => r.Status == (int)HttpStatusCode.Forbidden) + .Select(r => r.Id) + .ToHashSet(StringComparer.Ordinal); + + return [.. batchRequest.Requests.Where(r => forbiddenIds.Contains(r.Id) && !string.IsNullOrEmpty(r.Url))]; + } + catch (JsonException ex) + { + Logger.LogDebug(ex, "Failed to parse batch request or response"); + return []; + } + } + + private static string GetInstructions(GraphPermissionsType permissionsType) => permissionsType switch + { + GraphPermissionsType.Delegated => "To fix it, add the permission to the delegated permissions of your app registration in Microsoft Entra ID, include it in the scopes your app requests when acquiring the access token, and make sure the user (or an administrator, for permissions that require admin consent) consents to it.", + GraphPermissionsType.Application => "To fix it, add the permission to the application permissions of your app registration in Microsoft Entra ID, grant admin consent for it, and acquire a new access token.", + _ => string.Empty + }; + + private static string GetPermissionsTypeString(GraphPermissionsType permissionsType) => permissionsType switch + { + GraphPermissionsType.Delegated => "delegated", + GraphPermissionsType.Application => "application", + _ => permissionsType.ToString() + }; + + private static string CapitalizeFirst(string value) => + string.IsNullOrEmpty(value) ? value : char.ToUpperInvariant(value[0]) + value[1..]; + + private static string FormatList(IEnumerable values) => string.Join(", ", values); +} diff --git a/DevProxy.Plugins/Reporting/GraphMinimalPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Reporting/GraphMinimalPermissionsGuidancePlugin.cs index e7e0c1333..57a15520c 100644 --- a/DevProxy.Plugins/Reporting/GraphMinimalPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Reporting/GraphMinimalPermissionsGuidancePlugin.cs @@ -10,7 +10,6 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; -using System.IdentityModel.Tokens.Jwt; using System.Net.Http.Json; using System.Text.Json; @@ -312,48 +311,7 @@ private async Task EvaluateMinimalScopesAsync( private static (GraphPermissionsType type, IEnumerable permissions) GetPermissionsAndType(RequestLog request) { var authHeader = request.Context?.Session.Request.Headers.GetFirst("Authorization"); - if (authHeader == null) - { - return (GraphPermissionsType.Application, []); - } - - var token = authHeader.Value.Replace("Bearer ", string.Empty, StringComparison.OrdinalIgnoreCase); - var tokenChunks = token.Split('.'); - if (tokenChunks.Length != 3) - { - return (GraphPermissionsType.Application, []); - } - - try - { - var handler = new JwtSecurityTokenHandler(); - var jwtSecurityToken = handler.ReadJwtToken(token); - - var scopeClaim = jwtSecurityToken.Claims.FirstOrDefault(c => c.Type == "scp"); - if (scopeClaim == null) - { - // possibly an application token - // roles is an array so we need to handle it differently - var roles = jwtSecurityToken.Claims - .Where(c => c.Type == "roles") - .Select(c => c.Value); - if (!roles.Any()) - { - return (GraphPermissionsType.Application, []); - } - else - { - return (GraphPermissionsType.Application, roles); - } - } - else - { - return (GraphPermissionsType.Delegated, scopeClaim.Value.Split(' ')); - } - } - catch - { - return (GraphPermissionsType.Application, []); - } + var (type, permissions) = GraphUtils.GetPermissionsFromAuthorizationHeader(authHeader?.Value); + return (type ?? GraphPermissionsType.Application, permissions); } } diff --git a/DevProxy.Plugins/Utils/GraphUtils.cs b/DevProxy.Plugins/Utils/GraphUtils.cs index d06547c83..d95de3385 100644 --- a/DevProxy.Plugins/Utils/GraphUtils.cs +++ b/DevProxy.Plugins/Utils/GraphUtils.cs @@ -7,6 +7,7 @@ using DevProxy.Abstractions.Utils; using DevProxy.Plugins.Models; using Microsoft.Extensions.Logging; +using System.IdentityModel.Tokens.Jwt; using System.Net.Http.Json; using System.Text.Json; using System.Text.RegularExpressions; @@ -118,6 +119,47 @@ internal async Task> UpdateUserScopesAsync(IEnumerable + /// Returns permissions and type (delegated or application) from the bearer token + /// in the specified Authorization header value. Returns null type and no permissions + /// if the header is missing or the token can't be read as a JWT. + /// + internal static (GraphPermissionsType? Type, IEnumerable Permissions) GetPermissionsFromAuthorizationHeader(string? authorizationHeader) + { + if (string.IsNullOrEmpty(authorizationHeader)) + { + return (null, []); + } + + var token = authorizationHeader.Replace("Bearer ", string.Empty, StringComparison.OrdinalIgnoreCase).Trim(); + if (token.Split('.').Length != 3) + { + return (null, []); + } + + try + { + var jwtSecurityToken = new JwtSecurityTokenHandler().ReadJwtToken(token); + + var scopeClaim = jwtSecurityToken.Claims.FirstOrDefault(c => c.Type == "scp"); + if (scopeClaim is not null) + { + return (GraphPermissionsType.Delegated, scopeClaim.Value.Split(' ', StringSplitOptions.RemoveEmptyEntries)); + } + + // possibly an application token; roles is an array + var roles = jwtSecurityToken.Claims + .Where(c => c.Type == "roles") + .Select(c => c.Value) + .ToArray(); + return (GraphPermissionsType.Application, roles); + } + catch + { + return (null, []); + } + } + internal static string GetTokenizedUrl(string absoluteUrl) { absoluteUrl = CanonicalizeDriveItemPathUrl(absoluteUrl); diff --git a/DevProxy/config/m365.json b/DevProxy/config/m365.json index 3f96a2228..97567ddbc 100644 --- a/DevProxy/config/m365.json +++ b/DevProxy/config/m365.json @@ -105,6 +105,21 @@ "https://microsoftgraph.chinacloudapi.cn/beta/*" ] }, + { + "name": "GraphMissingPermissionsGuidancePlugin", + "enabled": true, + "pluginPath": "~appFolder/plugins/DevProxy.Plugins.dll", + "urlsToWatch": [ + "https://graph.microsoft.com/v1.0/*", + "https://graph.microsoft.com/beta/*", + "https://graph.microsoft.us/v1.0/*", + "https://graph.microsoft.us/beta/*", + "https://dod-graph.microsoft.us/v1.0/*", + "https://dod-graph.microsoft.us/beta/*", + "https://microsoftgraph.chinacloudapi.cn/v1.0/*", + "https://microsoftgraph.chinacloudapi.cn/beta/*" + ] + }, { "name": "CachingGuidancePlugin", "enabled": true, diff --git a/DevProxy/config/microsoft-graph.json b/DevProxy/config/microsoft-graph.json index ccad5b7c3..3eabf8d42 100644 --- a/DevProxy/config/microsoft-graph.json +++ b/DevProxy/config/microsoft-graph.json @@ -43,6 +43,11 @@ "enabled": true, "pluginPath": "~appFolder/plugins/DevProxy.Plugins.dll" }, + { + "name": "GraphMissingPermissionsGuidancePlugin", + "enabled": true, + "pluginPath": "~appFolder/plugins/DevProxy.Plugins.dll" + }, { "name": "GraphRandomErrorPlugin", "enabled": true, diff --git a/skills/dev-proxy/references/configuration.md b/skills/dev-proxy/references/configuration.md index 4eea14fd6..e74d103b8 100644 --- a/skills/dev-proxy/references/configuration.md +++ b/skills/dev-proxy/references/configuration.md @@ -289,6 +289,7 @@ Or filter by request headers: |--------|-------------| | `GraphBetaSupportGuidancePlugin` | Warns when beta endpoints are used | | `GraphClientRequestIdGuidancePlugin` | Recommends client-request-id header | +| `GraphMissingPermissionsGuidancePlugin` | Explains which permissions are missing when Graph returns 403 Forbidden | | `GraphConnectorGuidancePlugin` | Graph connector guidance | | `GraphSdkGuidancePlugin` | Recommends official SDKs | | `GraphSelectGuidancePlugin` | Warns when $select is missing | diff --git a/skills/dev-proxy/references/plugin-catalog.md b/skills/dev-proxy/references/plugin-catalog.md index 412a46806..b79d1e64f 100644 --- a/skills/dev-proxy/references/plugin-catalog.md +++ b/skills/dev-proxy/references/plugin-catalog.md @@ -262,6 +262,7 @@ Same Azure config properties as above. |--------|-------------| | `GraphBetaSupportGuidancePlugin` | Warns when beta endpoints are used | | `GraphClientRequestIdGuidancePlugin` | Recommends client-request-id header | +| `GraphMissingPermissionsGuidancePlugin` | Explains which permissions are missing when Graph returns 403 Forbidden | | `GraphConnectorGuidancePlugin` | Graph connector guidance | | `GraphSdkGuidancePlugin` | Recommends official SDKs | | `GraphSelectGuidancePlugin` | Warns when $select is missing | From 84abe2e982cbd2b6d512cb25d82d24f33ca2bf66 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 12:29:09 +0100 Subject: [PATCH 02/10] Address review feedback on GraphMissingPermissionsGuidancePlugin - Skip responses generated by plugins (expose RespondedByPlugin on IProxySession) - Recognize dod-graph.microsoft.us in ProxyUtils.IsGraphUrl - Lower DevX lookup timeout to 3s and analyze batch requests in parallel Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Proxy/Http/IProxySession.cs | 6 ++ DevProxy.Abstractions/Utils/ProxyUtils.cs | 1 + ...phMissingPermissionsGuidancePluginTests.cs | 36 ++++++++++++ .../GraphMissingPermissionsGuidancePlugin.cs | 57 +++++++++++-------- .../Http/CanonicalProxySession.cs | 10 ++-- 5 files changed, 79 insertions(+), 31 deletions(-) diff --git a/DevProxy.Abstractions/Proxy/Http/IProxySession.cs b/DevProxy.Abstractions/Proxy/Http/IProxySession.cs index 9bec624dd..cbe125fb3 100644 --- a/DevProxy.Abstractions/Proxy/Http/IProxySession.cs +++ b/DevProxy.Abstractions/Proxy/Http/IProxySession.cs @@ -49,6 +49,12 @@ public interface IProxySession /// bool HasResponse { get; } + /// + /// True when the current was produced by a plugin (for + /// example a mock) rather than received from the origin. + /// + bool RespondedByPlugin { get; } + /// /// Produces a mocked text response and short-circuits the exchange: no /// request is sent upstream and becomes non-null. This diff --git a/DevProxy.Abstractions/Utils/ProxyUtils.cs b/DevProxy.Abstractions/Utils/ProxyUtils.cs index a4b1ed1bd..68a389bbb 100644 --- a/DevProxy.Abstractions/Utils/ProxyUtils.cs +++ b/DevProxy.Abstractions/Utils/ProxyUtils.cs @@ -116,6 +116,7 @@ public static bool IsGraphUrl(Uri uri) ArgumentNullException.ThrowIfNull(uri); return uri.Host.StartsWith("graph.microsoft.", StringComparison.OrdinalIgnoreCase) || + uri.Host.StartsWith("dod-graph.microsoft.", StringComparison.OrdinalIgnoreCase) || uri.Host.StartsWith("microsoftgraph.", StringComparison.OrdinalIgnoreCase); } diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index 4efdc6286..4e138b866 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -192,6 +192,42 @@ public async Task Batch_ForbiddenInnerResponse_LogsFailedForThatRequest() Assert.Contains("requesturl=%2Fme%2Fmessages", Assert.Single(handler.Requests).Query, StringComparison.Ordinal); } + [Fact] + public async Task MockedForbiddenResponse_IsIgnored() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages", [("Authorization", BearerToken(scp: "User.Read"))]); + exchange.Session.Respond("{}", HttpStatusCode.Forbidden, []); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + Assert.Empty(factory.LogsOfType(MessageType.Failed)); + Assert.Empty(factory.LogsOfType(MessageType.Warning)); + Assert.Empty(handler.Requests); + } + + [Fact] + public async Task Forbidden_DodGraphHost_LogsFailed() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var factory = new CapturingLoggerFactory(); +#pragma warning disable CA2000 // HttpClient doesn't own the handler; the test disposes it + var plugin = new GraphMissingPermissionsGuidancePlugin( + new HttpClient(handler, disposeHandler: false), + new Logger(factory), + KestrelProxyHarness.BuildUrlsToWatch("dod-graph.microsoft.us")); +#pragma warning restore CA2000 + + var exchange = TestExchange + .Request("GET", "https://dod-graph.microsoft.us/v1.0/me/messages", [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + _ = Assert.Single(factory.LogsOfType(MessageType.Failed)); + } + private static (GraphMissingPermissionsGuidancePlugin Plugin, CapturingLoggerFactory Factory) CreatePlugin(HttpMessageHandler handler) { var factory = new CapturingLoggerFactory(); diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index eaf46dc5d..d5ebce378 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -21,7 +21,7 @@ public sealed class GraphMissingPermissionsGuidancePlugin( ILogger logger, ISet urlsToWatch) : BasePlugin(logger, urlsToWatch) { - private static readonly TimeSpan PermissionsLookupTimeout = TimeSpan.FromSeconds(10); + private static readonly TimeSpan PermissionsLookupTimeout = TimeSpan.FromSeconds(3); #pragma warning disable CA2213 // Disposable fields should be disposed - HttpClient is injected via DI private readonly HttpClient _httpClient = httpClient; @@ -60,6 +60,11 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT Logger.LogRequest("No response", MessageType.Skipped, loggingContext); return; } + if (e.ProxySession.RespondedByPlugin) + { + Logger.LogRequest("Skipping response generated by a plugin", MessageType.Skipped, loggingContext); + return; + } var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader( request.Headers.GetFirst("Authorization")?.Value); @@ -80,30 +85,34 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT } var graphVersion = ProxyUtils.IsGraphBetaUrl(request.RequestUri) ? "beta" : "v1.0"; - foreach (var failedRequest in failedRequests) + // analyze in parallel to limit the delay before the response is sent to the client + var guidance = await Task.WhenAll(failedRequests.Select(failedRequest => { var relativeUrl = failedRequest.Url.StartsWith('/') ? failedRequest.Url : $"/{failedRequest.Url}"; var absoluteUrl = $"https://{request.RequestUri.Host}/{graphVersion}{relativeUrl}"; - await AnalyzeForbiddenRequestAsync( + return AnalyzeForbiddenRequestAsync( failedRequest.Method, absoluteUrl, $"Batch request {failedRequest.Id} ({failedRequest.Method} {failedRequest.Url})", permissionsType, tokenPermissions, - loggingContext, cancellationToken); + })); + foreach (var (message, messageType) in guidance) + { + LogGuidance(message, messageType, loggingContext); } } else if (response.StatusCode == HttpStatusCode.Forbidden) { - await AnalyzeForbiddenRequestAsync( + var (message, messageType) = await AnalyzeForbiddenRequestAsync( request.Method, request.RequestUri.AbsoluteUri, "Request", permissionsType, tokenPermissions, - loggingContext, cancellationToken); + LogGuidance(message, messageType, loggingContext); } else { @@ -113,22 +122,26 @@ await AnalyzeForbiddenRequestAsync( Logger.LogTrace("Left {Name}", nameof(AfterResponseAsync)); } - private async Task AnalyzeForbiddenRequestAsync( + private void LogGuidance(string? message, MessageType messageType, LoggingContext loggingContext) + { + if (message is not null) + { + Logger.LogRequest(message, messageType, loggingContext); + } + } + + private async Task<(string? Message, MessageType Type)> AnalyzeForbiddenRequestAsync( string method, string absoluteUrl, string requestDescription, GraphPermissionsType? permissionsType, IEnumerable tokenPermissions, - LoggingContext loggingContext, CancellationToken cancellationToken) { if (permissionsType is null) { - Logger.LogRequest( - $"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't read permissions from the access token on the request so it can't tell if the request failed due to missing permissions. Make sure the request includes a valid bearer token (JWT) issued for Microsoft Graph.", - MessageType.Warning, - loggingContext); - return; + return ($"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't read permissions from the access token on the request so it can't tell if the request failed due to missing permissions. Make sure the request includes a valid bearer token (JWT) issued for Microsoft Graph.", + MessageType.Warning); } string tokenizedUrl; @@ -139,17 +152,14 @@ private async Task AnalyzeForbiddenRequestAsync( catch (Exception ex) { Logger.LogDebug(ex, "Failed to tokenize URL {Url}", absoluteUrl); - return; + return (null, MessageType.Skipped); } var requiredPermissions = await GetPermissionsAsync(method, tokenizedUrl, permissionsType.Value, cancellationToken); if (requiredPermissions is null || requiredPermissions.Length == 0) { - Logger.LogRequest( - $"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't determine which permissions are required for {method} {tokenizedUrl}. Check the Microsoft Graph API reference for the permissions required to call this API.", - MessageType.Warning, - loggingContext); - return; + return ($"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't determine which permissions are required for {method} {tokenizedUrl}. Check the Microsoft Graph API reference for the permissions required to call this API.", + MessageType.Warning); } var tokenPermissionsSet = new HashSet(tokenPermissions, StringComparer.OrdinalIgnoreCase); @@ -160,11 +170,8 @@ private async Task AnalyzeForbiddenRequestAsync( if (grantingPermissions.Length > 0) { - Logger.LogRequest( - $"{requestDescription} failed with 403 Forbidden even though the access token has the {FormatList(grantingPermissions)} {GetPermissionsTypeString(permissionsType.Value)} permission(s) which allow calling {method} {tokenizedUrl}. The request likely failed for another reason, for example: the {(permissionsType == GraphPermissionsType.Delegated ? "signed-in user" : "app")} doesn't have access to the resource, a required license or administrative role is missing, or the resource is restricted by a policy. Check the error details in the response for more information.", - MessageType.Warning, - loggingContext); - return; + return ($"{requestDescription} failed with 403 Forbidden even though the access token has the {FormatList(grantingPermissions)} {GetPermissionsTypeString(permissionsType.Value)} permission(s) which allow calling {method} {tokenizedUrl}. The request likely failed for another reason, for example: the {(permissionsType == GraphPermissionsType.Delegated ? "signed-in user" : "app")} doesn't have access to the resource, a required license or administrative role is missing, or the resource is restricted by a policy. Check the error details in the response for more information.", + MessageType.Warning); } var leastPrivileged = requiredPermissions @@ -190,7 +197,7 @@ private async Task AnalyzeForbiddenRequestAsync( (otherPermissions.Length > 0 ? $" Alternatively, use one of the following higher-privileged permissions: {FormatList(otherPermissions)}." : string.Empty) + " " + GetInstructions(permissionsType.Value); - Logger.LogRequest(message, MessageType.Failed, loggingContext); + return (message, MessageType.Failed); } private async Task GetPermissionsAsync( diff --git a/DevProxy.Proxy.Kestrel/Http/CanonicalProxySession.cs b/DevProxy.Proxy.Kestrel/Http/CanonicalProxySession.cs index 4aa89ac59..d7fe7a0be 100644 --- a/DevProxy.Proxy.Kestrel/Http/CanonicalProxySession.cs +++ b/DevProxy.Proxy.Kestrel/Http/CanonicalProxySession.cs @@ -61,12 +61,10 @@ public CanonicalProxySession(string sessionId, MutableHttpRequest request, int? /// public bool HasResponse => _response is not null; - /// - /// True when the current was produced by a plugin (via - /// ) - /// rather than received from the origin. The engine uses this to short-circuit - /// upstream forwarding. - /// + /// + /// + /// The engine uses this to short-circuit upstream forwarding. + /// public bool RespondedByPlugin { get; private set; } /// The concrete response, for engine write-back. Null until set. From d12a6135a743b17a0d50a0f48b8a1a5b74762a4c Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 12:36:11 +0100 Subject: [PATCH 03/10] Coalesce concurrent Graph permission lookups Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 80 +++++++++++++++++-- .../GraphMissingPermissionsGuidancePlugin.cs | 35 +++++++- 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index 4e138b866..40c532c01 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -228,6 +228,49 @@ public async Task Forbidden_DodGraphHost_LogsFailed() _ = Assert.Single(factory.LogsOfType(MessageType.Failed)); } + [Fact] + public async Task Batch_ConcurrentForbiddenRequestsWithSameKey_ShareSingleLookup() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson) { Delay = TimeSpan.FromMilliseconds(200) }; + var (plugin, factory) = CreatePlugin(handler); + + var ids = new[] { "0b8a5d2e-1c3f-4e5a-9b7c-6d8e9f0a1b2c", "4f3e2d1c-0b9a-4876-a543-210fedcba987", "9a8b7c6d-5e4f-4a3b-8c2d-1e0f9a8b7c6d" }; + var batchRequest = JsonSerializer.Serialize(new + { + requests = ids.Select((id, i) => new { id = $"{i + 1}", method = "GET", url = $"/me/messages/{id}" }) + }); + var batchResponse = JsonSerializer.Serialize(new + { + responses = ids.Select((_, i) => new { id = $"{i + 1}", status = 403, body = new { } }) + }); + + var exchange = TestExchange + .Request("POST", "https://graph.microsoft.com/v1.0/$batch", [("Authorization", BearerToken(scp: "User.Read")), ("Content-Type", "application/json")], batchRequest) + .WithResponse(HttpStatusCode.OK, [("Content-Type", "application/json")], batchResponse); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + Assert.Equal(3, factory.LogsOfType(MessageType.Failed).Count); + _ = Assert.Single(handler.Requests); + } + + [Fact] + public async Task FailedLookup_IsRetriedOnNextRequest() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson) { FailuresBeforeSuccess = 1 }; + var (plugin, factory) = CreatePlugin(handler); + + for (var i = 0; i < 2; i++) + { + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages", [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + } + + _ = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Equal(2, handler.Requests.Count); + } + private static (GraphMissingPermissionsGuidancePlugin Plugin, CapturingLoggerFactory Factory) CreatePlugin(HttpMessageHandler handler) { var factory = new CapturingLoggerFactory(); @@ -263,15 +306,42 @@ static string Encode(string value) => Convert.ToBase64String(Encoding.UTF8.GetBy private sealed class StubPermissionsHandler(string? responseJson, HttpStatusCode statusCode = HttpStatusCode.OK) : HttpMessageHandler { - public List Requests { get; } = []; + private readonly Lock _lock = new(); + private readonly List _requests = []; + + public IReadOnlyList Requests + { + get + { + lock (_lock) + { + return [.. _requests]; + } + } + } + + public TimeSpan Delay { get; init; } = TimeSpan.Zero; - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + public int FailuresBeforeSuccess { get; init; } + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - Requests.Add(request.RequestUri!); - return Task.FromResult(new HttpResponseMessage(statusCode) + int requestCount; + lock (_lock) + { + _requests.Add(request.RequestUri!); + requestCount = _requests.Count; + } + + if (Delay > TimeSpan.Zero) + { + await Task.Delay(Delay, cancellationToken); + } + + return new HttpResponseMessage(requestCount <= FailuresBeforeSuccess ? HttpStatusCode.InternalServerError : statusCode) { Content = new StringContent(responseJson ?? "{}", Encoding.UTF8, "application/json") - }); + }; } } } diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index d5ebce378..d3fcc69bd 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -27,6 +27,7 @@ public sealed class GraphMissingPermissionsGuidancePlugin( private readonly HttpClient _httpClient = httpClient; #pragma warning restore CA2213 private readonly ConcurrentDictionary _permissionsCache = new(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary>> _pendingLookups = new(StringComparer.OrdinalIgnoreCase); public override string Name => nameof(GraphMissingPermissionsGuidancePlugin); @@ -214,12 +215,40 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex return cachedPermissions; } + // coalesce concurrent lookups for the same key; the shared lookup isn't + // tied to any caller's cancellation token so that one cancelled caller + // doesn't fail the others + Lazy>? newLookup = null; + newLookup = new(LookupAsync); + var lookup = _pendingLookups.GetOrAdd(cacheKey, newLookup); + return await lookup.Value.WaitAsync(cancellationToken); + + async Task LookupAsync() + { + try + { + return await FetchPermissionsAsync(cacheKey, method, tokenizedUrl, scopeType); + } + finally + { + // successful results are in the cache; failed lookups are + // removed so that they can be retried + _ = _pendingLookups.TryRemove(new KeyValuePair>>(cacheKey, newLookup!)); + } + } + } + + private async Task FetchPermissionsAsync( + string cacheKey, + string method, + string tokenizedUrl, + string scopeType) + { var url = $"https://graph-devx-api.microsoft.com/permissions?scopeType={scopeType}&requesturl={Uri.EscapeDataString(tokenizedUrl)}&method={Uri.EscapeDataString(method.ToUpperInvariant())}"; try { - using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(PermissionsLookupTimeout); + using var cts = new CancellationTokenSource(PermissionsLookupTimeout); Logger.LogDebug("Retrieving permissions from {Url}", url); using var response = await _httpClient.GetAsync(new Uri(url), cts.Token); @@ -241,7 +270,7 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex _permissionsCache[cacheKey] = permissions; return permissions; } - catch (Exception ex) when ((ex is HttpRequestException or JsonException or OperationCanceledException) && !cancellationToken.IsCancellationRequested) + catch (Exception ex) when (ex is HttpRequestException or JsonException or OperationCanceledException) { Logger.LogDebug(ex, "Failed to retrieve permissions for {Method} {Url}", method, tokenizedUrl); return null; From c5317ce8ce70342bd9f7b2f8eb231f7e0c6a4ebb Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 13:03:50 +0100 Subject: [PATCH 04/10] Improve Graph missing permissions recommendations Treat self-only delegated permissions (User.Read, User.ReadWrite) as insufficient for /users/{id} requests and list permissions as unranked alternatives when none is marked least privileged. Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 45 +++++++++++++++++++ .../GraphMissingPermissionsGuidancePlugin.cs | 32 ++++++++++--- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index 40c532c01..cfa2662e9 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -271,6 +271,51 @@ public async Task FailedLookup_IsRetriedOnNextRequest() Assert.Equal(2, handler.Requests.Count); } + [Fact] + public async Task Forbidden_DelegatedSelfOnlyPermissionForOtherUser_LogsFailed() + { + using var handler = new StubPermissionsHandler(""" + [ + { "value": "User.Read", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, + { "value": "User.Read.All", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, + { "value": "User.ReadBasic.All", "scopeType": "DelegatedWork", "isLeastPrivilege": true, "isHidden": false } + ] + """); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/users/8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c", [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + Assert.Empty(factory.LogsOfType(MessageType.Warning)); + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("least privileged permission: User.ReadBasic.All", log.Message, StringComparison.Ordinal); + Assert.Contains("higher-privileged permissions: User.Read.All.", log.Message, StringComparison.Ordinal); + Assert.Contains("only allow access to the signed-in user", log.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Forbidden_NoLeastPrivilegedPermissionMarked_ListsUnrankedAlternatives() + { + using var handler = new StubPermissionsHandler(""" + [ + { "value": "User.Read.All", "scopeType": "Application", "isLeastPrivilege": false, "isHidden": false }, + { "value": "User.ReadBasic.All", "scopeType": "Application", "isLeastPrivilege": false, "isHidden": false } + ] + """); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/users", [("Authorization", BearerToken(roles: ["Mail.Read"]))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("Add any of the following permissions: User.Read.All, User.ReadBasic.All.", log.Message, StringComparison.Ordinal); + Assert.DoesNotContain("least privileged", log.Message, StringComparison.Ordinal); + } + private static (GraphMissingPermissionsGuidancePlugin Plugin, CapturingLoggerFactory Factory) CreatePlugin(HttpMessageHandler handler) { var factory = new CapturingLoggerFactory(); diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index d3fcc69bd..6a5461894 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -22,6 +22,7 @@ public sealed class GraphMissingPermissionsGuidancePlugin( ISet urlsToWatch) : BasePlugin(logger, urlsToWatch) { private static readonly TimeSpan PermissionsLookupTimeout = TimeSpan.FromSeconds(3); + private static readonly HashSet SelfOnlyDelegatedPermissions = new(["User.Read", "User.ReadWrite"], StringComparer.OrdinalIgnoreCase); #pragma warning disable CA2213 // Disposable fields should be disposed - HttpClient is injected via DI private readonly HttpClient _httpClient = httpClient; @@ -163,6 +164,26 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex MessageType.Warning); } + // delegated self-only permissions, like User.Read, are listed for + // /users/{id} but only grant access to the signed-in user + var selfOnlyNote = string.Empty; + if (permissionsType == GraphPermissionsType.Delegated && + tokenizedUrl.StartsWith("/users/", StringComparison.OrdinalIgnoreCase)) + { + var selfOnlyPermissions = requiredPermissions + .Where(p => SelfOnlyDelegatedPermissions.Contains(p.Value)) + .Select(p => p.Value) + .ToArray(); + var otherUserPermissions = requiredPermissions + .Where(p => !SelfOnlyDelegatedPermissions.Contains(p.Value)) + .ToArray(); + if (selfOnlyPermissions.Length > 0 && otherUserPermissions.Length > 0) + { + requiredPermissions = otherUserPermissions; + selfOnlyNote = $" The {FormatList(selfOnlyPermissions)} permission(s) only allow access to the signed-in user. If you're calling the API for the signed-in user, use /me instead."; + } + } + var tokenPermissionsSet = new HashSet(tokenPermissions, StringComparer.OrdinalIgnoreCase); var grantingPermissions = requiredPermissions .Select(p => p.Value) @@ -179,10 +200,6 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex .Where(p => p.IsLeastPrivilege) .Select(p => p.Value) .ToArray(); - if (leastPrivileged.Length == 0) - { - leastPrivileged = [requiredPermissions[0].Value]; - } var otherPermissions = requiredPermissions .Select(p => p.Value) .Except(leastPrivileged, StringComparer.OrdinalIgnoreCase) @@ -194,8 +211,11 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex var message = $"{requestDescription} failed with 403 Forbidden because the access token doesn't have the permissions required to call {method} {tokenizedUrl}. " + $"{CapitalizeFirst(GetPermissionsTypeString(permissionsType.Value))} permissions on the token: {tokenPermissionsString}. " + - $"Add the least privileged permission: {FormatList(leastPrivileged)}." + - (otherPermissions.Length > 0 ? $" Alternatively, use one of the following higher-privileged permissions: {FormatList(otherPermissions)}." : string.Empty) + + (leastPrivileged.Length > 0 + ? $"Add the least privileged permission: {FormatList(leastPrivileged)}." + + (otherPermissions.Length > 0 ? $" Alternatively, use one of the following higher-privileged permissions: {FormatList(otherPermissions)}." : string.Empty) + : $"Add any of the following permissions: {FormatList(otherPermissions)}.") + + selfOnlyNote + " " + GetInstructions(permissionsType.Value); return (message, MessageType.Failed); From 760813798cd11fca62143524997cefc48d61bc01 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 13:11:17 +0100 Subject: [PATCH 05/10] Keep self-only permissions for requests targeting the signed-in user Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 42 +++++++++++++++---- .../GraphMissingPermissionsGuidancePlugin.cs | 24 +++++++++-- DevProxy.Plugins/Utils/GraphUtils.cs | 26 ++++++++++++ 3 files changed, 80 insertions(+), 12 deletions(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index cfa2662e9..1331bfe3b 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -34,6 +34,14 @@ public sealed class GraphMissingPermissionsGuidancePluginTests ] """; + private const string SelfOnlyUserPermissionsJson = """ + [ + { "value": "User.Read", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, + { "value": "User.Read.All", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, + { "value": "User.ReadBasic.All", "scopeType": "DelegatedWork", "isLeastPrivilege": true, "isHidden": false } + ] + """; + private static ISet GraphWatch => KestrelProxyHarness.BuildUrlsToWatch("graph.microsoft.com"); [Fact] @@ -274,17 +282,11 @@ public async Task FailedLookup_IsRetriedOnNextRequest() [Fact] public async Task Forbidden_DelegatedSelfOnlyPermissionForOtherUser_LogsFailed() { - using var handler = new StubPermissionsHandler(""" - [ - { "value": "User.Read", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, - { "value": "User.Read.All", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false }, - { "value": "User.ReadBasic.All", "scopeType": "DelegatedWork", "isLeastPrivilege": true, "isHidden": false } - ] - """); + using var handler = new StubPermissionsHandler(SelfOnlyUserPermissionsJson); var (plugin, factory) = CreatePlugin(handler); var exchange = TestExchange - .Request("GET", "https://graph.microsoft.com/v1.0/users/8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c", [("Authorization", BearerToken(scp: "User.Read"))]) + .Request("GET", "https://graph.microsoft.com/v1.0/users/8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c", [("Authorization", BearerToken(scp: "User.Read", oid: "1d2c3b4a-5e6f-4a7b-8c9d-0e1f2a3b4c5d"))]) .WithResponse(HttpStatusCode.Forbidden); await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); @@ -295,6 +297,24 @@ public async Task Forbidden_DelegatedSelfOnlyPermissionForOtherUser_LogsFailed() Assert.Contains("only allow access to the signed-in user", log.Message, StringComparison.Ordinal); } + [Fact] + public async Task Forbidden_DelegatedSelfOnlyPermissionForSignedInUser_LogsWarningAboutOtherCauses() + { + using var handler = new StubPermissionsHandler(SelfOnlyUserPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + const string signedInUserId = "8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c"; + var exchange = TestExchange + .Request("GET", $"https://graph.microsoft.com/v1.0/users/{signedInUserId}", [("Authorization", BearerToken(scp: "User.Read", oid: signedInUserId))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + Assert.Empty(factory.LogsOfType(MessageType.Failed)); + var log = Assert.Single(factory.LogsOfType(MessageType.Warning)); + Assert.Contains("User.Read", log.Message, StringComparison.Ordinal); + Assert.Contains("another reason", log.Message, StringComparison.Ordinal); + } + [Fact] public async Task Forbidden_NoLeastPrivilegedPermissionMarked_ListsUnrankedAlternatives() { @@ -329,9 +349,13 @@ private static (GraphMissingPermissionsGuidancePlugin Plugin, CapturingLoggerFac return (plugin, factory); } - private static string BearerToken(string? scp = null, string[]? roles = null) + private static string BearerToken(string? scp = null, string[]? roles = null, string? oid = null) { var payload = new Dictionary { ["aud"] = "https://graph.microsoft.com" }; + if (oid is not null) + { + payload["oid"] = oid; + } if (scp is not null) { payload["scp"] = scp; diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index 6a5461894..a966279ca 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -68,8 +68,9 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT return; } - var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader( - request.Headers.GetFirst("Authorization")?.Value); + var authorizationHeader = request.Headers.GetFirst("Authorization")?.Value; + var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader(authorizationHeader); + var userIdentifiers = GraphUtils.GetUserIdentifiersFromAuthorizationHeader(authorizationHeader); if (ProxyUtils.IsGraphBatchUrl(request.RequestUri)) { @@ -98,6 +99,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT $"Batch request {failedRequest.Id} ({failedRequest.Method} {failedRequest.Url})", permissionsType, tokenPermissions, + userIdentifiers, cancellationToken); })); foreach (var (message, messageType) in guidance) @@ -113,6 +115,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT "Request", permissionsType, tokenPermissions, + userIdentifiers, cancellationToken); LogGuidance(message, messageType, loggingContext); } @@ -138,6 +141,7 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex string requestDescription, GraphPermissionsType? permissionsType, IEnumerable tokenPermissions, + string[] userIdentifiers, CancellationToken cancellationToken) { if (permissionsType is null) @@ -168,7 +172,8 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex // /users/{id} but only grant access to the signed-in user var selfOnlyNote = string.Empty; if (permissionsType == GraphPermissionsType.Delegated && - tokenizedUrl.StartsWith("/users/", StringComparison.OrdinalIgnoreCase)) + tokenizedUrl.StartsWith("/users/", StringComparison.OrdinalIgnoreCase) && + !IsSignedInUser(absoluteUrl, userIdentifiers)) { var selfOnlyPermissions = requiredPermissions .Where(p => SelfOnlyDelegatedPermissions.Contains(p.Value)) @@ -327,6 +332,19 @@ private List GetForbiddenRequestsFromBatch(stri } } + private static bool IsSignedInUser(string absoluteUrl, string[] userIdentifiers) + { + // segments: "/", "{version}/", "users/", "{id}/", ... + var segments = new Uri(absoluteUrl).Segments; + if (userIdentifiers.Length == 0 || segments.Length < 4) + { + return false; + } + + var userId = Uri.UnescapeDataString(segments[3].TrimEnd('/')); + return userIdentifiers.Contains(userId, StringComparer.OrdinalIgnoreCase); + } + private static string GetInstructions(GraphPermissionsType permissionsType) => permissionsType switch { GraphPermissionsType.Delegated => "To fix it, add the permission to the delegated permissions of your app registration in Microsoft Entra ID, include it in the scopes your app requests when acquiring the access token, and make sure the user (or an administrator, for permissions that require admin consent) consents to it.", diff --git a/DevProxy.Plugins/Utils/GraphUtils.cs b/DevProxy.Plugins/Utils/GraphUtils.cs index d95de3385..99d20a8f4 100644 --- a/DevProxy.Plugins/Utils/GraphUtils.cs +++ b/DevProxy.Plugins/Utils/GraphUtils.cs @@ -160,6 +160,32 @@ internal static (GraphPermissionsType? Type, IEnumerable Permissions) Ge } } + internal static string[] GetUserIdentifiersFromAuthorizationHeader(string? authorizationHeader) + { + if (string.IsNullOrEmpty(authorizationHeader)) + { + return []; + } + + var token = authorizationHeader.Replace("Bearer ", string.Empty, StringComparison.OrdinalIgnoreCase).Trim(); + if (token.Split('.').Length != 3) + { + return []; + } + + try + { + var jwtSecurityToken = new JwtSecurityTokenHandler().ReadJwtToken(token); + return [.. jwtSecurityToken.Claims + .Where(c => c.Type is "oid" or "upn" or "preferred_username" && !string.IsNullOrEmpty(c.Value)) + .Select(c => c.Value)]; + } + catch + { + return []; + } + } + internal static string GetTokenizedUrl(string absoluteUrl) { absoluteUrl = CanonicalizeDriveItemPathUrl(absoluteUrl); From 9b45753bbda69ad7a33087a400a60bd8f07d3972 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:14:18 +0100 Subject: [PATCH 06/10] Log a tip when Graph URL can't be analyzed for missing permissions Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 42 +++++++++++++++++++ .../GraphMissingPermissionsGuidancePlugin.cs | 3 +- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index 1331bfe3b..b9832a0ae 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -315,6 +315,48 @@ public async Task Forbidden_DelegatedSelfOnlyPermissionForSignedInUser_LogsWarni Assert.Contains("another reason", log.Message, StringComparison.Ordinal); } + [Theory] + [InlineData("https://graph.microsoft.com/v1.0/users('8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c')")] + [InlineData("https://graph.microsoft.com/v1.0/users('john@contoso.com')/messages")] + public async Task Forbidden_UnparsableUrl_LogsTip(string url) + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", url, [("Authorization", BearerToken(scp: "User.Read"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Tip)); + Assert.Contains("couldn't analyze missing permissions", log.Message, StringComparison.Ordinal); + Assert.Empty(handler.Requests); + } + + [Fact] + public async Task Batch_ForbiddenInnerResponseWithUnparsableUrl_LogsTip() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var batchRequest = """ + { "requests": [ { "id": "1", "method": "GET", "url": "/users('john@contoso.com')/messages" } ] } + """; + var batchResponse = """ + { "responses": [ { "id": "1", "status": 403, "body": {} } ] } + """; + + var exchange = TestExchange + .Request("POST", "https://graph.microsoft.com/v1.0/$batch", [("Authorization", BearerToken(scp: "User.Read")), ("Content-Type", "application/json")], batchRequest) + .WithResponse(HttpStatusCode.OK, [("Content-Type", "application/json")], batchResponse); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Tip)); + Assert.Contains("Batch request 1", log.Message, StringComparison.Ordinal); + Assert.Contains("couldn't analyze missing permissions", log.Message, StringComparison.Ordinal); + Assert.Empty(handler.Requests); + } + [Fact] public async Task Forbidden_NoLeastPrivilegedPermissionMarked_ListsUnrankedAlternatives() { diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index a966279ca..4de0ccb21 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -158,7 +158,8 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex catch (Exception ex) { Logger.LogDebug(ex, "Failed to tokenize URL {Url}", absoluteUrl); - return (null, MessageType.Skipped); + return ($"{requestDescription} failed with 403 Forbidden, but Dev Proxy couldn't analyze missing permissions for {method} {absoluteUrl} because it couldn't parse the URL. Check the Microsoft Graph API reference for the permissions required to call this API.", + MessageType.Tip); } var requiredPermissions = await GetPermissionsAsync(method, tokenizedUrl, permissionsType.Value, cancellationToken); From da0ed084183dad0b115531a6f15227591d52d9a5 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:21:59 +0100 Subject: [PATCH 07/10] Decode the access token only when analyzing forbidden responses Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../GraphMissingPermissionsGuidancePlugin.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index 4de0ccb21..440d263b9 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -5,6 +5,7 @@ using DevProxy.Abstractions.Models; using DevProxy.Abstractions.Plugins; using DevProxy.Abstractions.Proxy; +using DevProxy.Abstractions.Proxy.Http; using DevProxy.Abstractions.Utils; using DevProxy.Plugins.Models; using DevProxy.Plugins.Utils; @@ -68,10 +69,6 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT return; } - var authorizationHeader = request.Headers.GetFirst("Authorization")?.Value; - var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader(authorizationHeader); - var userIdentifiers = GraphUtils.GetUserIdentifiersFromAuthorizationHeader(authorizationHeader); - if (ProxyUtils.IsGraphBatchUrl(request.RequestUri)) { if (response.StatusCode != HttpStatusCode.OK) @@ -87,6 +84,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT return; } + var (permissionsType, tokenPermissions, userIdentifiers) = GetTokenInfo(request); var graphVersion = ProxyUtils.IsGraphBetaUrl(request.RequestUri) ? "beta" : "v1.0"; // analyze in parallel to limit the delay before the response is sent to the client var guidance = await Task.WhenAll(failedRequests.Select(failedRequest => @@ -109,6 +107,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT } else if (response.StatusCode == HttpStatusCode.Forbidden) { + var (permissionsType, tokenPermissions, userIdentifiers) = GetTokenInfo(request); var (message, messageType) = await AnalyzeForbiddenRequestAsync( request.Method, request.RequestUri.AbsoluteUri, @@ -127,6 +126,16 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT Logger.LogTrace("Left {Name}", nameof(AfterResponseAsync)); } + private static (GraphPermissionsType? Type, IEnumerable Permissions, string[] UserIdentifiers) GetTokenInfo(IHttpRequest request) + { + var authorizationHeader = request.Headers.GetFirst("Authorization")?.Value; + var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader(authorizationHeader); + var userIdentifiers = permissionsType == GraphPermissionsType.Delegated + ? GraphUtils.GetUserIdentifiersFromAuthorizationHeader(authorizationHeader) + : []; + return (permissionsType, tokenPermissions, userIdentifiers); + } + private void LogGuidance(string? message, MessageType messageType, LoggingContext loggingContext) { if (message is not null) From aa6d0b07b3086f49a5c1e1cee5f0659a8594f1e0 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:31:34 +0100 Subject: [PATCH 08/10] Use personal-account permissions for Microsoft account tokens Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 21 +++++++++++++- .../GraphMissingPermissionsGuidancePlugin.cs | 29 ++++++++++++------- DevProxy.Plugins/Utils/GraphUtils.cs | 15 ++++++---- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index b9832a0ae..deb133734 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -357,6 +357,21 @@ public async Task Batch_ForbiddenInnerResponseWithUnparsableUrl_LogsTip() Assert.Empty(handler.Requests); } + [Fact] + public async Task Forbidden_PersonalAccountToken_UsesDelegatedPersonalPermissions() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/me/messages", [("Authorization", BearerToken(scp: "User.Read", tid: "9188040d-6c67-4c5b-b112-36a304b66dad"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + _ = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("scopeType=DelegatedPersonal", Assert.Single(handler.Requests).Query, StringComparison.Ordinal); + } + [Fact] public async Task Forbidden_NoLeastPrivilegedPermissionMarked_ListsUnrankedAlternatives() { @@ -391,9 +406,13 @@ private static (GraphMissingPermissionsGuidancePlugin Plugin, CapturingLoggerFac return (plugin, factory); } - private static string BearerToken(string? scp = null, string[]? roles = null, string? oid = null) + private static string BearerToken(string? scp = null, string[]? roles = null, string? oid = null, string? tid = null) { var payload = new Dictionary { ["aud"] = "https://graph.microsoft.com" }; + if (tid is not null) + { + payload["tid"] = tid; + } if (oid is not null) { payload["oid"] = oid; diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index 440d263b9..7467c9b9d 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -22,6 +22,7 @@ public sealed class GraphMissingPermissionsGuidancePlugin( ILogger logger, ISet urlsToWatch) : BasePlugin(logger, urlsToWatch) { + private const string ConsumersTenantId = "9188040d-6c67-4c5b-b112-36a304b66dad"; private static readonly TimeSpan PermissionsLookupTimeout = TimeSpan.FromSeconds(3); private static readonly HashSet SelfOnlyDelegatedPermissions = new(["User.Read", "User.ReadWrite"], StringComparer.OrdinalIgnoreCase); @@ -84,7 +85,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT return; } - var (permissionsType, tokenPermissions, userIdentifiers) = GetTokenInfo(request); + var (permissionsType, tokenPermissions, userIdentifiers, isPersonalAccount) = GetTokenInfo(request); var graphVersion = ProxyUtils.IsGraphBetaUrl(request.RequestUri) ? "beta" : "v1.0"; // analyze in parallel to limit the delay before the response is sent to the client var guidance = await Task.WhenAll(failedRequests.Select(failedRequest => @@ -98,6 +99,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT permissionsType, tokenPermissions, userIdentifiers, + isPersonalAccount, cancellationToken); })); foreach (var (message, messageType) in guidance) @@ -107,7 +109,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT } else if (response.StatusCode == HttpStatusCode.Forbidden) { - var (permissionsType, tokenPermissions, userIdentifiers) = GetTokenInfo(request); + var (permissionsType, tokenPermissions, userIdentifiers, isPersonalAccount) = GetTokenInfo(request); var (message, messageType) = await AnalyzeForbiddenRequestAsync( request.Method, request.RequestUri.AbsoluteUri, @@ -115,6 +117,7 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT permissionsType, tokenPermissions, userIdentifiers, + isPersonalAccount, cancellationToken); LogGuidance(message, messageType, loggingContext); } @@ -126,14 +129,18 @@ public override async Task AfterResponseAsync(ProxyResponseArgs e, CancellationT Logger.LogTrace("Left {Name}", nameof(AfterResponseAsync)); } - private static (GraphPermissionsType? Type, IEnumerable Permissions, string[] UserIdentifiers) GetTokenInfo(IHttpRequest request) + private static (GraphPermissionsType? Type, IEnumerable Permissions, string[] UserIdentifiers, bool IsPersonalAccount) GetTokenInfo(IHttpRequest request) { var authorizationHeader = request.Headers.GetFirst("Authorization")?.Value; var (permissionsType, tokenPermissions) = GraphUtils.GetPermissionsFromAuthorizationHeader(authorizationHeader); - var userIdentifiers = permissionsType == GraphPermissionsType.Delegated - ? GraphUtils.GetUserIdentifiersFromAuthorizationHeader(authorizationHeader) - : []; - return (permissionsType, tokenPermissions, userIdentifiers); + if (permissionsType != GraphPermissionsType.Delegated) + { + return (permissionsType, tokenPermissions, [], false); + } + + var (userIdentifiers, tenantId) = GraphUtils.GetUserInfoFromAuthorizationHeader(authorizationHeader); + var isPersonalAccount = string.Equals(tenantId, ConsumersTenantId, StringComparison.OrdinalIgnoreCase); + return (permissionsType, tokenPermissions, userIdentifiers, isPersonalAccount); } private void LogGuidance(string? message, MessageType messageType, LoggingContext loggingContext) @@ -151,6 +158,7 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex GraphPermissionsType? permissionsType, IEnumerable tokenPermissions, string[] userIdentifiers, + bool isPersonalAccount, CancellationToken cancellationToken) { if (permissionsType is null) @@ -171,7 +179,9 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex MessageType.Tip); } - var requiredPermissions = await GetPermissionsAsync(method, tokenizedUrl, permissionsType.Value, cancellationToken); + // personal Microsoft accounts support a different set of delegated permissions + var scopeType = isPersonalAccount ? "DelegatedPersonal" : GraphUtils.GetScopeTypeString(permissionsType.Value); + var requiredPermissions = await GetPermissionsAsync(method, tokenizedUrl, scopeType, cancellationToken); if (requiredPermissions is null || requiredPermissions.Length == 0) { return ($"{requestDescription} failed with 403 Forbidden. Dev Proxy couldn't determine which permissions are required for {method} {tokenizedUrl}. Check the Microsoft Graph API reference for the permissions required to call this API.", @@ -239,10 +249,9 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex private async Task GetPermissionsAsync( string method, string tokenizedUrl, - GraphPermissionsType permissionsType, + string scopeType, CancellationToken cancellationToken) { - var scopeType = GraphUtils.GetScopeTypeString(permissionsType); var cacheKey = $"{scopeType}|{method}|{tokenizedUrl}"; if (_permissionsCache.TryGetValue(cacheKey, out var cachedPermissions)) { diff --git a/DevProxy.Plugins/Utils/GraphUtils.cs b/DevProxy.Plugins/Utils/GraphUtils.cs index 99d20a8f4..9c6da1453 100644 --- a/DevProxy.Plugins/Utils/GraphUtils.cs +++ b/DevProxy.Plugins/Utils/GraphUtils.cs @@ -160,29 +160,32 @@ internal static (GraphPermissionsType? Type, IEnumerable Permissions) Ge } } - internal static string[] GetUserIdentifiersFromAuthorizationHeader(string? authorizationHeader) + internal static (string[] UserIdentifiers, string? TenantId) GetUserInfoFromAuthorizationHeader(string? authorizationHeader) { if (string.IsNullOrEmpty(authorizationHeader)) { - return []; + return ([], null); } var token = authorizationHeader.Replace("Bearer ", string.Empty, StringComparison.OrdinalIgnoreCase).Trim(); if (token.Split('.').Length != 3) { - return []; + return ([], null); } try { var jwtSecurityToken = new JwtSecurityTokenHandler().ReadJwtToken(token); - return [.. jwtSecurityToken.Claims + var userIdentifiers = jwtSecurityToken.Claims .Where(c => c.Type is "oid" or "upn" or "preferred_username" && !string.IsNullOrEmpty(c.Value)) - .Select(c => c.Value)]; + .Select(c => c.Value) + .ToArray(); + var tenantId = jwtSecurityToken.Claims.FirstOrDefault(c => c.Type == "tid")?.Value; + return (userIdentifiers, tenantId); } catch { - return []; + return ([], null); } } From 64ff73755153a86966e3ad4bdee92417dd12d1f6 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:39:14 +0100 Subject: [PATCH 09/10] Explain delegated access to another user's data in missing permissions guidance Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 32 +++++++++++++++++++ .../GraphMissingPermissionsGuidancePlugin.cs | 17 +++++++--- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index deb133734..f655dc66e 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -372,6 +372,38 @@ public async Task Forbidden_PersonalAccountToken_UsesDelegatedPersonalPermission Assert.Contains("scopeType=DelegatedPersonal", Assert.Single(handler.Requests).Query, StringComparison.Ordinal); } + [Fact] + public async Task Forbidden_DelegatedOtherUserData_IncludesSharingNote() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/users/8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c/messages", [("Authorization", BearerToken(scp: "User.Read", oid: "1d2c3b4a-5e6f-4a7b-8c9d-0e1f2a3b4c5d"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.Contains("Mail.ReadBasic", log.Message, StringComparison.Ordinal); + Assert.Contains("that user must share it with the signed-in user", log.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Forbidden_DelegatedSignedInUserData_OmitsSharingNote() + { + using var handler = new StubPermissionsHandler(MailPermissionsJson); + var (plugin, factory) = CreatePlugin(handler); + + const string signedInUserId = "8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c"; + var exchange = TestExchange + .Request("GET", $"https://graph.microsoft.com/v1.0/users/{signedInUserId}/messages", [("Authorization", BearerToken(scp: "User.Read", oid: signedInUserId))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.DoesNotContain("must share it", log.Message, StringComparison.Ordinal); + } + [Fact] public async Task Forbidden_NoLeastPrivilegedPermissionMarked_ListsUnrankedAlternatives() { diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index 7467c9b9d..3141f2994 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -188,12 +188,20 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex MessageType.Warning); } + var mayTargetOtherUser = permissionsType == GraphPermissionsType.Delegated && + tokenizedUrl.StartsWith("/users/", StringComparison.OrdinalIgnoreCase) && + !IsSignedInUser(absoluteUrl, userIdentifiers); + + // delegated permissions for another user's resources, like their + // mailbox, only work if that data is accessible to the signed-in user + var otherUserDataNote = mayTargetOtherUser && tokenizedUrl.Count(c => c == '/') > 2 + ? " If the request accesses another user's data, keep in mind that delegated permissions only allow access to data that the signed-in user can access. To access another user's data, such as their mail or calendar, that user must share it with the signed-in user (and the app must use the corresponding .Shared permission where available), or the app must use application permissions." + : string.Empty; + // delegated self-only permissions, like User.Read, are listed for // /users/{id} but only grant access to the signed-in user var selfOnlyNote = string.Empty; - if (permissionsType == GraphPermissionsType.Delegated && - tokenizedUrl.StartsWith("/users/", StringComparison.OrdinalIgnoreCase) && - !IsSignedInUser(absoluteUrl, userIdentifiers)) + if (mayTargetOtherUser) { var selfOnlyPermissions = requiredPermissions .Where(p => SelfOnlyDelegatedPermissions.Contains(p.Value)) @@ -217,7 +225,7 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex if (grantingPermissions.Length > 0) { - return ($"{requestDescription} failed with 403 Forbidden even though the access token has the {FormatList(grantingPermissions)} {GetPermissionsTypeString(permissionsType.Value)} permission(s) which allow calling {method} {tokenizedUrl}. The request likely failed for another reason, for example: the {(permissionsType == GraphPermissionsType.Delegated ? "signed-in user" : "app")} doesn't have access to the resource, a required license or administrative role is missing, or the resource is restricted by a policy. Check the error details in the response for more information.", + return ($"{requestDescription} failed with 403 Forbidden even though the access token has the {FormatList(grantingPermissions)} {GetPermissionsTypeString(permissionsType.Value)} permission(s) which allow calling {method} {tokenizedUrl}. The request likely failed for another reason, for example: the {(permissionsType == GraphPermissionsType.Delegated ? "signed-in user" : "app")} doesn't have access to the resource, a required license or administrative role is missing, or the resource is restricted by a policy. Check the error details in the response for more information." + otherUserDataNote, MessageType.Warning); } @@ -241,6 +249,7 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex (otherPermissions.Length > 0 ? $" Alternatively, use one of the following higher-privileged permissions: {FormatList(otherPermissions)}." : string.Empty) : $"Add any of the following permissions: {FormatList(otherPermissions)}.") + selfOnlyNote + + otherUserDataNote + " " + GetInstructions(permissionsType.Value); return (message, MessageType.Failed); From 304af41ab4deffe39d2f80f497776f10acb38f32 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:48:56 +0100 Subject: [PATCH 10/10] Limit sharing advice to mailbox, calendar and contacts access Refs #88 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...phMissingPermissionsGuidancePluginTests.cs | 24 +++++++++++++++++-- .../GraphMissingPermissionsGuidancePlugin.cs | 18 ++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs index f655dc66e..4435aec92 100644 --- a/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs +++ b/DevProxy.Integration.Tests/GraphMissingPermissionsGuidancePluginTests.cs @@ -385,7 +385,27 @@ public async Task Forbidden_DelegatedOtherUserData_IncludesSharingNote() var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); Assert.Contains("Mail.ReadBasic", log.Message, StringComparison.Ordinal); - Assert.Contains("that user must share it with the signed-in user", log.Message, StringComparison.Ordinal); + Assert.Contains("Mail.Read.Shared", log.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task Forbidden_DelegatedOtherUserDirectoryData_OmitsSharingNote() + { + using var handler = new StubPermissionsHandler(""" + [ + { "value": "GroupMember.Read.All", "scopeType": "DelegatedWork", "isLeastPrivilege": true, "isHidden": false }, + { "value": "Directory.Read.All", "scopeType": "DelegatedWork", "isLeastPrivilege": false, "isHidden": false } + ] + """); + var (plugin, factory) = CreatePlugin(handler); + + var exchange = TestExchange + .Request("GET", "https://graph.microsoft.com/v1.0/users/8e6b0bba-5bce-4b1b-9d5b-1f5e8b0d1a6c/memberOf", [("Authorization", BearerToken(scp: "User.Read", oid: "1d2c3b4a-5e6f-4a7b-8c9d-0e1f2a3b4c5d"))]) + .WithResponse(HttpStatusCode.Forbidden); + await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); + + var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); + Assert.DoesNotContain(".Shared", log.Message, StringComparison.Ordinal); } [Fact] @@ -401,7 +421,7 @@ public async Task Forbidden_DelegatedSignedInUserData_OmitsSharingNote() await plugin.AfterResponseAsync(exchange.ResponseArgs, CancellationToken.None); var log = Assert.Single(factory.LogsOfType(MessageType.Failed)); - Assert.DoesNotContain("must share it", log.Message, StringComparison.Ordinal); + Assert.DoesNotContain(".Shared", log.Message, StringComparison.Ordinal); } [Fact] diff --git a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs index 3141f2994..660022771 100644 --- a/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs +++ b/DevProxy.Plugins/Guidance/GraphMissingPermissionsGuidancePlugin.cs @@ -22,6 +22,9 @@ public sealed class GraphMissingPermissionsGuidancePlugin( ILogger logger, ISet urlsToWatch) : BasePlugin(logger, urlsToWatch) { + private static readonly HashSet SharedDataResources = new( + ["messages", "mailFolders", "events", "calendar", "calendars", "calendarView", "calendarGroups", "contacts", "contactFolders"], + StringComparer.OrdinalIgnoreCase); private const string ConsumersTenantId = "9188040d-6c67-4c5b-b112-36a304b66dad"; private static readonly TimeSpan PermissionsLookupTimeout = TimeSpan.FromSeconds(3); private static readonly HashSet SelfOnlyDelegatedPermissions = new(["User.Read", "User.ReadWrite"], StringComparer.OrdinalIgnoreCase); @@ -192,10 +195,10 @@ private void LogGuidance(string? message, MessageType messageType, LoggingContex tokenizedUrl.StartsWith("/users/", StringComparison.OrdinalIgnoreCase) && !IsSignedInUser(absoluteUrl, userIdentifiers); - // delegated permissions for another user's resources, like their - // mailbox, only work if that data is accessible to the signed-in user - var otherUserDataNote = mayTargetOtherUser && tokenizedUrl.Count(c => c == '/') > 2 - ? " If the request accesses another user's data, keep in mind that delegated permissions only allow access to data that the signed-in user can access. To access another user's data, such as their mail or calendar, that user must share it with the signed-in user (and the app must use the corresponding .Shared permission where available), or the app must use application permissions." + // delegated mail, calendar and contacts permissions only cover another + // user's data if that user shared it with the signed-in user + var otherUserDataNote = mayTargetOtherUser && IsSharedDataResource(tokenizedUrl) + ? " If the request accesses another user's mailbox, calendar, or contacts, delegated permissions only work if that user shared the data with the signed-in user (or delegated access to it), and the app uses the corresponding .Shared permission, such as Mail.Read.Shared or Calendars.Read.Shared. Alternatively, use application permissions." : string.Empty; // delegated self-only permissions, like User.Read, are listed for @@ -360,6 +363,13 @@ private List GetForbiddenRequestsFromBatch(stri } } + private static bool IsSharedDataResource(string tokenizedUrl) + { + // segments: "", "users", "{users-id}", "{resource}", ... + var segments = tokenizedUrl.Split('/'); + return segments.Length > 3 && SharedDataResources.Contains(segments[3]); + } + private static bool IsSignedInUser(string absoluteUrl, string[] userIdentifiers) { // segments: "/", "{version}/", "users/", "{id}/", ...