Adding generation of accessors for equivalent claims #1829

jmprieur · 2022-03-24T02:25:57Z

Adding generation of accessors for claims independently of AAD token version of the token
See #1800

Developer experience:

Add a using:

using Microsoft.IdentityModel.Tokens.Jwt,

In the code, on a ClaimsPrincipal or an IdentityPrincipal instance, you will get access to extension methods that retrieve the claims in the instance independently of them being long or short claims (which depends on the token version: v1.0 or v2.0 tokens). Developers will also get completion telling them if the claim is safe or not for use in authorization policies, and what privacy category they belong to

See also the description of the accessors:

ClaimsAccessorDoc.md

Design

This PR a bit different from others: It uses code generation
The claims are described in ClaimsKnowledge.tti (Text Template Include file)
This include file is included in:

ClaimTypeMapping.tt, which generates ClaimTypeMapping.cs (which was already existing),
ClaimsTypeAccessor.tt, which generates ClaimsTypeAccessor.gen.cs, which contains a partial class implementation of ClaimsTypeAccessor, the other partial being in ClaimsTypeAccessor.cs
ClaimsAccessorDoc.tt, which generates ClaimsAccessorDoc.md

AAD token version of the token See #1800

jmprieur

Added a few explanations

jmprieur · 2022-03-24T02:33:10Z

src/System.IdentityModel.Tokens.Jwt/ClaimTypeMapping.tt

@@ -0,0 +1,115 @@
+<#@ template debug="false" hostspecific="false" language="C#" #>


This is a T4 file that generates the ClaimTypeMapping.cs from the ClaimsKnowledge.tti.

jmprieur · 2022-03-24T02:34:13Z

src/System.IdentityModel.Tokens.Jwt/ClaimsKnowledge.tti

@@ -0,0 +1,141 @@
+<#+
+class ClaimsKnowledge


This class is not product code, but control code to generate the product code.
This data structure describes the claims information.

I am not understanding why we need a template model.
I have used .tt's when using typescript that will generate different jscript mapped to a release.

How does the tti help us?

It avoids writing 5000 lines of code

@jmprieur i will educate myself about this technique.

I can spend 10 minutes with you @brentschmaltz if you wish. Also you might want to install this Visual Studio extension: https://marketplace.visualstudio.com/items?itemName=bricelam.T4Language

@jmprieur Would this require users to install this extension to build?

no. not at all. It's an editor extension so that the syntax coloring help understanding the T4 code

jmprieur · 2022-03-24T02:37:38Z

src/System.IdentityModel.Tokens.Jwt/System.IdentityModel.Tokens.Jwt.csproj

+    </None>
+    <None Update="ClaimTypeAccessor.tt">
+      <LastGenOutput>ClaimTypeAccessor.gen.cs</LastGenOutput>
+      <Generator>TextTemplatingFileGenerator</Generator>


When you save the ClaimTypeAccessor.tt this generates the ClaimTypeAccessor.gen.cs which contain a partial class implementation of ClaimTypeAccessor.cs

src/System.IdentityModel.Tokens.Jwt/ClaimTypeMapping.cs

brentschmaltz · 2022-03-24T17:28:34Z

src/System.IdentityModel.Tokens.Jwt/ClaimsKnowledge.tti

+        new ClaimsKnowledge("Group", true, false, true, "", "\"http://schemas.xmlsoap.org/claims/Group\"", "\"group\""),
+        new ClaimsKnowledge("GroupsId", true, false, true, "", "ClaimTypes.GroupSid", "\"groupsid\""),
+        new ClaimsKnowledge("IdTyp", false, false, true, "", "\"idtyp\""),
+        new ClaimsKnowledge("Idp", false, false, true, "", "\"http://schemas.microsoft.com/identity/claims/identityprovider\"", "\"idp\""),


How do we know 'idp' (or any other claim) is or is not 'usableForAuthorization' ?

It might be. I've started a conversation with a few people about what claim should be. (same for the privacy)

- Adding indication of safety for authorization on all claims

src/System.IdentityModel.Tokens.Jwt/ClaimsAccessorsDoc.md

kalyankrishna1 · 2022-03-25T07:56:10Z

src/System.IdentityModel.Tokens.Jwt/ClaimsAccessorsDoc.md

