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

Fix context sql #2344

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
6 changes: 3 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# Version values referenced from https://hub.docker.com/_/microsoft-dotnet-aspnet

FROM mcr.microsoft.com/dotnet/sdk:6.0-cbl-mariner2.0. AS build
FROM mcr.microsoft.com/dotnet/sdk:8.0 AS build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will soon update this to 8.0. Please refrain from doing so right now.



WORKDIR /src
COPY [".", "./"]
RUN dotnet build "./src/Service/Azure.DataApiBuilder.Service.csproj" -c Docker -o /out -r linux-x64
RUN dotnet build "./src/Service/Azure.DataApiBuilder.Service.csproj" -f net8.0 -o /out -r linux-x64 --self-contained

FROM mcr.microsoft.com/dotnet/aspnet:6.0-cbl-mariner2.0 AS runtime
FROM mcr.microsoft.com/dotnet/aspnet:8.0 AS runtime

COPY --from=build /out /App
WORKDIR /App
Expand Down
1 change: 1 addition & 0 deletions src/Config/ObjectModel/AuthenticationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public record AuthenticationOptions(string Provider = nameof(EasyAuthType.Static
public const string CLIENT_PRINCIPAL_HEADER = "X-MS-CLIENT-PRINCIPAL";
public const string NAME_CLAIM_TYPE = "name";
public const string ROLE_CLAIM_TYPE = "roles";
public const string ORIGINAL_ROLE_CLAIM_TYPE = "original_roles";

/// <summary>
/// Returns whether the configured Provider matches an
Expand Down
7 changes: 6 additions & 1 deletion src/Core/Authorization/AuthorizationResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,14 @@ public static Dictionary<string, List<Claim>> GetAllAuthenticatedUserClaims(Http
// into a list and storing that in resolvedClaims using the claimType as the key.
foreach (Claim claim in identity.Claims)
{
// 'roles' claim has already been processed.
// 'roles' claim has already been processed. But we preserve the original 'roles' claim
if (claim.Type.Equals(AuthenticationOptions.ROLE_CLAIM_TYPE))
{
if(!resolvedClaims.TryAdd(AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE, new List<Claim>() { claim }))
{
resolvedClaims[AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE].Add(claim);
}

continue;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Core/Parsers/RequestParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class RequestParser
/// <summary>
/// Prefix used for specifying limit in the query string of the URL.
/// </summary>
public const string FIRST_URL = "$first";
public const string FIRST_URL = "$top";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will provide a synonym $top soon. Lets not make this change in this PR. Thank you for your patience.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related #2474

/// <summary>
/// Prefix used for specifying paging in the query string of the URL.
/// </summary>
Expand Down
44 changes: 42 additions & 2 deletions src/Core/Resolvers/DWSqlQueryBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,21 @@ public string Build(SqlQueryStructure structure)
/// <returns></returns>
private string BuildAsJson(SqlQueryStructure structure, bool subQueryStructure = false)
{
string subQueryAlias = "CountQuery";

string countSql = $" CROSS JOIN ( {BuildSqlCountQuery(structure)} ) {subQueryAlias}";

//Add a new column to the structure
structure.Columns.Add(new LabelledColumn("", subQueryAlias, "RecordCount", "RecordCount", subQueryAlias));

//Add a subquery 'a' ti the structure
structure.JoinQueries.Add(subQueryAlias, structure);

string columns = GenerateColumnsAsJson(structure, subQueryStructure);
string fromSql = $"{BuildSqlQuery(structure)}";

structure.JoinQueries.Remove(subQueryAlias);

string fromSql = $"{BuildSqlQuery(structure, countSql)}";
string query = $"SELECT {columns}"
+ $" FROM ({fromSql}) AS {QuoteIdentifier(structure.SourceAlias)}";
return query;
Expand All @@ -64,7 +77,7 @@ private string BuildAsJson(SqlQueryStructure structure, bool subQueryStructure =
/// FROM dbo_books AS[table0]
/// OUTER APPLY(SubQuery generated by recursive call to build function, will create the _subq tables)
/// </summary>
private string BuildSqlQuery(SqlQueryStructure structure)
private string BuildSqlQuery(SqlQueryStructure structure, string? subQuery)
{
string dataIdent = QuoteIdentifier(SqlQueryStructure.DATA_IDENT);
StringBuilder fromSql = new();
Expand All @@ -87,11 +100,38 @@ private string BuildSqlQuery(SqlQueryStructure structure)

string query = $"SELECT TOP {structure.Limit()} {columns}"
+ $" FROM {fromSql}"
+ $" {subQuery}"
+ $" WHERE {predicates}"
+ orderBy;
return query;
}

private string BuildSqlCountQuery(SqlQueryStructure structure)
{
string dataIdent = QuoteIdentifier(SqlQueryStructure.DATA_IDENT);
StringBuilder fromSql = new();

fromSql.Append($"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} " +
$"AS {QuoteIdentifier($"{structure.SourceAlias}")}{Build(structure.Joins)}");

fromSql.Append(string.Join(
"",
structure.JoinQueries.Select(
x => $" OUTER APPLY ({BuildAsJson(x.Value, true)}) AS {QuoteIdentifier(x.Key)}({dataIdent})")));

string predicates = JoinPredicateStrings(
structure.GetDbPolicyForOperation(EntityActionOperation.Read),
structure.FilterPredicates,
Build(structure.Predicates),
Build(structure.PaginationMetadata.PaginationPredicate));

string query = $"SELECT cast(count(1) as varchar(50)) as RecordCount "
+ $" FROM {fromSql}"
+ $" WHERE {predicates}";

return query;
}

private static string GenerateColumnsAsJson(SqlQueryStructure structure, bool subQueryStructure = false)
{
string columns;
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Resolvers/MsSqlQueryExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona
string paramName = $"{SESSION_PARAM_NAME}{counter.Next()}";
parameters.Add(paramName, new(claimValue));
// Append statement to set read only param value - can be set only once for a connection.
string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 1;";
string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 0;";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, did you experience - your mutation operations being blocked because of this setting?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the SQL Endpoint did reject the second query as described in #2341

sessionMapQuery = sessionMapQuery.Append(statementToSetReadOnlyParam);
}

Expand Down
27 changes: 24 additions & 3 deletions src/Core/Resolvers/SqlResponseHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ public static OkObjectResult FormatFindResult(
? DetermineExtraFieldsInResponse(findOperationResponse, context.FieldsToBeReturned)
: DetermineExtraFieldsInResponse(findOperationResponse.EnumerateArray().First(), context.FieldsToBeReturned);

//Remove RecordCOunt from extraFieldsInResponse if present
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this code is not used, please remove it out completely.

if (extraFieldsInResponse.Contains("RecordCount"))
{
extraFieldsInResponse.Remove("RecordCount");
}
*/
uint defaultPageSize = runtimeConfig.DefaultPageSize();
uint maxPageSize = runtimeConfig.MaxPageSize();

Expand Down Expand Up @@ -113,13 +120,24 @@ public static OkObjectResult FormatFindResult(
queryStringParameters: context!.ParsedQueryString,
after);

//Get the element RecordCount from the first element of the array
JsonElement recordCountElement = rootEnumerated[0].GetProperty("RecordCount");
string jsonRecordCount = JsonSerializer.Serialize(new[]
{
new
{
recordCount = @$"{rootEnumerated[0].GetProperty("RecordCount")}"
}
});

// When there are extra fields present, they are removed before returning the response.
if (extraFieldsInResponse.Count > 0)
{
rootEnumerated = RemoveExtraFieldsInResponseWithMultipleItems(rootEnumerated, extraFieldsInResponse);
}

rootEnumerated.Add(nextLink);
rootEnumerated.Add(JsonSerializer.Deserialize<JsonElement>(jsonRecordCount));
return OkResponse(JsonSerializer.SerializeToElement(rootEnumerated));
}

Expand Down Expand Up @@ -218,13 +236,16 @@ public static OkObjectResult OkResponse(JsonElement jsonResult)
// we strip the "[" and "]" and then save the nextLink element
// into a dictionary with a key of "nextLink" and a value that
// represents the nextLink data we require.
string nextLinkJsonString = JsonSerializer.Serialize(resultEnumerated[resultEnumerated.Count - 1]);
string nextLinkJsonString = JsonSerializer.Serialize(resultEnumerated[resultEnumerated.Count - 2]);
string recordCountJsonString = JsonSerializer.Serialize(resultEnumerated[resultEnumerated.Count - 1]);
Dictionary<string, object> nextLink = JsonSerializer.Deserialize<Dictionary<string, object>>(nextLinkJsonString[1..^1])!;
IEnumerable<JsonElement> value = resultEnumerated.Take(resultEnumerated.Count - 1);
Dictionary<string, object> recordCount = JsonSerializer.Deserialize<Dictionary<string, object>>(recordCountJsonString[1..^1])!;
IEnumerable<JsonElement> value = resultEnumerated.Take(resultEnumerated.Count - 2);
return new OkObjectResult(new
{
value = value,
@nextLink = nextLink["nextLink"]
@nextLink = nextLink["nextLink"],
@recordCount = recordCount["recordCount"]
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<PackageVersion Include="Microsoft.AspNetCore.Http" Version="2.2.2" />
<PackageVersion Include="Microsoft.Azure.Cosmos" Version="3.38.1" />
<!--When updating Microsoft.Data.SqlClient, update license URL in scripts/notice-generation.ps1-->
<PackageVersion Include="Microsoft.Data.SqlClient" Version="5.2.0" />
<PackageVersion Include="Microsoft.Data.SqlClient" Version="5.1.4" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please retain the updated 5.2.0 version

<PackageVersion Include="Microsoft.Extensions.Logging.ApplicationInsights" Version="2.22.0" />
<PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.5.0" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,8 @@ public void UniqueClaimsResolvedForDbPolicy_SessionCtx_Usage()
new("sub", "Aa_0RISCzzZ-abC1De2fGHIjKLMNo123pQ4rStUVWXY"),
new("oid", "55296aad-ea7f-4c44-9a4c-bb1e8d43a005"),
new(AuthenticationOptions.ROLE_CLAIM_TYPE, TEST_ROLE),
new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE2")
new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE2"),
new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE3")
};

//Add identity object to the Mock context object.
Expand All @@ -1315,6 +1316,7 @@ public void UniqueClaimsResolvedForDbPolicy_SessionCtx_Usage()
Assert.AreEqual(expected: "Aa_0RISCzzZ-abC1De2fGHIjKLMNo123pQ4rStUVWXY", actual: claimsInRequestContext["sub"], message: "Expected the sub claim to be present.");
Assert.AreEqual(expected: "55296aad-ea7f-4c44-9a4c-bb1e8d43a005", actual: claimsInRequestContext["oid"], message: "Expected the oid claim to be present.");
Assert.AreEqual(claimsInRequestContext[AuthenticationOptions.ROLE_CLAIM_TYPE], actual: TEST_ROLE, message: "The roles claim should have the value:" + TEST_ROLE);
Assert.AreEqual(expected: "[\"" + TEST_ROLE + "\",\"ROLE2\",\"ROLE3\"]", actual: claimsInRequestContext[AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE], message: "Original roles should be preserved in a new context");
}

/// <summary>
Expand Down Expand Up @@ -1365,7 +1367,7 @@ public void ValidateUnauthenticatedUserClaimsAreNotResolvedWhenProcessingUserCla
Dictionary<string, string> resolvedClaims = AuthorizationResolver.GetProcessedUserClaims(context.Object);

// Assert
Assert.AreEqual(expected: authenticatedUserclaims.Count, actual: resolvedClaims.Count, message: "Only two claims should be present.");
Assert.AreEqual(expected: authenticatedUserclaims.Count + 1, actual: resolvedClaims.Count, message: "Only " + (authenticatedUserclaims.Count + 1) + " claims should be present.");
Assert.AreEqual(expected: "openid", actual: resolvedClaims["scp"], message: "Unexpected scp claim returned.");

bool didResolveUnauthenticatedRoleClaim = resolvedClaims[AuthenticationOptions.ROLE_CLAIM_TYPE] == "Don't_Parse_This_Role";
Expand Down