Skip to content

Conversation

@seantleonard
Copy link
Contributor

Why make this change?

{  
 "identityProvider": "test",
 "userId": "12345",
 "userDetails": "john@contoso.com",
 "userRoles": ["role1", "role2", "author"],
 "claims": [{
   "typ": "SeriesId",
   "val": 10000
 }]
}
- Static Web Apps (By-Design) did not handle this because it's authenticated user payload does not include claims (see #843)
- AppService (By-Design) because it's claims payload (claim type and claim value) are both strings. So the payload will never have a claim with `val` is a non string type. The int type in the example above can never occur.
  • Even though the issue behavior was by design for EasyAuth, this led to discovering a very nuanced behavior for JWT authentication. The .NET type Claim is created during JWT token validation/processing by .NET libraries for each claim present in a JWT token. When the claim is created, the argument valueType is supplied by a value the .NET library- System.IdentityModel.Tokens.Jwt infers from the JSON type in the JWT token:
  • Because a Claim object has a property of valueType, our Authorization policy processing functionality must take into account that claims will not always be of type "string". While we had some processing of various non-string types, the additional types ClaimValueTypes.UInteger32, ClaimValueTypes.UInteger64:, ClaimValueTypes.Integer, JsonClaimValueTypes.JsonNull were added for completeness in GetODataCompliantClaimValue(Claim claim) in AuthorizationResolver.cs.
    • Note: this conversion of claim value types is performed to insert OData compliant literals into the OData filter clause that represents the authorization policy so that the OData Uri parser can properly parse and evaluate the policy.
      • Proper formatting is defined in OData ABNF 4.01: an example being that string literals must be single quoted in an OData filter clause like: userName.
      • The process of authorization policy processing is rehashed as follows because it is relevant to the code changes in this PR:
      • Authorization Policy: @claims.name eq @item.username
      • The GetODataCompliantClaimValue() method is responsible for knowing that the claim name is of valueType string, so then the policy is resolved as: 'myGitHubUsername eq username` (@item removed as it denotes a reference to a database column).

What is this change?

In addition to the OData Authorization policy processing of claim value types described above, some related but needed changes are included:

  • Rename/refactor some methods to make code more concise.
    • e.g. rename TryProcessDBPolicy to ProcessDBPolicy because method does not use the "try" semantics.
  • Remove old logic for processing Claim Short Names, as we now remove InboundClaimMapping in Startup.cs to eliminate references to legacy XML URI claim types. We now rely on the claim types (claim names) as presented in the JWT tokens directly.
  • Added more extensive comments to relevant methods so background info is readily available.

How was this tested?

  • Integration Tests
  • Unit Tests - added and updated existing

…onciseness combined some methods as split was unnecessary) and refactored for adherance to current engine behavior to not look for legacy claimTypes with URIs (ADFS/XML claimtypes)
…w behavior of a "Try" prefixed method name.

Added unit tests for each JSON ClaimValue type that would be present after SecurityJwtHandler parses valid JWT token and validates how policies are processed into OData compatible strings.
Copy link
Contributor

@ayush3797 ayush3797 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ayush3797 ayush3797 merged commit 6cb0770 into main Dec 5, 2022
@ayush3797 ayush3797 deleted the dev/seleonar/authZ_policy_claimTypes branch December 5, 2022 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check for more data types when parsing Claim Type Values from Access Tokens

4 participants