Skip to content

Commit

Permalink
Filter client credentials in ToOptimizedFullDictionary method
Browse files Browse the repository at this point in the history
Filter client authentication because it's possible for a PAR request to include client auth, and we don't want that to cause client auth parameters to get passed into the authorization parameters message store, where they might get logged or otherwise stored
 insecurely.
  • Loading branch information
josephdecock committed Feb 9, 2025
1 parent 4ee3d55 commit 7bf7e92
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,16 @@ public static string ToOptimizedQueryString(this ValidatedAuthorizeRequest reque
[Obsolete("This method is obsolete and will be removed in a future version.")]
public static IDictionary<string, string[]> ToOptimizedFullDictionary(this ValidatedAuthorizeRequest request)
{
return request.ToOptimizedRawValues().ToFullDictionary();
var collection = request.ToOptimizedRawValues();

// Filter client authentication out of the dictionary that we're building
// It's possible for a PAR request to include client auth and we don't want
// that to cause these values to get passed into the (obsolete) authorization
// parameters message store, where they might get logged or otherwise stored
// insecurely.
collection.Remove(OidcConstants.TokenRequest.ClientAssertion);
collection.Remove(OidcConstants.TokenRequest.ClientSecret);

return collection.ToFullDictionary();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,23 @@ public void ToOptimizedFullDictionary_should_return_dictionary_with_array_for_re

Assert.Equal(2, res[OidcConstants.AuthorizeRequest.Resource].Length);
}

[Theory]
[Obsolete]
[InlineData(OidcConstants.TokenRequest.ClientAssertion)]
[InlineData(OidcConstants.TokenRequest.ClientSecret)]
public void ToOptimizedFullDictionary_should_filter_client_credentials(string filteredClaimType)
{
var request = new ValidatedAuthorizeRequest()
{
Raw = new System.Collections.Specialized.NameValueCollection
{
{ filteredClaimType, "" },
}
};

var result = request.ToOptimizedFullDictionary();

Assert.Equal(0, result.Count);

Check warning on line 67 in identity-server/test/IdentityServer.UnitTests/Extensions/ValidatedAuthorizeRequestExtensionsTests.cs

View workflow job for this annotation

GitHub Actions / Build

Do not use Assert.Equal() to check for collection size. Use Assert.Empty instead. (https://xunit.net/xunit.analyzers/rules/xUnit2013)

Check warning on line 67 in identity-server/test/IdentityServer.UnitTests/Extensions/ValidatedAuthorizeRequestExtensionsTests.cs

View workflow job for this annotation

GitHub Actions / Build

Do not use Assert.Equal() to check for collection size. Use Assert.Empty instead. (https://xunit.net/xunit.analyzers/rules/xUnit2013)

Check warning on line 67 in identity-server/test/IdentityServer.UnitTests/Extensions/ValidatedAuthorizeRequestExtensionsTests.cs

View workflow job for this annotation

GitHub Actions / Build

Do not use Assert.Equal() to check for collection size. Use Assert.Empty instead. (https://xunit.net/xunit.analyzers/rules/xUnit2013)

Check warning on line 67 in identity-server/test/IdentityServer.UnitTests/Extensions/ValidatedAuthorizeRequestExtensionsTests.cs

View workflow job for this annotation

GitHub Actions / Build

Do not use Assert.Equal() to check for collection size. Use Assert.Empty instead. (https://xunit.net/xunit.analyzers/rules/xUnit2013)
}
}

0 comments on commit 7bf7e92

Please sign in to comment.