[Bug] Serialization broken since 8.0.1

[SimonSchwendele]

Which version of Microsoft.IdentityModel are you using?
Microsoft.IdentityModel 7.7.1 ( broken on 8.0.1 )

Where is the issue?

• M.IM.JsonWebTokens
• M.IM.KeyVaultExtensions
• M.IM.Logging
• M.IM.ManagedKeyVaultSecurityKey
• M.IM.Protocols
• M.IM.Protocols.OpenIdConnect
• M.IM.Protocols.SignedHttpRequest
• M.IM.Protocols.WsFederation
• M.IM.TestExtensions
• M.IM.Tokens
• M.IM.Tokens.Saml
• M.IM.Validators
• M.IM.Xml
• S.IM.Tokens.Jwt
• Other (please describe)

Is this a new or an existing app?
The app is in production and I haven't upgraded Microsoft.IdentityModel.*, but started seeing this issue.

Repro

public interface ITargetInterface
{
     void DoThings();
}

public class ClassToDoThings : MarshalByRefObj, ITargetInterface
{
    public void DoThings() => Console.WriteLine("Did something");
}

...
...

var baseDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location!);
var setup = new AppDomainSetup
 {
    ApplicationName = "SomeApp",
    ApplicationBase = baseDir,
    PrivateBinPath =  "AppBinDir",
    ShadowCopyFiles = "false"
 };
 var appDomain = AppDomain.CreateDomain("SomeApp",
                                         securityInfo: null,
                                         setup);

When I invoke Activator.CreateInstance(ClassToDoThingsType) as MarshalByRefObject
I get the following error.

Expected behavior
A new instance of the object is created accross the domains.

Actual behavior

System.Runtime.Serialization.SerializationException: Type 'Microsoft.IdentityModel.Tokens.CaseSensitiveClaimsIdentity' in Assembly 'Microsoft.IdentityModel.Tokens, Version=8.0.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' is not marked as serializable.

Server stack trace: 
   at System.Runtime.Serialization.FormatterServices.InternalGetSerializableMembers(RuntimeType type)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at System.Runtime.Serialization.FormatterServices.GetSerializableMembers(Type type, StreamingContext context)
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.InitMemberInfo()
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.InitSerialize(Object obj, ISurrogateSelector surrogateSelector, StreamingContext context, SerObjectInfoInit serObjectInfoInit, IFormatterConverter converter, ObjectWriter objectWriter, SerializationBinder binder)
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.Serialize(Object obj, ISurrogateSelector surrogateSelector, StreamingContext context, SerObjectInfoInit serObjectInfoInit, IFormatterConverter converter, ObjectWriter objectWriter, SerializationBinder binder)
   at System.Runtime.Serialization.Formatters.Binary.ObjectWriter.Serialize(Object graph, Header[] inHeaders, __BinaryWriter serWriter, Boolean fCheck)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph, Header[] headers, Boolean fCheck)
   at System.Security.Claims.ClaimsPrincipal.SerializeIdentities()
   at System.Security.Claims.ClaimsPrincipal.OnSerializingMethod(StreamingContext context)
   at System.Runtime.Serialization.SerializationEvents.InvokeOnSerializing(Object obj, StreamingContext context)
   at System.Runtime.Serialization.SerializationObjectManager.RegisterObject(Object obj)
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.InitSerialize(Object obj, ISurrogateSelector surrogateSelector, StreamingContext context, SerObjectInfoInit serObjectInfoInit, IFormatterConverter converter, ObjectWriter objectWriter, SerializationBinder binder)
   at System.Runtime.Serialization.Formatters.Binary.ObjectWriter.Write(WriteObjectInfo objectInfo, NameInfo memberNameInfo, NameInfo typeNameInfo)
   at System.Runtime.Serialization.Formatters.Binary.ObjectWriter.Serialize(Object graph, Header[] inHeaders, __BinaryWriter serWriter, Boolean fCheck)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph, Header[] headers, Boolean fCheck)
   at System.Runtime.Remoting.Channels.CrossAppDomainSerializer.SerializeMessageParts(ArrayList argsToSerialize)
   at System.Runtime.Remoting.Messaging.SmuggledMethodCallMessage..ctor(IMethodCallMessage mcm)
   at System.Runtime.Remoting.Messaging.SmuggledMethodCallMessage.SmuggleIfPossible(IMessage msg)
   at System.Runtime.Remoting.Channels.CrossAppDomainSink.SyncProcessMessage(IMessage reqMsg)

Possible solution

The issue is due to the recently introduced type CaseSensitiveClaimsIdentity in #2700 .
It could be made serializable.

Additional context / logs / screenshots / links to code

Add any other context about the problem here, such as logs and screenshots or links to code.

[pahlquist]

Thanks for fixing this. According to the notes above this is slated for the 8.2.0 milestone, which looks to be aimed for release in early November. Is there any potential for this fix to be released sooner than that? I'm trying to upgrade our project from v7.2.0 to v8.1.0 and this bug is a showstopper.

Apologies in advance if this isn't the right spot to make this request.

[kllysng]

Thanks for fixing this. According to the notes above this is slated for the 8.2.0 milestone, which looks to be aimed for release in early November. Is there any potential for this fix to be released sooner than that? I'm trying to upgrade our project from v7.2.0 to v8.1.0 and this bug is a showstopper.

Apologies in advance if this isn't the right spot to make this request.

@pahlquist, appreciate you bringing this to our attention. We are adding this to our 8.1.1 milestone which will be released imminently.

[jennyf19]

@pahlquist released in https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/releases/tag/8.1.1

[SimonSchwendele]

Sorry to necrobump this issue again but I just tried to update again and another one broke on me.

System.Runtime.Serialization.SerializationException: Type 'System.IdentityModel.Tokens.Jwt.JwtSecurityToken' in Assembly 'System.IdentityModel.Tokens.Jwt, Version=8.1.2.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' is not marked as serializable.

Is it possible to make that one serializable as well ?

I tried forking it but it appears like this could be problematic
https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs

[pahlquist]

@SimonSchwendele, same result here. I've also forked it and am following the trail of the other handful of classes that would need to be marked serializable and came across the SuppressMessage attribute to which you're referring that implies JwtPayload (and JwtHeader) shouldn't be marked serializable. The documented justification of "Serialize not really supported." isn't all that convincing. At the end of the day, we're talking about a class that represents a JWT...the sole purpose of which is to be passed from system to system...so it seems justifiable to request that it be marked serializable. That said, you're right that it might require some refactoring, so I understand it might be a bit complex.

Thanks to @kellyyangsong and @jennyf19 for the attention you've given this, it's much appreciated!

[SimonSchwendele]

@SimonSchwendele, same result here. I've also forked it and am following the trail of the other handful of classes that would need to be marked serializable and came across the SuppressMessage attribute to which you're referring that implies JwtPayload (and JwtHeader) shouldn't be marked serializable. The documented justification of "Serialize not really supported." isn't all that convincing. At the end of the day, we're talking about a class that represents a JWT...the sole purpose of which is to be passed from system to system...so it seems justifiable to request that it be marked serializable. That said, you're right that it might require some refactoring, so I understand it might be a bit complex.

Thanks to @kellyyangsong and @jennyf19 for the attention you've given this, it's much appreciated!

Well the existing code is serializable that much ist true.
The problem however yields from its baseclass "Dictionary<string, object>".
The serialization breaks the moment someone puts a nonserializable object into this dictionary and so on.
Its more like "Serialize not [reliable enough] supported."