Skip to content

Commit f9fd1aa

Browse files
bartizanwaldekmastykarz
bartizan
and
waldekmastykarz
authored
Extend MinimalPermissionsGuidancePlugin with scopes to ignore (#1365)
* Add PermissionsToExclude property and initialization * Move ApiSpecsFolderPath property initialization to dedicated method InitializeApiSpecsFolderPath * Small code adjustments * Move GetMethodAndUrl to common convert method ToMethodAndUrl * Add MethodAndUrl record instead of tuple and MethodAndUrlComparer * Move MethodAndUrlUtils to GraphUtils unit * Move common GetRequestsFromBatch and GetTokenizedUrl methods to GraphUtils * Exclude permissions from excessive list * Add ExcludedPermissions property to report class * Add logging of what permissions are excluded * Add permissionsToExclude to MinimalPermissionsGuidancePlugin config schema * Update schema only for v1.1.0 * Lower accessibility level to internal for GetMethodAndUrl, GetTokenizedUrl, GetRequestsFromBatch * Move GetMethodAndUrl func out to MethodAndUrlUtils unit * Evaluate excessivePermissions and properly initiate flag UsesMinimalPermissions * Refactor: Remove unused using directives and improve exception handling in MinimalPermissionsGuidancePlugin --------- Co-authored-by: Waldek Mastykarz <[email protected]>
1 parent 80126c4 commit f9fd1aa

9 files changed

+155
-180
lines changed

‎DevProxy.Plugins/Generation/MockGeneratorPlugin.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using DevProxy.Abstractions.Proxy;
88
using DevProxy.Abstractions.Utils;
99
using DevProxy.Plugins.Mocking;
10-
using DevProxy.Plugins.Utils;
1110
using Microsoft.Extensions.Logging;
1211
using System.Text.Json;
1312
using Titanium.Web.Proxy.EventArguments;
@@ -34,7 +33,6 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
3433

3534
Logger.LogInformation("Creating mocks from recorded requests...");
3635

37-
var methodAndUrlComparer = new MethodAndUrlComparer();
3836
var mocks = new List<MockResponse>();
3937

4038
foreach (var request in e.RequestLogs)

‎DevProxy.Plugins/Reporting/GraphMinimalPermissionsGuidancePlugin.cs

Lines changed: 19 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using DevProxy.Abstractions.Models;
65
using DevProxy.Abstractions.Proxy;
76
using DevProxy.Abstractions.Plugins;
87
using DevProxy.Abstractions.Utils;
@@ -77,9 +76,8 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
7776
return;
7877
}
7978

80-
var methodAndUrlComparer = new MethodAndUrlComparer();
81-
var delegatedEndpoints = new List<(string method, string url)>();
82-
var applicationEndpoints = new List<(string method, string url)>();
79+
var delegatedEndpoints = new List<MethodAndUrl>();
80+
var applicationEndpoints = new List<MethodAndUrl>();
8381

8482
// scope for delegated permissions
8583
IEnumerable<string> scopesToEvaluate = [];
@@ -94,21 +92,21 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
9492
}
9593

9694
var methodAndUrlString = request.Message;
97-
var methodAndUrl = GetMethodAndUrl(methodAndUrlString);
98-
if (methodAndUrl.method.Equals("OPTIONS", StringComparison.OrdinalIgnoreCase))
95+
var methodAndUrl = MethodAndUrlUtils.GetMethodAndUrl(methodAndUrlString);
96+
if (methodAndUrl.Method.Equals("OPTIONS", StringComparison.OrdinalIgnoreCase))
9997
{
10098
continue;
10199
}
102100

103-
if (!ProxyUtils.MatchesUrlToWatch(UrlsToWatch, methodAndUrl.url))
101+
if (!ProxyUtils.MatchesUrlToWatch(UrlsToWatch, methodAndUrl.Url))
104102
{
105-
Logger.LogDebug("URL not matched: {Url}", methodAndUrl.url);
103+
Logger.LogDebug("URL not matched: {Url}", methodAndUrl.Url);
106104
continue;
107105
}
108106

