Added methods in JsonWebTokenHandler that allow custom JWT header claims to be added #1223

mafurman · 2019-07-18T23:22:28Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

brentschmaltz · 2019-07-23T14:54:43Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

+        /// <summary>
+        /// Encrypts a JWS.
+        /// </summary>
+        /// <param name="innerJwt">A 'JSON Web Token' (JWT) in JWS Compact Serialization Format.</param>


should we modify this parameter name and comment, we don't really know or verify that the string is a JWS in Compact Serialization Format.

I agree that the parameter name and comment should be changed, or perhaps maybe we should verify that the string is in JWS format?

In the RFC the value to be encrypted is referred to as 'plaintext', so we could call it that.

However, if something other than a JWS is passed into this method, the result produced by this method will no longer be something our library can verify. I feel like this may cause confusion.

The name of the method also happens to be 'EncryptToken', which may also get confusing.

brentschmaltz · 2019-07-23T15:01:32Z

src/Microsoft.IdentityModel.Tokens/SecurityTokenDescriptor.cs

+        /// will result in an exception being thrown. 
+        /// <remarks> These claims are only added to the outer header (in case of a JWE).</remarks>
+        /// </summary>
+        public IDictionary<string, object> AdditionalHeaderClaims { get; set; }


This property is only for JWT's. What will SAML handlers do with this?

I was thinking this would be a property that's only used when creating JWT tokens. I could make the comment clearer with regards to this if you think that'll help.

On a related note, IDictionary<string, object> Claims is currently not being used by the SAML handlers either.

this captures the issue for SAML and the dictionary: #1155

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

leastprivilege · 2019-07-26T06:35:08Z

I haven't looked at the API yet - but per our recent discussion, please make sure it is easily possible to set the typ header value.

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

brentschmaltz · 2019-08-12T21:55:59Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

