Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch least privilege permissions for set of request URLs and not individual request URLs #1510

Merged
merged 3 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions GraphWebApi/Controllers/PermissionsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ public async Task<IActionResult> GetPermissionScopes([FromQuery]ScopeType? scope

string localeCode = GetPreferredLocaleLanguage(Request);

var requestUrls = requestUrl != null ? new List<string> { requestUrl } : null;
var requests = requestUrl != null
? new List<RequestInfo> { new RequestInfo { RequestUrl = requestUrl, HttpMethod = method } }
: null;

PermissionResult result = await _permissionsStore.GetScopesAsync(
requestUrls: requestUrls,
requests: requests,
locale: localeCode,
scopeType: scopeType,
method: method,
includeHidden: includeHidden,
leastPrivilegeOnly: leastPrivilegeOnly,
org: org,
Expand All @@ -76,24 +78,22 @@ public async Task<IActionResult> GetPermissionScopes([FromQuery]ScopeType? scope

[HttpPost]
[Produces("application/json")]
public async Task<IActionResult> GetPermissionScopes([FromBody] List<string> requestUrls,
public async Task<IActionResult> GetPermissionScopes([FromBody] List<RequestInfo> requests,
millicentachieng marked this conversation as resolved.
Show resolved Hide resolved
[FromQuery] ScopeType? scopeType = null,
[FromQuery] string method = null,
[FromQuery] string org = null,
[FromQuery] string branchName = null,
[FromQuery] bool leastPrivilegeOnly = true,
[FromQuery] bool includeHidden = false)
{
if (requestUrls == null || !requestUrls.Any())
if (requests == null || !requests.Any())
return BadRequest("Request URLs cannot be null or empty");

string localeCode = GetPreferredLocaleLanguage(Request);

PermissionResult result = await _permissionsStore.GetScopesAsync(
requestUrls: requestUrls,
requests: requests,
locale: localeCode,
scopeType: scopeType,
method: method,
includeHidden: includeHidden,
leastPrivilegeOnly: leastPrivilegeOnly,
org: org,
Expand Down
56 changes: 30 additions & 26 deletions GraphWebApi/wwwroot/OpenApi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -257,20 +257,24 @@ paths:
description: Get list of permissions that enable access to a specified set of resources
operationId: permissions.ListPermissionsSet
parameters:
- $ref: "#/components/parameters/method"
- $ref: "#/components/parameters/scopeType"
- $ref: "#/components/parameters/org"
- $ref: "#/components/parameters/branchName"
- $ref: "#/components/parameters/includeHidden"
- $ref: "#/components/parameters/leastPrivilegeOnly"
requestBody:
description: An array of URLs
description: An array of URLs and corresponding HTTP methods
content:
application/json:
schema:
type: array
items:
type: string
type: object
properties:
requestUrl:
type: string
method:
type: string
responses:
"200":
description: Ordered array of permissions that enable access to a specified resource (or all the Microsoft Graph API endpoints), from least to most privileged along with detailed information about the permissions.
Expand Down Expand Up @@ -446,17 +450,17 @@ paths:
description: Get the Microsoft Graph API metadata CSDL document as an OpenAPI document.
operationId: openapi.GetDocument
parameters:
- $ref: '#/components/parameters/url'
- $ref: '#/components/parameters/tags'
- $ref: '#/components/parameters/operationIds'
- $ref: '#/components/parameters/openApiVersion'
- $ref: '#/components/parameters/graphVersion'
- $ref: '#/components/parameters/style'
- $ref: '#/components/parameters/title'
- $ref: '#/components/parameters/format'
- $ref: '#/components/parameters/forceRefresh'
- $ref: '#/components/parameters/singularizeOperationIds'
- $ref: '#/components/parameters/fileName'
- $ref: "#/components/parameters/url"
- $ref: "#/components/parameters/tags"
- $ref: "#/components/parameters/operationIds"
- $ref: "#/components/parameters/openApiVersion"
- $ref: "#/components/parameters/graphVersion"
- $ref: "#/components/parameters/style"
- $ref: "#/components/parameters/title"
- $ref: "#/components/parameters/format"
- $ref: "#/components/parameters/forceRefresh"
- $ref: "#/components/parameters/singularizeOperationIds"
- $ref: "#/components/parameters/fileName"
responses:
"200":
description: List of operations associated to a tag/operationIds/url
Expand All @@ -471,14 +475,14 @@ paths:
description: Get all the operations from the Microsoft Graph API metadata CSDL document.
operationId: openapi.operations.ListOperations
parameters:
- $ref: '#/components/parameters/openApiVersion'
- $ref: '#/components/parameters/graphVersion'
- $ref: '#/components/parameters/style'
- $ref: '#/components/parameters/title'
- $ref: '#/components/parameters/format'
- $ref: '#/components/parameters/forceRefresh'
- $ref: '#/components/parameters/singularizeOperationIds'
- $ref: '#/components/parameters/fileName'
- $ref: "#/components/parameters/openApiVersion"
- $ref: "#/components/parameters/graphVersion"
- $ref: "#/components/parameters/style"
- $ref: "#/components/parameters/title"
- $ref: "#/components/parameters/format"
- $ref: "#/components/parameters/forceRefresh"
- $ref: "#/components/parameters/singularizeOperationIds"
- $ref: "#/components/parameters/fileName"
responses:
"200":
description: OK
Expand All @@ -492,8 +496,8 @@ paths:
description: Get the Microsoft Graph API metadata CSDL document as an OpenAPIUrlTreeNode document.
operationId: openapi.tree.GetUrlTreeNode
parameters:
- $ref: '#/components/parameters/graphVersions'
- $ref: '#/components/parameters/forceRefresh'
- $ref: "#/components/parameters/graphVersions"
- $ref: "#/components/parameters/forceRefresh"
responses:
"200":
description: Simplified OpenAPI document(s) rendered as an OpenApiUrlTreeNode document.
Expand Down Expand Up @@ -869,7 +873,7 @@ components:
- yaml
default: json
description: The OpenAPI document output format.
forceRefresh:
forceRefresh:
name: forceRefresh
in: query
schema:
Expand Down Expand Up @@ -899,4 +903,4 @@ components:
schema:
type: string
required: false
description: Overrides the OpenAPI file name for the specified OpenApi style.
description: Overrides the OpenAPI file name for the specified OpenApi style.
69 changes: 38 additions & 31 deletions PermissionsService.Test/PermissionsStoreShould.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ public async Task GetAllRequiredPermissionScopesGivenAnExistingRequestUrl()
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/security/alerts/{alert_id}" },
method: "GET");
requests: new List<RequestInfo> { new RequestInfo { RequestUrl = "/security/alerts/{alert_id}", HttpMethod = "GET" } });

// Assert
Assert.Collection(result.Results,
Expand Down Expand Up @@ -83,8 +82,8 @@ public async Task GetRequiredPermissionScopesGivenAnExistingRequestUrlByScopeTyp
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/security/alerts/{alert_id}" },
method: "GET", scopeType: scopeType);
new List<RequestInfo> { new RequestInfo { RequestUrl = "/security/alerts/{alert_id}", HttpMethod = "GET" } },
scopeType: scopeType);

// Assert
if (scopeType == ScopeType.DelegatedWork)
Expand Down Expand Up @@ -148,8 +147,7 @@ public async Task GetLeastPrivilegePermissionScopesGivenAnExistingRequestUrl()
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/security/alerts/{alert_id}" },
method: "GET",
new List<RequestInfo> { new RequestInfo { RequestUrl = "/security/alerts/{alert_id}", HttpMethod = "GET" } },
scopeType: ScopeType.Application,
leastPrivilegeOnly: true);

Expand Down Expand Up @@ -198,8 +196,7 @@ public async Task ReturnNullGivenANonExistentRequestUrl()
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/foo/bar/{alert_id}" }, // non-existent request url
method: "GET");
new List<RequestInfo> { new RequestInfo { RequestUrl = "/foo/bar/{id}", HttpMethod = "GET" } }); // non-existent request url