109-
var requestsFromBatch = Array.Empty<(string method, string url)>();
107+
var requestsFromBatch = Array.Empty<MethodAndUrl>();
110108

111-
var uri = new Uri(methodAndUrl.url);
109+
var uri = new Uri(methodAndUrl.Url);
112110
if (!ProxyUtils.IsGraphUrl(uri))
113111
{
114112
continue;
@@ -117,11 +115,11 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
117115
if (ProxyUtils.IsGraphBatchUrl(uri))
118116
{
119117
var graphVersion = ProxyUtils.IsGraphBetaUrl(uri) ? "beta" : "v1.0";
120-
requestsFromBatch = GetRequestsFromBatch(request.Context?.Session.HttpClient.Request.BodyString!, graphVersion, uri.Host);
118+
requestsFromBatch = GraphUtils.GetRequestsFromBatch(request.Context?.Session.HttpClient.Request.BodyString!, graphVersion, uri.Host);
121119
}
122120
else
123121
{
124-
methodAndUrl = (methodAndUrl.method, GetTokenizedUrl(methodAndUrl.url));
122+
methodAndUrl = new(methodAndUrl.Method, GraphUtils.GetTokenizedUrl(methodAndUrl.Url));
125123
}
126124

127125
var (type, permissions) = GetPermissionsAndType(request);
@@ -162,8 +160,8 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
162160
}
163161

164162
// Remove duplicates
165-
delegatedEndpoints = [.. delegatedEndpoints.Distinct(methodAndUrlComparer)];
166-
applicationEndpoints = [.. applicationEndpoints.Distinct(methodAndUrlComparer)];
163+
delegatedEndpoints = [.. delegatedEndpoints.Distinct()];
164+
applicationEndpoints = [.. applicationEndpoints.Distinct()];
167165

168166
if (delegatedEndpoints.Count == 0 && applicationEndpoints.Count == 0)
169167
{
@@ -177,8 +175,7 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
177175

178176
Logger.LogWarning("This plugin is in preview and may not return the correct results.\r\nPlease review the permissions and test your app before using them in production.\r\nIf you have any feedback, please open an issue at https://aka.ms/devproxy/issue.\r\n");
179177

180-
if (Configuration.PermissionsToExclude is not null &&
181-
Configuration.PermissionsToExclude.Any())
178+
if (Configuration.PermissionsToExclude?.Any() == true)
182179
{
183180
Logger.LogInformation("Excluding the following permissions: {Permissions}", string.Join(", ", Configuration.PermissionsToExclude));
184181
}
@@ -188,7 +185,7 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
188185
var delegatedPermissionsInfo = new GraphMinimalPermissionsInfo();
189186
report.DelegatedPermissions = delegatedPermissionsInfo;
190187

191-
Logger.LogInformation("Evaluating delegated permissions for: {Endpoints}", string.Join(", ", delegatedEndpoints.Select(e => $"{e.method} {e.url}")));
188+
Logger.LogInformation("Evaluating delegated permissions for: {Endpoints}", string.Join(", ", delegatedEndpoints.Select(e => $"{e.Method} {e.Url}")));
192189

193190
await EvaluateMinimalScopesAsync(delegatedEndpoints, scopesToEvaluate, GraphPermissionsType.Delegated, delegatedPermissionsInfo, cancellationToken);
194191
}
@@ -198,7 +195,7 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
198195
var applicationPermissionsInfo = new GraphMinimalPermissionsInfo();
199196
report.ApplicationPermissions = applicationPermissionsInfo;
200197

201-
Logger.LogInformation("Evaluating application permissions for: {Endpoints}", string.Join(", ", applicationEndpoints.Select(e => $"{e.method} {e.url}")));
198+
Logger.LogInformation("Evaluating application permissions for: {Endpoints}", string.Join(", ", applicationEndpoints.Select(e => $"{e.Method} {e.Url}")));
202199