+        {
+            var header = new JObject();
+
+            if (!string.IsNullOrEmpty(encryptingCredentials.Alg))


Alg, Enc can not be null.

azure-activedirectory-identitymodel-extensions-for-dotnet/src/Microsoft.IdentityModel.Tokens/EncryptingCredentials.cs

Line 100 in b16a4d5

private set => _alg = string.IsNullOrEmpty(value) ? throw LogHelper.LogArgumentNullException("alg") : value;

azure-activedirectory-identitymodel-extensions-for-dotnet/src/Microsoft.IdentityModel.Tokens/EncryptingCredentials.cs

Line 109 in b16a4d5

private set => _enc = string.IsNullOrEmpty(value) ? throw LogHelper.LogArgumentNullException("enc") : value;

Good point! Fixed.

brentschmaltz · 2019-08-12T21:57:08Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

+            var header = new JObject
+                {
+                    { JwtHeaderParameterNames.Alg, signingCredentials.Algorithm },
+                    { JwtHeaderParameterNames.Kid, signingCredentials.Key.KeyId },


KeyId can be null.

Fixed. Also added in a test for this.

brentschmaltz · 2019-08-12T22:07:38Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

@@ -199,7 +269,7 @@ public virtual string CreateToken(SecurityTokenDescriptor tokenDescriptor)
            if (!payload.Any())


#1237 was opened as an empty payload should be allowed.

Fixed. Also added in some tests.

brentschmaltz · 2019-08-12T22:12:45Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

+        /// <exception cref="ArgumentNullException">if <paramref name="compressionAlgorithm"/> is null.</exception>
+        /// <exception cref="ArgumentNullException">if <paramref name="additionalHeaderClaims"/> is null.</exception>
+        /// <returns>A JWE in compact serialization format.</returns>
+        public virtual string CreateToken(string payload, SigningCredentials signingCredentials, EncryptingCredentials encryptingCredentials, string compressionAlgorithm, IDictionary<string, object> additionalHeaderClaims)


need to add a comment to capture the SecurityTokenException thrown if any default header claims are found.

Good catch! I fixed this in all the relevant methods.

brentschmaltz · 2019-08-12T22:13:25Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

+        /// <exception cref="ArgumentNullException">if <paramref name="signingCredentials"/> is null.</exception>
+        /// <exception cref="ArgumentNullException">if <paramref name="additionalHeaderClaims"/> is null.</exception>
+        /// <returns>A JWS in Compact Serialization Format.</returns>
+        public virtual string CreateToken(string payload, SigningCredentials signingCredentials, IDictionary<string, object> additionalHeaderClaims)


need to add a comment to capture the SecurityTokenException thrown if any default header claims are found.

brentschmaltz · 2019-08-12T22:16:06Z

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

            var rawHeader = Base64UrlEncodedUnsignedJWSHeader;
-            if (signingCredentials != null && !JsonWebTokenManager.KeyToHeaderCache.TryGetValue(JsonWebTokenManager.GetHeaderCacheKey(signingCredentials), out rawHeader))
+            // If there's no additional header claims to be added to the header and the token will be signed, try to retrieve a header value from the cache.
+            if (additionalHeaderClaims == null && signingCredentials != null && !JsonWebTokenManager.KeyToHeaderCache.TryGetValue(JsonWebTokenManager.GetHeaderCacheKey(signingCredentials), out rawHeader))


(nit) could add additionalHeaderClaims.Count > 0

Do you mean that we should check for this before seeing if 'additionalHeaderClaims' intersects _defaultHeaderParameters? Or that we should check for this so that we still use the header cache if 'additionalHeaderClaims' is empty?

I added in a check in both places.

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs

…ims to be added

steveoshima · 2019-10-06T20:42:17Z

FYI, this currently isnt working with RSA-PSS support.

GeoK · 2019-10-07T17:33:49Z

Hi @steveoshima - It's not clear what exactly is not working. Can you provide more information or/and a small repro project?

GeoK · 2019-10-15T17:30:07Z

@steveoshima - Are you still hitting the issue when using RSA-PSS?
We are planning on releasing 5.6.0 soon and we would appreciate your input.

steveoshima · 2019-10-16T17:56:10Z

@steveoshima - Are you still hitting the issue when using RSA-PSS?
We are planning on releasing 5.6.0 soon and we would appreciate your input.

I attempted to check different branches to get RSA-PSS with custom headers working last week. However, no branches worked that i tried. I spotted the cusom header test with RSA-PSS was incomplete. I messaged about this on the issue. See my comment here - #1210

You will see the test is missing additonal header setup data, if this is added im sure the error will occur. -

azure-activedirectory-identitymodel-extensions-for-dotnet/test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenHandlerTests.cs

Line 1347 in 1c16adc

#if NET461 || NET_CORE

mafurman requested review from GeoK, brentschmaltz and henrik-me July 18, 2019 23:22

GeoK requested changes Jul 19, 2019

View reviewed changes

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Show resolved Hide resolved

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Outdated Show resolved Hide resolved

brentschmaltz reviewed Jul 23, 2019

View reviewed changes

mafurman force-pushed the mafurman/customJwtHeader branch from 6e43d3c to 0f506ac Compare July 23, 2019 20:28

GeoK requested changes Jul 25, 2019

View reviewed changes

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Outdated Show resolved Hide resolved

GeoK reviewed Aug 5, 2019

View reviewed changes

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Outdated Show resolved Hide resolved

mafurman force-pushed the mafurman/customJwtHeader branch from 9696e13 to ef50621 Compare August 7, 2019 00:33

GeoK reviewed Aug 8, 2019

View reviewed changes

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Outdated Show resolved Hide resolved

GeoK approved these changes Aug 8, 2019

View reviewed changes

brentschmaltz reviewed Aug 12, 2019

View reviewed changes

brentschmaltz approved these changes Aug 16, 2019

View reviewed changes

GeoK reviewed Aug 23, 2019

View reviewed changes

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Show resolved Hide resolved

GeoK reviewed Aug 23, 2019

View reviewed changes

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs Outdated Show resolved Hide resolved

mafurman force-pushed the mafurman/customJwtHeader branch 2 times, most recently from 1e3a6da to e5f426e Compare August 23, 2019 19:12

GeoK approved these changes Aug 23, 2019

View reviewed changes

mafurman force-pushed the mafurman/customJwtHeader branch from e5f426e to 5a2deab Compare August 27, 2019 00:03

mafurman force-pushed the mafurman/customJwtHeader branch from 5a2deab to 9df81a5 Compare September 5, 2019 21:33

Added methods in JsonWebTokenHandler that allow custom JWT header cla…

f9746b6

…ims to be added

mafurman force-pushed the mafurman/customJwtHeader branch from 9df81a5 to f9746b6 Compare September 6, 2019 01:23

mafurman merged commit be4e090 into dev Sep 6, 2019

GeoK mentioned this pull request Oct 16, 2019

Allow creation of a customized header using JsonWebTokenHandler #1210

Closed

mafurman deleted the mafurman/customJwtHeader branch September 28, 2021 01:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added methods in JsonWebTokenHandler that allow custom JWT header claims to be added #1223

Added methods in JsonWebTokenHandler that allow custom JWT header claims to be added #1223

mafurman commented Jul 18, 2019 •

edited

Loading

brentschmaltz Jul 23, 2019

mafurman Jul 23, 2019

brentschmaltz Jul 23, 2019

mafurman Jul 23, 2019

brentschmaltz Aug 16, 2019 •

edited

Loading

leastprivilege commented Jul 26, 2019

brentschmaltz Aug 12, 2019

mafurman Aug 14, 2019

brentschmaltz Aug 12, 2019

mafurman Aug 14, 2019 •

edited

Loading

brentschmaltz Aug 12, 2019

mafurman Aug 14, 2019

brentschmaltz Aug 12, 2019

mafurman Aug 14, 2019

brentschmaltz Aug 12, 2019

mafurman Aug 14, 2019

brentschmaltz Aug 12, 2019

mafurman Aug 14, 2019

steveoshima commented Oct 6, 2019

GeoK commented Oct 7, 2019

GeoK commented Oct 15, 2019

steveoshima commented Oct 16, 2019 •

edited

Loading

		@@ -199,7 +269,7 @@ public virtual string CreateToken(SecurityTokenDescriptor tokenDescriptor)
		if (!payload.Any())

Added methods in JsonWebTokenHandler that allow custom JWT header claims to be added #1223

Added methods in JsonWebTokenHandler that allow custom JWT header claims to be added #1223

Conversation

mafurman commented Jul 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brentschmaltz Aug 16, 2019 • edited Loading

Choose a reason for hiding this comment

leastprivilege commented Jul 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mafurman Aug 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveoshima commented Oct 6, 2019

GeoK commented Oct 7, 2019

GeoK commented Oct 15, 2019

steveoshima commented Oct 16, 2019 • edited Loading

mafurman commented Jul 18, 2019 •

edited

Loading

brentschmaltz Aug 16, 2019 •

edited

Loading

mafurman Aug 14, 2019 •

edited

Loading

steveoshima commented Oct 16, 2019 •

edited

Loading