// Assert
Assert.Empty(result.Results);
Expand All @@ -210,29 +207,31 @@ public async Task ReturnNullGivenANonExistentHttpVerb()
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/security/alerts/{alert_id}" },
method: "Foobar"); // non-existent http verb
new List<RequestInfo> {
new RequestInfo {
RequestUrl = "/security/alerts/{alert_id}",
HttpMethod = "Foobar" } }); // non-existent http verb

// Assert
Assert.Empty(result.Results);
}

[Theory]
[InlineData(true, 10)]
[InlineData(false, 12)]
[InlineData(true, 6)]
[InlineData(false, 10)]
public async Task ReturnLeastPrivilegePermissionsForSetOfResources(bool leastPrivilegeOnly, int expectedCount)
{
// Arrange
var requestUrls = new List<string>()
var requests = new List<RequestInfo>()
{
"/security/alerts/{alert_id}",
"/sites/{site_id}",
"/me/tasks/lists/delta"
new RequestInfo { RequestUrl = "/security/alerts/{alert_id}", HttpMethod = "GET" },
new RequestInfo { RequestUrl = "/sites/{site_id}", HttpMethod = "GET" },
new RequestInfo { RequestUrl = "/me/tasks/lists/delta", HttpMethod = "GET" }
};

// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: requestUrls,
requests: requests,
leastPrivilegeOnly: leastPrivilegeOnly);