203200
await EvaluateMinimalScopesAsync(applicationEndpoints, rolesToEvaluate, GraphPermissionsType.Application, applicationPermissionsInfo, cancellationToken);
204201
}
@@ -218,7 +215,7 @@ private void InitializePermissionsToExclude()
218215
}
219216

220217
private async Task EvaluateMinimalScopesAsync(
221-
IEnumerable<(string method, string url)> endpoints,
218+
IEnumerable<MethodAndUrl> endpoints,
222219
IEnumerable<string> permissionsFromAccessToken,
223220
GraphPermissionsType scopeType,
224221
GraphMinimalPermissionsInfo permissionsInfo,
@@ -229,12 +226,12 @@ private async Task EvaluateMinimalScopesAsync(
229226
throw new InvalidOperationException("GraphUtils is not initialized. Make sure to call InitializeAsync first.");
230227
}
231228

232-
var payload = endpoints.Select(e => new GraphRequestInfo { Method = e.method, Url = e.url });
229+
var payload = endpoints.Select(e => new GraphRequestInfo { Method = e.Method, Url = e.Url });
233230

234231
permissionsInfo.Operations = [.. endpoints.Select(e => new GraphMinimalPermissionsOperationInfo
235232
{
236-
Method = e.method,
237-
Endpoint = e.url
233+
Method = e.Method,
234+
Endpoint = e.Url
238235
})];
239236
permissionsInfo.PermissionsFromTheToken = permissionsFromAccessToken;
240237

@@ -290,40 +287,6 @@ private async Task EvaluateMinimalScopesAsync(
290287
}
291288
}
292289

293-
private static (string method, string url)[] GetRequestsFromBatch(string batchBody, string graphVersion, string graphHostName)
294-
{
295-
var requests = new List<(string method, string url)>();
296-
297-
if (string.IsNullOrEmpty(batchBody))
298-
{
299-
return [.. requests];
300-
}
301-
302-
try
303-
{
304-
var batch = JsonSerializer.Deserialize<GraphBatchRequestPayload>(batchBody, ProxyUtils.JsonSerializerOptions);
305-
if (batch == null)
306-
{
307-
return [.. requests];
308-
}
309-
310-
foreach (var request in batch.Requests)
311-
{
312-
try
313-
{
314-
var method = request.Method;
315-
var url = request.Url;
316-
var absoluteUrl = $"https://{graphHostName}/{graphVersion}{url}";
317-
requests.Add((method, GetTokenizedUrl(absoluteUrl)));
318-
}
319-
catch { }
320-
}
321-
}
322-
catch { }
323-
324-
return [.. requests];
325-
}
326-
327290
/// <summary>
328291
/// Returns permissions and type (delegated or application) from the access token
329292
/// used on the request.
@@ -377,20 +340,4 @@ private static (GraphPermissionsType type, IEnumerable<string> permissions) GetP
377340
return (GraphPermissionsType.Application, []);
378341
}
379342
}
380-
381-
private static (string method, string url) GetMethodAndUrl(string message)
382-
{
383-
var info = message.Split(" ");
384-
if (info.Length > 2)
385-
{
386-
info = [info[0], string.Join(" ", info.Skip(1))];
387-
}
388-
return (method: info[0], url: info[1]);
389-
}
390-
391-
private static string GetTokenizedUrl(string absoluteUrl)
392-
{
393-
var sanitizedUrl = ProxyUtils.SanitizeUrl(absoluteUrl);
394-
return "/" + string.Join("", new Uri(sanitizedUrl).Segments.Skip(2).Select(Uri.UnescapeDataString));
395-
}
396343
}

‎DevProxy.Plugins/Reporting/GraphMinimalPermissionsGuidancePluginReport.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ void transformPermissionsInfo(GraphMinimalPermissionsInfo permissionsInfo, strin
6262
transformPermissionsInfo(ApplicationPermissions, "application");
6363
}
6464

