Skip to content

Commit b41cfe5

Browse files
M4AlMu4allAniruddh25RubenCerna2079
committed
Fix Session Context Key set as Read_Only & maintain original roles from the JWT token (#2344)
## Why make this change? - Fixes #2341 - The session context in SQL server is `read_only = 1` which prevents users from doing multiple requests on the same connection. - The row level security is not accurately implemented when using a JWT token. ## What is this change? Changes the session context from `read_only = 1` to `read_only = 0` to allow multiple requests to be done in the same connection, Creates a copy of the original 'roles' from the JWT token to use it on the SQL Filter Predicate to accurately implement row level security. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests Updated `AuthorizationResolver` tests to ensure the original roles copy is working properly. ## Sample Request(s) Sample of a JWT token (only the relevant part) ``` { "aud": "api://ddcf6b31-5d01-407d-97cf-8efefc455d32", "iss": "https://sts.windows.net/9215c785-95c3-49b0-bdba-2062df5aedb5/", "roles": [ "user", "Allow_Customer_OPS025235", "Allow_Customer_OPS004095" ], "ver": "1.0" } ``` X-MS-API-ROLE: user before my change the extra 'roles' that do not match the X-MS-API-ROLE header would never reach the database context. With my change you can do things like this in SQL Predicates to filter out only subsets of the data: ``` CREATE FUNCTION dbo.ops_fact_order_Predicate(@CustomerNo varchar(max)) RETURNS TABLE WITH SCHEMABINDING AS RETURN SELECT 1 AS fn_securitypredicate_result WHERE @CustomerNo in ( select trim(replace(replace(replace([value], '"', ''), ']', ''), 'Allow_Customer_', '')) from STRING_SPLIT ( CAST(SESSION_CONTEXT(N'original_roles') as varchar(max)) , ',' , 0) where trim(replace(replace([value], '"', ''), ']', '')) like 'Allow_Customer%' ) CREATE SECURITY POLICY dbo.ops_fact_order_Policy ADD FILTER PREDICATE dbo.ops_fact_order_Predicate(CustomerNo) ON [gold_ops].[ops_fact_order]; ``` --------- Co-authored-by: KobeLenjou <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]> Co-authored-by: Ruben Cerna <[email protected]> Co-authored-by: RubenCerna2079 <[email protected]>
1 parent 4df624f commit b41cfe5

File tree

4 files changed

+12
-4
lines changed

4 files changed

+12
-4
lines changed

src/Config/ObjectModel/AuthenticationOptions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public record AuthenticationOptions(string Provider = nameof(EasyAuthType.Static
1717
public const string CLIENT_PRINCIPAL_HEADER = "X-MS-CLIENT-PRINCIPAL";
1818
public const string NAME_CLAIM_TYPE = "name";
1919
public const string ROLE_CLAIM_TYPE = "roles";
20+
public const string ORIGINAL_ROLE_CLAIM_TYPE = "original_roles";
2021

2122
/// <summary>
2223
/// Returns whether the configured Provider matches an

src/Core/Authorization/AuthorizationResolver.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,9 +617,14 @@ public static Dictionary<string, List<Claim>> GetAllAuthenticatedUserClaims(Http
617617
// into a list and storing that in resolvedClaims using the claimType as the key.
618618
foreach (Claim claim in identity.Claims)
619619
{
620-
// 'roles' claim has already been processed.
620+
// 'roles' claim has already been processed. But we preserve the original 'roles' claim.
621621
if (claim.Type.Equals(AuthenticationOptions.ROLE_CLAIM_TYPE))
622622
{
623+
if (!resolvedClaims.TryAdd(AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE, new List<Claim>() { claim }))
624+
{
625+
resolvedClaims[AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE].Add(claim);
626+
}
627+
623628
continue;
624629
}
625630

src/Core/Resolvers/MsSqlQueryExecutor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona
284284
string paramName = $"{SESSION_PARAM_NAME}{counter.Next()}";
285285
parameters.Add(paramName, new(claimValue));
286286
// Append statement to set read only param value - can be set only once for a connection.
287-
string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 1;";
287+
string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 0;";
288288
sessionMapQuery = sessionMapQuery.Append(statementToSetReadOnlyParam);
289289
}
290290

src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,7 +1293,8 @@ public void UniqueClaimsResolvedForDbPolicy_SessionCtx_Usage()
12931293
new("sub", "Aa_0RISCzzZ-abC1De2fGHIjKLMNo123pQ4rStUVWXY"),
12941294
new("oid", "55296aad-ea7f-4c44-9a4c-bb1e8d43a005"),
12951295
new(AuthenticationOptions.ROLE_CLAIM_TYPE, TEST_ROLE),
1296-
new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE2")
1296+
new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE2"),
1297+
new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE3")
12971298
};
12981299

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

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

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

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

0 commit comments

Comments
 (0)