+string | GetTenantId | True | OII | "tid"<BR/> "http://schemas.microsoft.com/identity/claims/tenantid"
+string | GetAcr | False |  | "acr"<BR/> "http://schemas.microsoft.com/claims/authnclassreference"
+string | GetAdfs1Email | False | EUPI | "adfs1email"<BR/> "http://schemas.xmlsoap.org/claims/EmailAddress"
+string | GetAdfs1Upn | False | EUPI | "adfs1upn"<BR/> "http://schemas.xmlsoap.org/claims/UPN"


GetAdfs1Upn

Wouldn't this cause confusion as it references a claim like upn which will almost always be present in tokens issued to users?
It might give n incorrect impression that the user authenticated with ADFS, even if they didn't.

I don't know what this claim is. Maybe it's only available when users logged with UPN

src/System.IdentityModel.Tokens.Jwt/ClaimsAccessorsDoc.md

kalyankrishna1 · 2022-03-25T07:58:15Z

src/System.IdentityModel.Tokens.Jwt/ClaimsAccessorsDoc.md

+string | GetObjectId | True | EUPI | "oid"<BR/> "http://schemas.microsoft.com/identity/claims/objectidentifier"
+IEnumerable<string> | GetScopes | True |  | "scp"<BR/> "http://schemas.microsoft.com/identity/claims/scope"
+string | GetTenantId | True | OII | "tid"<BR/> "http://schemas.microsoft.com/identity/claims/tenantid"
+string | GetAcr | False |  | "acr"<BR/> "http://schemas.microsoft.com/claims/authnclassreference"


Acr

there is also acrs, and these two acr and acrs cause enough confusion. So my suggestion would be to use full name if possible , like GetAuthncClassReference

@kalyankrishna1: do you mean GetAuthenticationClassReference?

src/System.IdentityModel.Tokens.Jwt/ClaimsAccessorsDoc.md

kalyankrishna1 · 2022-03-25T08:03:09Z

src/System.IdentityModel.Tokens.Jwt/ClaimsAccessorsDoc.md

+IEnumerable<string> | GetRole | True |  | "role"<BR/> "roles"<BR/> ClaimTypes.Role
+string | GetSid | False |  | "sid"
+string | GetUpn | False |  | "upn"<BR/> ClaimTypes.Upn
+string | GetWinaccountname | False |  | "winaccountname"<BR/> ClaimTypes.WindowsAccountName


GetWinaccountname

EUPI?

brentschmaltz · 2022-03-25T16:00:03Z

src/System.IdentityModel.Tokens.Jwt/ClaimTypeAccessor.tt

@@ -0,0 +1,196 @@
+<#@ template debug="false" hostspecific="false" language="C#" #>


A couple of general concerns:

some of the categories seems to be AAD specific, IdentityModel has always tried to be neutral?

can users set their own categories?

there is no general spec for the category of claims, how can one say claim 'a' is in a category in general?

Do we want to move this into SAL?

The categories are privacy categories (well defined. could almost be an enum). For 3) will follow-up offline

…m/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet into jmprieur/ClaimsMappingImprovement

brentschmaltz · 2022-04-06T16:14:40Z

src/Microsoft.IdentityModel.Aad/ClaimsTypeAccessor.cs

+using System.Security.Claims;
+using System.Linq;
+
+namespace System.IdentityModel.Aad


The ns is System.IdentityModel.Aad, but folder and file names are Microsoft.IdentityModel.*

All our new work uses Microsoft.IdentityModel.*

Thank @brentschmaltz: I fixed the generators (which fixed the code)
a38cc2f

brentschmaltz · 2022-04-11T15:03:47Z

@jmprieur this statement needs some adjustment:

that retrieve the claims in the instance independently of them being long or short claims (which depends on the token version: v1.0 or v2.0 tokens)

long or short claim types is not a result of token version, it is a result of the claims mapping in the JwtSecurityTokenHandler.
AAD v1.0 and v2.0 have different claims that represent the same concept see access_tokens..

For example: v1.appid <-> v2.azp, v1.appidacr <-> v2. azpacr

brentschmaltz · 2022-04-11T15:08:57Z

src/Microsoft.IdentityModel.Aad/ClaimsAccessorsDoc.md