65-
if (ExcludedPermissions is not null &&
66-
ExcludedPermissions.Any())
65+
if (ExcludedPermissions?.Any() == true)
6766
{
6867
_ = sb.AppendLine("## Excluded permissions")
6968
.AppendLine()
@@ -112,10 +111,9 @@ void transformPermissionsInfo(GraphMinimalPermissionsInfo permissionsInfo, strin
112111
transformPermissionsInfo(ApplicationPermissions, "Application");
113112
}
114113

115-
if (ExcludedPermissions is not null &&
116-
ExcludedPermissions.Any())
114+
if (ExcludedPermissions?.Any() == true)
117115
{
118-
_ = sb.AppendLine("Excluded: permissions:")
116+
_ = sb.AppendLine("Excluded permissions:")
119117
.AppendLine()
120118
.AppendLine(string.Join(", ", ExcludedPermissions));
121119
}

‎DevProxy.Plugins/Reporting/GraphMinimalPermissionsPlugin.cs

Lines changed: 12 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using DevProxy.Abstractions.Proxy;
66
using DevProxy.Abstractions.Plugins;
77
using DevProxy.Abstractions.Utils;
8-
using DevProxy.Abstractions.Models;
98
using DevProxy.Plugins.Models;
109
using DevProxy.Plugins.Utils;
1110
using Microsoft.Extensions.Configuration;
@@ -60,8 +59,7 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
6059
return;
6160
}
6261

63-
var methodAndUrlComparer = new MethodAndUrlComparer();
64-
var endpoints = new List<(string method, string url)>();
62+
var endpoints = new List<MethodAndUrl>();
6563

6664
foreach (var request in e.RequestLogs)
6765
{
@@ -71,19 +69,19 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
7169
}
7270

7371
var methodAndUrlString = request.Message;
74-
var methodAndUrl = GetMethodAndUrl(methodAndUrlString);
75-
if (methodAndUrl.method.Equals("OPTIONS", StringComparison.OrdinalIgnoreCase))
72+
var methodAndUrl = MethodAndUrlUtils.GetMethodAndUrl(methodAndUrlString);
73+
if (methodAndUrl.Method.Equals("OPTIONS", StringComparison.OrdinalIgnoreCase))
7674
{
7775
continue;
7876
}
7977

80-
if (!ProxyUtils.MatchesUrlToWatch(UrlsToWatch, methodAndUrl.url))
78+
if (!ProxyUtils.MatchesUrlToWatch(UrlsToWatch, methodAndUrl.Url))
8179
{
82-
Logger.LogDebug("URL not matched: {Url}", methodAndUrl.url);
80+
Logger.LogDebug("URL not matched: {Url}", methodAndUrl.Url);
8381
continue;
8482
}
8583

86-
var uri = new Uri(methodAndUrl.url);
84+
var uri = new Uri(methodAndUrl.Url);
8785
if (!ProxyUtils.IsGraphUrl(uri))
8886
{
8987
continue;
@@ -92,26 +90,26 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
9290
if (ProxyUtils.IsGraphBatchUrl(uri))
9391
{
9492
var graphVersion = ProxyUtils.IsGraphBetaUrl(uri) ? "beta" : "v1.0";
95-
var requestsFromBatch = GetRequestsFromBatch(request.Context?.Session.HttpClient.Request.BodyString!, graphVersion, uri.Host);
93+
var requestsFromBatch = GraphUtils.GetRequestsFromBatch(request.Context?.Session.HttpClient.Request.BodyString!, graphVersion, uri.Host);
9694
endpoints.AddRange(requestsFromBatch);
9795
}
9896
else
9997
{
100-
methodAndUrl = (methodAndUrl.method, GetTokenizedUrl(methodAndUrl.url));
98+
methodAndUrl = new(methodAndUrl.Method, GraphUtils.GetTokenizedUrl(methodAndUrl.Url));
10199
endpoints.Add(methodAndUrl);
102100
}
103101
}
104102