// Assert
Expand All @@ -252,8 +251,7 @@ public async Task RemoveFunctionParametersFromRequestUrlsDuringLoadingAndQueryin
var result =
await _permissionsStore.GetScopesAsync(
scopeType: ScopeType.DelegatedWork,
requestUrls: new List<string>() { url },
method: "GET");
requests: new List<RequestInfo>() { new RequestInfo { RequestUrl = url, HttpMethod = "GET" } });

// Assert
Assert.Collection(result.Results,
Expand All @@ -271,8 +269,12 @@ public async Task ReturnScopesForRequestUrlWhoseScopesInformationNotAvailable()
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/lorem/ipsum/{id}" },
method: "GET"); // bogus permission whose scopes info are unavailable
new List<RequestInfo> {
new RequestInfo {
RequestUrl = "/lorem/ipsum/{id}", // bogus URL whose scopes info are unavailable
HttpMethod = "GET"
}
});

// Assert
Assert.Collection(result.Results,
Expand All @@ -297,8 +299,7 @@ public async Task ReturnLocalizedPermissionsDescriptionsForSupportedLanguage()
{
// Act
var result = await _permissionsStore.GetScopesAsync(
requestUrls: new List<string>() { "/security/alerts/{alert_id}" },
method: "GET",
requests: new List<RequestInfo>() { new RequestInfo { RequestUrl = "/security/alerts/{alert_id}", HttpMethod = "GET" } },
scopeType: ScopeType.DelegatedWork,
locale: "es-ES");

Expand All @@ -325,8 +326,11 @@ public async Task ReturnsErrorsForEmptyOrNullRequestUrls()
{
// Act
PermissionResult result =
await _permissionsStore.GetScopesAsync(requestUrls: new List<string>() { "", null },
method: "GET");
await _permissionsStore.GetScopesAsync(
requests: new List<RequestInfo>() {
new RequestInfo { RequestUrl = "", HttpMethod = "GET" },
new RequestInfo { RequestUrl = null, HttpMethod = "GET" } }
);
// Assert
Assert.Empty(result.Results);
Assert.NotEmpty(result.Errors);
Expand All @@ -349,8 +353,9 @@ public async Task ReturnsErrorsForNonExistentRequestUrls()
{
// Act
PermissionResult result =
await _permissionsStore.GetScopesAsync(requestUrls: new List<string>() { "/foo/bar" },
method: "GET");
await _permissionsStore.GetScopesAsync(
requests: new List<RequestInfo>() {
new RequestInfo { RequestUrl = "/foo/bar", HttpMethod = "GET" } });
// Assert
Assert.Empty(result.Results);
Assert.NotEmpty(result.Errors);
Expand All @@ -359,7 +364,7 @@ public async Task ReturnsErrorsForNonExistentRequestUrls()
item =>
{
Assert.Equal("/foo/bar", item.Url);
Assert.Equal("Permissions information for /foo/bar were not found.", item.Message);
Assert.Equal("Permissions information for 'GET /foo/bar' were not found.", item.Message);
});
}

Expand All @@ -368,8 +373,10 @@ public async Task ReturnsUniqueListOfPermissionsForPathsWithSharedPermissions()
{
// Act
PermissionResult result =
await _permissionsStore.GetScopesAsync(requestUrls: new List<string>() { "/accessreviews", "/accessreviews/{id}" },
method: "GET", scopeType: ScopeType.DelegatedWork);
await _permissionsStore.GetScopesAsync(requests: new List<RequestInfo>() {
new RequestInfo { RequestUrl = "/accessreviews", HttpMethod = "GET" },
new RequestInfo { RequestUrl = "/accessreviews/{id}", HttpMethod = "GET" } },
scopeType: ScopeType.DelegatedWork);

// Assert
Assert.Collection(result.Results,
Expand Down Expand Up @@ -409,7 +416,7 @@ public async Task FetchPermissionsDescriptionsFromGithubGivenARequestUrl()
string branchName = "Branch";

// Act
var result = await _permissionsStore.GetScopesAsync(org: org, branchName: branchName, scopeType: ScopeType.DelegatedWork, requestUrls: new List<string>() { "/security/alerts/{alert_id}" }, method: "GET");
var result = await _permissionsStore.GetScopesAsync(org: org, branchName: branchName, scopeType: ScopeType.DelegatedWork, requests: new List<RequestInfo>() { new RequestInfo { RequestUrl = "/security/alerts/{alert_id}", HttpMethod = "GET" } });

// Assert
Assert.Collection(result.Results,
Expand Down
12 changes: 5 additions & 7 deletions PermissionsService/Interfaces/IPermissionStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,17 @@ public interface IPermissionsStore
/// <summary>
/// Retrieves permissions scopes information for a set of URLs.
/// </summary>
/// <param name="requestUrls">The list of request URLs to fetch permissions for.</param>
/// <param name="locale">Optional: The language code for the preferred localized file.</param>
/// <param name="requests">The list of request URLs to fetch permissions for.</param>
/// <param name="locale">Optional: The language code for the preferred localized file.<</param>
/// <param name="scopeType">Optional: The type of scope to be retrieved for the target request url.</param>
/// <param name="method">Optional: The target http verb of the request url whose scopes are to be retrieved.</param>
/// <param name="includeHidden">Optional: Whether to include hidden permissions or not: Defaults to false.</param>
/// <param name="isLeastPrivilege">Optional: Whether to only return least privilege permissions on not. Defaults to false.</param>
/// <param name="leastPrivilegeOnly">Optional: Whether to only return least privilege permissions on not. Defaults to false.</param>
/// <param name="org">Optional: The name of the org/owner of the repo.</param>
/// <param name="branchName">Optional: The name of the branch containing the files</param>
/// <param name="branchName">Optional: The name of the branch containing the files.</param>
/// <returns></returns>
Task<PermissionResult> GetScopesAsync(List<string> requestUrls = null,
Task<PermissionResult> GetScopesAsync(List<RequestInfo> requests = null,
string locale = null,
ScopeType? scopeType = null,
string method = null,
bool includeHidden = false,
bool leastPrivilegeOnly = false,
string org = null,
Expand Down
19 changes: 19 additions & 0 deletions PermissionsService/Models/RequestInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Newtonsoft.Json;

namespace PermissionsService.Models
{
public class RequestInfo
{
[JsonProperty(PropertyName = "requestUrl")]
public string RequestUrl
{
get; set;
}

[JsonProperty(PropertyName = "method")]
public string HttpMethod
{
get; set;
}
}
}
Loading