From 9355694feac770e71df52ba6d0b93dea9546c218 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Mon, 7 Nov 2022 15:53:10 +0530 Subject: [PATCH 1/4] Handling READ operation with no fields referenced in url --- .../Authorization/RestAuthorizationHandler.cs | 6 ++++++ .../Sql Query Structures/SqlQueryStructure.cs | 13 ------------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Service/Authorization/RestAuthorizationHandler.cs b/src/Service/Authorization/RestAuthorizationHandler.cs index dd4db530fb..0a83055350 100644 --- a/src/Service/Authorization/RestAuthorizationHandler.cs +++ b/src/Service/Authorization/RestAuthorizationHandler.cs @@ -190,6 +190,12 @@ public Task HandleAsync(AuthorizationHandlerContext context) // - For other operation types, columnsToCheck is a result of identifying // any reference to a column in all parts of a request (body, URL, querystring) IEnumerable fieldsReturnedForFind = _authorizationResolver.GetAllowedExposedColumns(entityName, roleName, operation); + if (fieldsReturnedForFind.Count() == 0) + { + // READ operations with no accessible fields fail authorization. + context.Fail(); + } + restContext.UpdateReturnFields(fieldsReturnedForFind); } else diff --git a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 9b46bc5ed3..b1db72228a 100644 --- a/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Service/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -146,19 +146,6 @@ public SqlQueryStructure( IsListQuery = context.IsMany; TableAlias = $"{DatabaseObject.SchemaName}_{DatabaseObject.Name}"; AddFields(context, sqlMetadataProvider); - if (Columns.Count == 0) - { - SourceDefinition sourceDefinition = GetUnderlyingSourceDefinition(); - foreach (KeyValuePair column in sourceDefinition.Columns) - { - // We only include columns that are exposed for use in requests - if (sqlMetadataProvider.TryGetExposedColumnName(EntityName, column.Key, out string? name)) - { - AddColumn(column.Key, name!); - } - } - } - foreach (KeyValuePair predicate in context.PrimaryKeyValuePairs) { sqlMetadataProvider.TryGetBackingColumn(EntityName, predicate.Key, out string? backingColumn); From c8e0237a86538ccbccc7072ee5bc17bc09a2158c Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Thu, 10 Nov 2022 23:53:18 +0530 Subject: [PATCH 2/4] Adding test case --- .../REST/RestAuthorizationHandlerUnitTests.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index d8bfea1d18..0f0a160556 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -234,16 +234,19 @@ public async Task EntityRoleOperationResourceTest() // Negative tests where authorization fails for Find requests with no $f filter query string parameter [DataRow(new string[] { "col1", "col2", "col3", "col4", "col5" }, DisplayName = "Find - Request all allowed + 1 disallowed column(s)")] [DataRow(new string[] { "col1", "col5", "col6", "col7", "col9" }, DisplayName = "Find - Request 1 allowed + > 1 disallowed column(s)")] + [DataRow(new string[] { }, true, false, DisplayName = "Find - Request on entity with no included columns. The request url contains no key,select,orderby,filter.")] #pragma warning restore format [TestMethod] - public async Task FindColumnPermissionsTests(string[] columnsRequestedInput) + public async Task FindColumnPermissionsTests(string[] columnsRequestedInput, + bool areNoAllowedExposedColumns = false, + bool expectedAuthorizationResult = true) { IEnumerable columnsRequested = new List( columnsRequestedInput); - IEnumerable allowedColumns = new List( - new string[] { "col1", "col2", "col3", "col4" }); + IEnumerable allowedColumns = + new List(new string[] { "col1", "col2", "col3", "col4" }); + bool areColumnsAllowed = true; - bool expectedAuthorizationResult = true; // Creates Mock AuthorizationResolver to return a preset result based on [TestMethod] input. Mock authorizationResolver = new(); @@ -257,7 +260,7 @@ public async Task FindColumnPermissionsTests(string[] columnsRequestedInput) AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, Operation.Read - )).Returns(allowedColumns); + )).Returns(areNoAllowedExposedColumns ? new List() : allowedColumns); string httpMethod = HttpConstants.GET; HttpContext httpContext = CreateHttpContext(httpMethod); From ff392c73c885c31a2007cfb655f968eeb97dd78b Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 11 Nov 2022 00:08:56 +0530 Subject: [PATCH 3/4] fixing test --- .../REST/RestAuthorizationHandlerUnitTests.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index 0f0a160556..a3871460bf 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -243,8 +243,12 @@ public async Task FindColumnPermissionsTests(string[] columnsRequestedInput, { IEnumerable columnsRequested = new List( columnsRequestedInput); - IEnumerable allowedColumns = - new List(new string[] { "col1", "col2", "col3", "col4" }); + IEnumerable allowedColumns = new List(); + + if (!areNoAllowedExposedColumns) + { + allowedColumns = new List(new string[] { "col1", "col2", "col3", "col4" }); + } bool areColumnsAllowed = true; @@ -260,7 +264,7 @@ public async Task FindColumnPermissionsTests(string[] columnsRequestedInput, AuthorizationHelpers.TEST_ENTITY, AuthorizationHelpers.TEST_ROLE, Operation.Read - )).Returns(areNoAllowedExposedColumns ? new List() : allowedColumns); + )).Returns(allowedColumns); string httpMethod = HttpConstants.GET; HttpContext httpContext = CreateHttpContext(httpMethod); From 5566f62bbc737e1b7b83c4f38ec4c20a3970f741 Mon Sep 17 00:00:00 2001 From: Ayush Agarwal Date: Fri, 11 Nov 2022 10:43:33 +0530 Subject: [PATCH 4/4] nits --- .../Authorization/REST/RestAuthorizationHandlerUnitTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs index a3871460bf..4b298f78c5 100644 --- a/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs +++ b/src/Service.Tests/Authorization/REST/RestAuthorizationHandlerUnitTests.cs @@ -234,18 +234,18 @@ public async Task EntityRoleOperationResourceTest() // Negative tests where authorization fails for Find requests with no $f filter query string parameter [DataRow(new string[] { "col1", "col2", "col3", "col4", "col5" }, DisplayName = "Find - Request all allowed + 1 disallowed column(s)")] [DataRow(new string[] { "col1", "col5", "col6", "col7", "col9" }, DisplayName = "Find - Request 1 allowed + > 1 disallowed column(s)")] - [DataRow(new string[] { }, true, false, DisplayName = "Find - Request on entity with no included columns. The request url contains no key,select,orderby,filter.")] + [DataRow(new string[] { }, false, false, DisplayName = "Find - Request on entity with no included columns. The request url contains no key,select,orderby,filter.")] #pragma warning restore format [TestMethod] public async Task FindColumnPermissionsTests(string[] columnsRequestedInput, - bool areNoAllowedExposedColumns = false, + bool areAllowedExposedColumns = true, bool expectedAuthorizationResult = true) { IEnumerable columnsRequested = new List( columnsRequestedInput); IEnumerable allowedColumns = new List(); - if (!areNoAllowedExposedColumns) + if (areAllowedExposedColumns) { allowedColumns = new List(new string[] { "col1", "col2", "col3", "col4" }); }