Remove Unused code path in SqlQueryStructure

[seantleonard]

Issue

Tests like those in ODataASTVisitorUnitTests fail because the integration test fixture does not take authorization into consideration. These tests need to be updated.

Background

SqlQueryStructure handling REST requests checks if Columns.Count == 0 here:
https://github.com/Azure/hawaii-gql/blob/38588fc1496f82299dfa067744db5fcc2c5d062e/DataGateway.Service/Resolvers/Sql%20Query%20Structures/SqlQueryStructure.cs#L134-L141
And proceeds to add all columns from the table definition as "FieldsToBeReturned" in the results. This code path will not be reached in production code with Authorization mechanics in place. The behavior with authorization is that, for Find Requests, the results will only contain fields that the request is allowed to access.

For example, consider the following permissions config:

{
"Book": {
      "source": "books",
      "permissions": [
        {
          "role": "Author",
          "actions": [
            {
              "action": "read",
              "fields": {
                "include": [ "*" ],
                "exclude": [ "publisher_id" ]
              }
            }
          ]
        }
      ],
      "relationships": {
      }
    }
}

and REST Request: http://localhost:5001/rest/book/id/8
the expected result does not include the publisher_id field:

{
    "value": [
        {
            "id": 8,
            "title": "Time to Eat"
        }
    ]
}

[ayush3797]

Though the code can be removed as Rest requests don't depend on it, the existing {MsSql/PostgreSql/MySql}FindApiTests depend on the code block to add FieldsToBeReturned. If the code block is removed, the tests would fail. Because the call to _authorizationService.AuthorizeAsync() for ColumnsPermissionsRequirement is mocked to return true when a call to it is made, the FieldsToBeReturned are not populated as the Authorisation stage for column permissions is actually not executed (since it is mocked to return true straightaway.).

[ayush3797]

If we want to remove the code block, we would need to setup a pipeline for the tests because it depends the Authorisation middleware to be added (services.AddAuthorization), IAuthorizationHandler,RestAuthorizationHandler being connected via dependency injection, so that when AuthorizeAsyn() is called on an AuthorizationService object, the call gets resolved to RestAuthorizationHandler.HandleAsync() method.

[seantleonard]

@ayush3797 , will #706 help with closing this out?

Since the new test fixture for REST setups up all services as you mention, like services.AddAuthorization()

[ayush3797]

Yep, it will.

[seantleonard]

@ayush3797, can you reevaluate now that REST API test refactor is merged?

[ayush3797]

Can there be a case where an operation such as Read is defined for an entity but it does not provide access to any column in the entity, i.e. all fields in the entity are excluded for Read? In such a case, the code would still hit the block in question in this issue and proceed to add all the fields in the entity to the result to be returned.

To me, this now becomes a case of validation of the config based on the data in the config. If an operation is allowed on an entity it should allow access to at least one valid field in the entity.

For eg.

{
"Book": {
      "source": "books",
      "permissions": [
        {
          "role": "Author",
          "actions": [
            {
              "action": "read",
              "fields": {
                "include": [ "xyz", "id" ]
              }
            }
          ]
        }
      ]
    }
}

Read action here has only one valid field on book entity accessible -> id . xyz is not an existing field on the book entity. If id was not present in the included list, we should have thrown an exception that the config is invalid based on the data it possesses.