+string | GetAuthenticationContextClassReference | False |  | acr<BR/> http://schemas.microsoft.com/claims/authnclassreference
+string | GetAdfs1Email | False | EUPI | adfs1email<BR/> http://schemas.xmlsoap.org/claims/EmailAddress
+string | GetAdfs1Upn | False | EUPI | adfs1upn<BR/> http://schemas.xmlsoap.org/claims/UPN
+string | GetAmr | False |  | amr<BR/> http://schemas.microsoft.com/claims/authnmethodsreferences


amr is definitely used in Authorization as this defines how the subject authenticated with the identity provider.

brentschmaltz · 2022-04-11T15:10:10Z

src/Microsoft.IdentityModel.Aad/ClaimsAccessorsDoc.md

+
+Type | Accessor | Used for authorization | Privacy | Claims
+--   | --       | --                     | --      | --     
+string | GetActor | False |  | actort<BR/> http://schemas.xmlsoap.org/ws/2009/09/identity/claims/actor


i think actor is PII as the return is another token.

brentschmaltz · 2022-04-11T15:14:06Z

src/Microsoft.IdentityModel.Aad/ClaimsTypeAccessor.cs

+        /// <returns>All the values of the claims or its equivalents.</returns>
+        public static IEnumerable<string> GetAllEquivalentClaims(this ClaimsPrincipal user, string claimType)
+        {
+            if (user is null)


we have been using the coding style:

_ = user ?? throw new ArgumentNullException();

brentschmaltz · 2022-04-11T15:14:26Z

src/Microsoft.IdentityModel.Aad/ClaimsTypeAccessor.cs

+        {
+            if (user is null)
+            {
+                throw new ArgumentNullException(nameof(user));


How will this plug into our logging model?

brentschmaltz · 2022-04-11T15:17:15Z

src/Microsoft.IdentityModel.Aad/ClaimsTypeAccessor.cs

+
+            if (string.IsNullOrEmpty(claimType))
+            {
+                throw new ArgumentException($"'{nameof(claimType)}' cannot be null or empty.", nameof(claimType));


We have been using a single location for log messages, each with an identifier that allows for quickly identifying the assembly and location of the issue.
see: LogMessages

brentschmaltz · 2022-04-11T15:35:27Z

src/Microsoft.IdentityModel.Aad/ClaimsTypeAccessor.cs

+        /// <summary> 
+        /// Verifies that the token has any of the claims required. 
+        /// </summary> 
+        /// <param name="user">claims identity</param> 


this parameter is a ClaimsPrincipal.
we may want to write:

/// <param name="claimsPrincipal"> the <see cref="ClaimsPrincipal"/> to check for a claim /// of one of the requiredClaims.</param> This comment applies to methods below.

brentschmaltz · 2022-04-11T15:42:04Z

src/Microsoft.IdentityModel.Aad/ClaimsTypeAccessor.cs

+        /// <param name="user">Claims principal</param>
+        /// <param name="claimType">Claims type</param>
+        /// <returns></returns>
+        public static string GetEquivalentClaim(this ClaimsPrincipal user, string claimType)


This method is not returning a Claim, but a Claim.Value (which may be a complex type, bool, int, null).
In this case, the user will not know the type of the Value as that is contained in Claim.ValueType.

One option is to return a Claim.
Our JWT -> Claim logic is here.

JsonWebToken has added TryGetPayloadValue to try and deal with this.

jmprieur added 3 commits March 23, 2022 19:25

Adding generation of accessors for claims independently of

049c4fd

AAD token version of the token See #1800

Fixing spacing

bbdaf02

Fixing a comment.

2b89eea

jmprieur commented Mar 24, 2022

View reviewed changes

brentschmaltz reviewed Mar 24, 2022

View reviewed changes

src/System.IdentityModel.Tokens.Jwt/ClaimTypeMapping.cs Outdated Show resolved Hide resolved

brentschmaltz reviewed Mar 24, 2022

View reviewed changes

jmprieur added 3 commits March 24, 2022 16:05

Remove the fmt claim from the mapping.

bb86cda

Adding a generator for some documentation

8ae45d9

- Fixing names form a few claims accessors

c9f408d

- Adding indication of safety for authorization on all claims

jmprieur changed the title ~~Adding generation of accessors for claims independently of~~ Adding generation of accessors for equivalent claims Mar 25, 2022