JWT tokens can now be created from both SecurityTokenDescriptor.Subject and SecurityTokenDescriptor.Claims #1235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Also fixes #1086 |
| // If a key is present in both tokenDescriptor.Subject.Claims and tokenDescriptor.Claims, but the key is not the same case in both (e.g. "key" and "KEY"), | ||
| // the key present in tokenDescriptor.Subject.Claims is the one that will remain after the merge. However, the corresponding value will be overriden with the value | ||
| // that was present in tokenDescriptor.Claims. | ||
| if (tokenDescriptor.Claims != null) |
There was a problem hiding this comment.
nit: add logic ( tokenDescriptor.Claims != null && tokenDescriptor.Claims.Count is > 0 )
| @@ -54,6 +55,45 @@ public class JwtTokenUtilities | |||
| /// </summary> | |||
| public static Regex RegexJwe = new Regex(JwtConstants.JweCompactSerializationRegex, RegexOptions.Compiled | RegexOptions.CultureInvariant, TimeSpan.FromMilliseconds(100)); | |||
|
|
|||
| internal static Dictionary<string, object> CreateDictionaryFromClaims(IEnumerable<Claim> claims) | |||
| { | |||
| if (claims == null) | |||
There was a problem hiding this comment.
since this is an internal method, how about in both cases (claims == null) && foreach loop is never entered (there are no claims), just return empty dictionary.
| { | ||
| if (claim.ValueType == ClaimValueTypes.Boolean) | ||
| { | ||
| bool boolValue; |
There was a problem hiding this comment.
use the pattern in ParseTimeValue, if (double.TryParse(claim.Value, out int intValue))
| @@ -778,7 +795,66 @@ public static TheoryData<CreateTokenTheoryData> CreateJWSUsingSecurityTokenDescr | |||
| ValidIssuer = "Issuer" | |||
| } | |||
| }, | |||
|
|
|||
| new CreateTokenTheoryData // Test checks that values in SecurityTokenDescriptor.Subject.Claims | |||
There was a problem hiding this comment.
can we start comments on new line?
GeoK
reviewed
Aug 23, 2019
src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs
Outdated
Show resolved
Hide resolved
b6ad019 to
cb40a29
Compare
GeoK
approved these changes
Aug 23, 2019
cb40a29 to
d8875f5
Compare
…ct and SecurityTokenDescriptor.Claims
d8875f5 to
b39bca1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1193.
If both SecurityTokenDescriptor.Subject and SecurityTokenDescriptor.Claims are set, both are used when creating a JWT token. However, if duplicate values are present, the ones found in SecurityTokenDescriptor.Claims will override those found in SecurityTokenDescriptor.Subject.