Skip to content

Conversation

@jmprieur
Copy link
Contributor

Add a custom serializer for CredentialDescription in .NET 8.

Fixes #176

@jmprieur jmprieur requested a review from a team as a code owner April 22, 2025 00:37
@jmprieur jmprieur requested a review from Copilot April 22, 2025 00:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a custom JSON serializer/deserializer for CredentialDescription to support new scenarios in .NET 8+. Key changes include:

  • New test cases in CredentialDescriptionJsonConverterTest.cs validating roundtrip serialization for various CredentialSource values.
  • Updates to the PublicAPI files to include the new CredentialDescriptionJsonConverter and related APIs.
  • Implementation of CredentialDescriptionJsonConverter in the ApplicationOptions folder.

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.

File Description
test/Microsoft.Identity.Abstractions.Tests/CredentialDescriptionJsonConverterTest.cs Adds tests to verify correct roundtrip serialization for different credential sources.
src/Microsoft.Identity.Abstractions/PublicAPI/net9.0/PublicAPI.Unshipped.txt Exposes the new converter API for upcoming releases.
src/Microsoft.Identity.Abstractions/PublicAPI/net9.0/PublicAPI.Shipped.txt Documents the shipped public API changes including the serializer.
src/Microsoft.Identity.Abstractions/ApplicationOptions/CredentialDescriptionJsonConverter.cs Provides the implementation of the custom JSON converter for CredentialDescription.
Files not reviewed (2)
  • Directory.Build.props: Language not supported
  • src/Microsoft.Identity.Abstractions/PublicAPI/net8.0/PublicAPI.Unshipped.txt: Language not supported
Comments suppressed due to low confidence (1)

test/Microsoft.Identity.Abstractions.Tests/CredentialDescriptionJsonConverterTest.cs:162

  • Consider adding test cases for additional CredentialSource values (for instance, StoreWithDistinguishedName) to ensure that the converter handles all supported sources consistently.
public void SerializeDeserialize_CustomSignedAssertion()

Copy link
Contributor

@keegan-caruso keegan-caruso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the version, it looks right

Revert version

Signed-off-by: Jean-Marc Prieur <[email protected]>
@jmprieur jmprieur merged commit bdcd2a4 into main Apr 29, 2025
4 checks passed
@jmprieur jmprieur deleted the jmprieur/credentialDescriptionSerializers branch April 29, 2025 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add a custom JSon serializer for CredentialDescription in .NET 8+

4 participants