Skip to content

Commit

Permalink
Add audiences to security token descriptor (#2575)
Browse files Browse the repository at this point in the history
* added SecurityTokenDescriptor.Audiences and refactored code to support

* refactored Audience vs Audiences logic

* adjusted Audiences validation logic to more accurately evaluate for null or empty strings

* Redesigned to avoid adding internal members and to support using both Audience and Audiences simultaneously

* Fixed json formatting of audiences

* Wrote unit tests for JsonWebTokenHandler jws and jwe

* fixed bug in JwtSecurityTokenHandler

* added test for audiences validation

* added missing brackets

* refactor WriteObject for readability and change IList case to IEnumerable

* Add public method AddAudience for inexpensive deduplication when adding values

* moved Audiences injection for JwtSecurityTokenHandler to the claims dictionary

* added error msg

* added unit tests ensuring same correct behavior for JwtSecurityTokenHandler and JsonWebTokenHandler

* restoring existing note

* samlv1 unit tests

* saml2 unit tests

* changed logic to avoid altering the claims object in the securityTokenDescriptor

* added a couple more unit tests

* private method renamed for accuracy

* Added benchmarks to look at the performance of Audiences Vs Audience Members

* removing use of 'collection expressions' as they don't work in ADO build

* Redesigned Audiences to use IList

* removing unneeded string

* Add constructor overload to maintain public api

* changed serializer back to using IList

* removed unneeded using

* re-adding enumerable to switch

* reverting change to IList in jsonserializerprimitives

* added a method to concat Audience and Audiences when writing to json

* added duplicate check

* changed _audiences private member to List<string>

* Changed IEnumerable back to IList

* altered logging logic to avoid unneeded alloc

* removed Linq where from hotpath

* formatting fixes/changes

* Added UriKind.Absolute

* removed extra space

* Removed AddAudiences method

* Added API taking multiple Audiences but not single one for completeness

* added details on Aud claim priority to method summary

* fixed bug and made variable names more clear

* set up tests to track expected behavior

* changed variables to original names

* reverted changes to WriteObject

* formatting changes

* small changes from PR feedback

* syntax fix

* changed IsNullOrWhitespace to IsNullOrEmpty

* reverted change to test since incepting code change was reverted

* removed unnecessary code leftover from old solutions

* replaced linq with foreach

* added null check as a result of dropping Linq usage

* fixing accidental comment edit

* removed unnecessary local string

* refactored to only create one list when making SamlAudienceRestrictionCondition

* reduced list allocation from one to zero or one

* removing duplicate methods

* removed unneeded using

* add note to features section in changelog

* Changes per latest PR comments

* adjusted logic to use local var since ICollection can't return last item without iterating through the entire collection

* fixed which test was first in theory data
  • Loading branch information
JoshLozensky authored Jun 22, 2024
1 parent c24bfe6 commit 55cc10e
Show file tree
Hide file tree
Showing 19 changed files with 792 additions and 52 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ See the [releases](https://github.com/AzureAD/azure-activedirectory-identitymode
7.6.1
=====
### New Features:
- Added an Audiences member to the SecurityTokenDescriptor to make it easier to define multiple audiences in JWT and SAML tokens. Addresses issue [#1479](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/1479) with PR [#2575](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2575)
- Add missing metadata parameters to OpenIdConnectConfiguration. See issue [#2498](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2498) for details.


### Bug Fixes:
- Fix over-reporting of `IDX14100`. See issue [#2058](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2058) and PR [#2618](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2618) for details.
- `JwtRegisteredClaimNames` now contains previously missing Standard OpenIdConnect claims. See issue [#1598](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/1598) for details.
Expand Down
45 changes: 45 additions & 0 deletions benchmark/Microsoft.IdentityModel.Benchmarks/BenchmarkUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ public class BenchmarkUtils

public const string Audience = "http://www.contoso.com/protected";

public readonly static IList<string> Audiences = new string[] {
"http://www.contoso.com/protected",
"http://www.contoso.com/protected1",
"http://www.contoso.com/protected2",
"http://www.contoso.com/protected3",
"http://www.contoso.com/protected4"
};

private static RSA _rsa;
private static SymmetricSecurityKey _symmetricKey;

Expand Down Expand Up @@ -60,6 +68,43 @@ public static Dictionary<string, object> Claims
}
}

public static Dictionary<string, object> ClaimsNoAudience
{
get
{
DateTime now = DateTime.UtcNow;
return new Dictionary<string, object>()
{
{ "role", new List<string>() { "role1", "Developer", "Sales"} },
{ JwtRegisteredClaimNames.Email, "[email protected]" },
{ JwtRegisteredClaimNames.Exp, EpochTime.GetIntDate(now + TimeSpan.FromDays(1)) },
{ JwtRegisteredClaimNames.Nbf, EpochTime.GetIntDate(now) },
{ JwtRegisteredClaimNames.Iat, EpochTime.GetIntDate(now) },
{ JwtRegisteredClaimNames.GivenName, "Bob" },
{ JwtRegisteredClaimNames.Iss, Issuer },
};
}
}

public static Dictionary<string, object> ClaimsMultipleAudiences
{
get
{
DateTime now = DateTime.UtcNow;
return new Dictionary<string, object>()
{
{ "role", new List<string>() { "role1", "Developer", "Sales"} },
{ JwtRegisteredClaimNames.Email, "[email protected]" },
{ JwtRegisteredClaimNames.Exp, EpochTime.GetIntDate(now + TimeSpan.FromDays(1)) },
{ JwtRegisteredClaimNames.Nbf, EpochTime.GetIntDate(now) },
{ JwtRegisteredClaimNames.Iat, EpochTime.GetIntDate(now) },
{ JwtRegisteredClaimNames.GivenName, "Bob" },
{ JwtRegisteredClaimNames.Iss, Issuer },
{ JwtRegisteredClaimNames.Aud, Audiences }
};
}
}

public static Dictionary<string, object> ClaimsExtendedExample
{
get
Expand Down
44 changes: 43 additions & 1 deletion benchmark/Microsoft.IdentityModel.Benchmarks/CreateTokenTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ public class CreateTokenTests
{
private JsonWebTokenHandler _jsonWebTokenHandler;
private SecurityTokenDescriptor _tokenDescriptor;
private SecurityTokenDescriptor _tokenDescriptorMultipleAudiencesMemberAndClaims;
private SecurityTokenDescriptor _tokenDescriptorMultipleAudiencesMemberOnly;
private SecurityTokenDescriptor _tokenDescriptorSingleAudienceUsingAudiencesMember;

[GlobalSetup]
public void Setup()
Expand All @@ -23,11 +26,50 @@ public void Setup()
_tokenDescriptor = new SecurityTokenDescriptor
{
Claims = BenchmarkUtils.Claims,
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256,
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256
};

_tokenDescriptorSingleAudienceUsingAudiencesMember = new SecurityTokenDescriptor
{
Claims = BenchmarkUtils.ClaimsNoAudience,
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256
};

_tokenDescriptorMultipleAudiencesMemberOnly = new SecurityTokenDescriptor
{
Claims = BenchmarkUtils.ClaimsNoAudience,
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256
};

_tokenDescriptorMultipleAudiencesMemberAndClaims = new SecurityTokenDescriptor
{
Claims = BenchmarkUtils.ClaimsMultipleAudiences,
SigningCredentials = BenchmarkUtils.SigningCredentialsRsaSha256
};

_tokenDescriptorSingleAudienceUsingAudiencesMember.Audiences.Add(BenchmarkUtils.Audience);
foreach (var audience in BenchmarkUtils.Audiences)
{
_tokenDescriptorMultipleAudiencesMemberOnly.Audiences.Add(audience);
_tokenDescriptorMultipleAudiencesMemberAndClaims.Audiences.Add(audience);
}
}

[Benchmark]
public string JsonWebTokenHandler_CreateToken() => _jsonWebTokenHandler.CreateToken(_tokenDescriptor);

[Benchmark]
public string JsonWebTokenHandler_CreateToken_SingleAudienceUsingAudiencesMemberOnly() =>
_jsonWebTokenHandler.CreateToken(_tokenDescriptorSingleAudienceUsingAudiencesMember);

[Benchmark]
public string JsonWebTokenHandler_CreateToken_MultipleAudiencesMemberOnly() =>
_jsonWebTokenHandler.CreateToken(_tokenDescriptorMultipleAudiencesMemberOnly);

[Benchmark]
public string JsonWebTokenHandler_CreateToken_MultipleAudiencesMemberAndClaims() =>
_jsonWebTokenHandler.CreateToken(_tokenDescriptorMultipleAudiencesMemberAndClaims);


}
}
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,10 @@ int sizeOfEncodedHeaderAndPayloadAsciiBytes
/// <summary>
/// A <see cref="SecurityTokenDescriptor"/> can contain claims from multiple locations.
/// This method consolidates the claims and adds default times {exp, iat, nbf} if needed.
/// In the case of a claim from this set: {Audience, Issuer, Expires, IssuedAt, NotBefore} being defined in multiple
/// locations in the SecurityTokenDescriptor, the following priority is used:
/// SecurityTokenDescriptor.{Audience/Audiences, Issuer, Expires, IssuedAt, NotBefore} > SecurityTokenDescriptor.Claims >
/// SecurityTokenDescriptor.Subject.Claims
/// </summary>
/// <param name="writer">The <see cref="Utf8JsonWriter"/> to use.</param>
/// <param name="tokenDescriptor">The <see cref="SecurityTokenDescriptor"/> used to create the token.</param>
Expand All @@ -706,11 +710,20 @@ internal static void WriteJwsPayload(

writer.WriteStartObject();

if (!string.IsNullOrEmpty(tokenDescriptor.Audience))
if (tokenDescriptor.Audiences.Count > 0)
{
if (!tokenDescriptor.Audience.IsNullOrEmpty())
JsonPrimitives.WriteStrings(ref writer, JwtPayloadUtf8Bytes.Aud, tokenDescriptor.Audiences, tokenDescriptor.Audience);
else
JsonPrimitives.WriteStrings(ref writer, JwtPayloadUtf8Bytes.Aud, tokenDescriptor.Audiences);

audienceSet = true;
}
else if (!tokenDescriptor.Audience.IsNullOrEmpty())
{
writer.WritePropertyName(JwtPayloadUtf8Bytes.Aud);
writer.WriteStringValue(tokenDescriptor.Audience);
audienceSet = true;
}

if (!string.IsNullOrEmpty(tokenDescriptor.Issuer))
Expand Down Expand Up @@ -742,7 +755,7 @@ internal static void WriteJwsPayload(
}

// Duplicates are resolved according to the following priority:
// SecurityTokenDescriptor.{Audience, Issuer, Expires, IssuedAt, NotBefore}, SecurityTokenDescriptor.Claims, SecurityTokenDescriptor.Subject.Claims
// SecurityTokenDescriptor.{Audience/Audiences, Issuer, Expires, IssuedAt, NotBefore}, SecurityTokenDescriptor.Claims, SecurityTokenDescriptor.Subject.Claims
// SecurityTokenDescriptor.Claims are KeyValuePairs<string,object>, whereas SecurityTokenDescriptor.Subject.Claims are System.Security.Claims.Claim and are processed differently.

if (tokenDescriptor.Claims != null && tokenDescriptor.Claims.Count > 0)
Expand All @@ -755,7 +768,15 @@ internal static void WriteJwsPayload(
if (audienceSet)
{
if (LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(LogHelper.FormatInvariant(LogMessages.IDX14113, LogHelper.MarkAsNonPII(nameof(tokenDescriptor.Audience))));
{
string descriptorMemberName = null;
if (tokenDescriptor.Audiences.Count > 0)
descriptorMemberName = nameof(tokenDescriptor.Audiences);
else if (!string.IsNullOrEmpty(tokenDescriptor.Audience))
descriptorMemberName = nameof(tokenDescriptor.Audience);

LogHelper.LogInformation(LogHelper.FormatInvariant(LogMessages.IDX14113, LogHelper.MarkAsNonPII(descriptorMemberName)));
}

continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Linq;
using System.Security.Claims;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using Microsoft.IdentityModel.Abstractions;
Expand Down Expand Up @@ -354,7 +353,7 @@ protected virtual IEnumerable<ClaimsIdentity> CreateClaimsIdentities(SamlSecurit
/// <exception cref="ArgumentNullException">if <paramref name="tokenDescriptor"/> is null.</exception>
protected virtual SamlConditions CreateConditions(SecurityTokenDescriptor tokenDescriptor)
{
if (null == tokenDescriptor)
if (tokenDescriptor == null)
throw LogArgumentNullException(nameof(tokenDescriptor));

var conditions = new SamlConditions();
Expand All @@ -368,12 +367,41 @@ protected virtual SamlConditions CreateConditions(SecurityTokenDescriptor tokenD
else if (SetDefaultTimesOnTokenCreation)
conditions.NotOnOrAfter = DateTime.UtcNow + TimeSpan.FromMinutes(TokenLifetimeInMinutes);

if (!string.IsNullOrEmpty(tokenDescriptor.Audience))
if (tokenDescriptor.Audiences.Count > 0)
{
if (!tokenDescriptor.Audience.IsNullOrEmpty())
conditions.Conditions.Add(CreateAudienceRestrictionCondition(tokenDescriptor.Audience, tokenDescriptor.Audiences));
else
conditions.Conditions.Add(CreateAudienceRestrictionCondition(tokenDescriptor.Audiences));
}
else if (!tokenDescriptor.Audience.IsNullOrEmpty())
{
conditions.Conditions.Add(new SamlAudienceRestrictionCondition(new Uri(tokenDescriptor.Audience)));
}

return conditions;
}


private static SamlAudienceRestrictionCondition CreateAudienceRestrictionCondition(IList<string> audiences)
{
SamlAudienceRestrictionCondition audRestrictionCondition = new();
for (int i = 0; i < audiences.Count; i++)
audRestrictionCondition.Audiences.Add(new Uri(audiences[i]));

return audRestrictionCondition;
}

private static SamlCondition CreateAudienceRestrictionCondition(string audience, IList<string> audiences)
{
SamlAudienceRestrictionCondition audRestrictionCondition = new(new Uri(audience));
for (int i = 0; i < audiences.Count; i++)
audRestrictionCondition.Audiences.Add(new Uri(audiences[i]));

return audRestrictionCondition;
}


/// <summary>
/// Generates an enumeration of SamlStatements from a SecurityTokenDescriptor.
/// Only SamlAttributeStatements and SamlAuthenticationStatements are generated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,14 @@ protected virtual Saml2Conditions CreateConditions(SecurityTokenDescriptor token
else if (SetDefaultTimesOnTokenCreation)
conditions.NotOnOrAfter = DateTime.UtcNow + TimeSpan.FromMinutes(TokenLifetimeInMinutes);

if (!string.IsNullOrEmpty(tokenDescriptor.Audience))
if (tokenDescriptor.Audiences.Count > 0)
{
var audienceRestriction = new Saml2AudienceRestriction(tokenDescriptor.Audiences);
if (!string.IsNullOrEmpty(tokenDescriptor.Audience))
audienceRestriction.Audiences.Add(tokenDescriptor.Audience);
conditions.AudienceRestrictions.Add(audienceRestriction);
}
else if (!string.IsNullOrEmpty(tokenDescriptor.Audience))
conditions.AudienceRestrictions.Add(new Saml2AudienceRestriction(tokenDescriptor.Audience));

return conditions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,7 @@ public static void WriteObject(ref Utf8JsonWriter writer, string key, object obj
#if NET6_0_OR_GREATER
writer.WriteNumber(key, dub);
#else
#pragma warning disable CA1031 // Do not catch general exception types, we have seen TryParse fault.
#pragma warning disable CA1031 // Do not catch general exception types, we have seen TryParse fault.
try
{
if (decimal.TryParse(dub.ToString(CultureInfo.InvariantCulture), out decimal dec))
Expand All @@ -1164,7 +1164,7 @@ public static void WriteObject(ref Utf8JsonWriter writer, string key, object obj
{
writer.WriteNumber(key, dub);
}
#pragma warning restore CA1031
#pragma warning restore CA1031
#endif
else if (obj is decimal d)
writer.WriteNumber(key, d);
Expand All @@ -1174,7 +1174,7 @@ public static void WriteObject(ref Utf8JsonWriter writer, string key, object obj
#if NET6_0_OR_GREATER
writer.WriteNumber(key, f);
#else
#pragma warning disable CA1031 // Do not catch general exception types, we have seen TryParse fault.
#pragma warning disable CA1031 // Do not catch general exception types, we have seen TryParse fault.
try
{
if (decimal.TryParse(f.ToString(CultureInfo.InvariantCulture), out decimal dec))
Expand All @@ -1186,7 +1186,7 @@ public static void WriteObject(ref Utf8JsonWriter writer, string key, object obj
{
writer.WriteNumber(key, f);
}
#pragma warning restore CA1031
#pragma warning restore CA1031
#endif
else if (obj is Guid g)
writer.WriteString(key, g);
Expand All @@ -1197,7 +1197,7 @@ public static void WriteObject(ref Utf8JsonWriter writer, string key, object obj
LogMessages.IDX11025,
LogHelper.MarkAsNonPII(objType.ToString()),
LogHelper.MarkAsNonPII(key))));
}
}

/// <summary>
/// Writes values into an array.
Expand Down Expand Up @@ -1318,6 +1318,16 @@ public static void WriteStrings(ref Utf8JsonWriter writer, ReadOnlySpan<byte> pr

writer.WriteEndArray();
}
#endregion

public static void WriteStrings(ref Utf8JsonWriter writer, ReadOnlySpan<byte> propertyName, IList<string> strings, string extraString)
{
writer.WriteStartArray(propertyName);
foreach (string str in strings)
writer.WriteStringValue(str);

writer.WriteStringValue(extraString);
writer.WriteEndArray();
}
#endregion
}
}
1 change: 0 additions & 1 deletion src/Microsoft.IdentityModel.Tokens/LogMessages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ internal static class LogMessages
public const string IDX11025 = "IDX11025: Cannot serialize object of type: '{0}' into property: '{1}'.";
public const string IDX11026 = "IDX11026: Unable to get claim value as a string from claim type:'{0}', value type was:'{1}'. Acceptable types are String, IList<String>, and System.Text.Json.JsonElement.";


#pragma warning restore 1591
}
}
12 changes: 11 additions & 1 deletion src/Microsoft.IdentityModel.Tokens/SecurityTokenDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Security.Claims;
using System.Threading;

namespace Microsoft.IdentityModel.Tokens
{
Expand All @@ -12,11 +13,20 @@ namespace Microsoft.IdentityModel.Tokens
/// </summary>
public class SecurityTokenDescriptor
{
private List<string> _audiences;

/// <summary>
/// Gets or sets the value of the 'audience' claim.
/// Gets or sets the value of the {"": audience} claim. Will be combined with <see cref="Audiences"/> and any "Aud" claims in
/// <see cref="Claims"/> or <see cref="Subject"/> when creating a token.
/// </summary>
public string Audience { get; set; }

/// <summary>
/// Gets the list audiences to include in the token's 'Aud' claim. Will be combined with <see cref="Audiences"/> and any
/// "Aud" claims in <see cref="Claims"/> or <see cref="Subject"/> when creating a token.
/// </summary>
public IList<string> Audiences => _audiences ?? Interlocked.CompareExchange(ref _audiences, [], null) ?? _audiences;

/// <summary>
/// Defines the compression algorithm that will be used to compress the JWT token payload.
/// </summary>
Expand Down
Loading

0 comments on commit 55cc10e

Please sign in to comment.