105103
// Remove duplicates
106-
endpoints = [.. endpoints.Distinct(methodAndUrlComparer)];
104+
endpoints = [.. endpoints.Distinct()];
107105

108106
if (endpoints.Count == 0)
109107
{
110108
Logger.LogInformation("No requests to Microsoft Graph endpoints recorded. Will not retrieve minimal permissions.");
111109
return;
112110
}
113111

114-
Logger.LogInformation("Retrieving minimal permissions for:\r\n{Endpoints}\r\n", string.Join(Environment.NewLine, endpoints.Select(e => $"- {e.method} {e.url}")));
112+
Logger.LogInformation("Retrieving minimal permissions for:\r\n{Endpoints}\r\n", string.Join(Environment.NewLine, endpoints.Select(e => $"- {e.Method} {e.Url}")));
115113

116114
Logger.LogWarning("This plugin is in preview and may not return the correct results.\r\nPlease review the permissions and test your app before using them in production.\r\nIf you have any feedback, please open an issue at https://aka.ms/devproxy/issue.\r\n");
117115

@@ -125,15 +123,15 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
125123
}
126124

127125
private async Task<GraphMinimalPermissionsPluginReport?> DetermineMinimalScopesAsync(
128-
IEnumerable<(string method, string url)> endpoints,
126+
IEnumerable<MethodAndUrl> endpoints,
129127
CancellationToken cancellationToken)
130128
{
131129
if (_graphUtils is null)
132130
{
133131
throw new InvalidOperationException("GraphUtils is not initialized. Make sure to call InitializeAsync first.");
134132
}
135133

136-
var payload = endpoints.Select(e => new GraphRequestInfo { Method = e.method, Url = e.url });
134+
var payload = endpoints.Select(e => new GraphRequestInfo { Method = e.Method, Url = e.Url });
137135

138136
try
139137
{
@@ -178,54 +176,4 @@ public override async Task AfterRecordingStopAsync(RecordingArgs e, Cancellation
178176
return null;
179177
}
180178
}
181-
182-
private static (string method, string url)[] GetRequestsFromBatch(string batchBody, string graphVersion, string graphHostName)
183-
{
184-
var requests = new List<(string, string)>();
185-
186-
if (string.IsNullOrEmpty(batchBody))
187-
{
188-
return [.. requests];
189-
}
190-
191-
try
192-
{
193-
var batch = JsonSerializer.Deserialize<GraphBatchRequestPayload>(batchBody, ProxyUtils.JsonSerializerOptions);
194-
if (batch == null)
195-
{
196-
return [.. requests];
197-
}
198-
199-
foreach (var request in batch.Requests)
200-
{
201-
try
202-
{
203-
var method = request.Method;
204-
var url = request.Url;
205-
var absoluteUrl = $"https://{graphHostName}/{graphVersion}{url}";
206-
requests.Add((method, GetTokenizedUrl(absoluteUrl)));
207-
}
208-
catch { }
209-
}
210-
}
211-
catch { }
212-
213-
return [.. requests];
214-
}
215-
216-
private static (string method, string url) GetMethodAndUrl(string message)
217-
{
218-
var info = message.Split(" ");
219-
if (info.Length > 2)
220-
{
221-
info = [info[0], string.Join(" ", info.Skip(1))];
222-
}
223-
return (info[0], info[1]);
224-
}
225-
226-
private static string GetTokenizedUrl(string absoluteUrl)
227-
{
228-
var sanitizedUrl = ProxyUtils.SanitizeUrl(absoluteUrl);
229-
return "/" + string.Join("", new Uri(sanitizedUrl).Segments.Skip(2).Select(Uri.UnescapeDataString));
230-
}
231179
}

0 commit comments

Comments
 (0)