Let consumers register their own Credential subclasses [minor]

CredentialCache.Test/CredentialCacheTests.cs

@@ -232,6 +232,9 @@ public void CredentialSerializationRoundTripsAllKnownTypes()
 	[TestMethod]
 	public void RegisterMultipleCredentialFactoriesCreatesCredentialsCorrectly()
 	{
+		// AnotherCredential is defined here rather than in the library, so it has to be registered
+		// before any store will take it.
+		CredentialSerialization.RegisterDerivedType<AnotherCredential>(nameof(AnotherCredential));
 		using CredentialCache cache = NewCache();
 		CredentialWithNothingFactory factory1 = new();
 		AnotherCredentialFactory factory2 = new();

CredentialCache.Test/CustomCredentialTypeTests.cs

@@ -0,0 +1,116 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.CredentialCache.Test;
+
+using ktsu.CredentialCache.Storage;
+
+/// <summary>
+/// Tests that a <see cref="Credential"/> subclass defined outside the library can be registered and
+/// stored, and that one which was never registered is refused the same way by every store.
+/// </summary>
+/// <remarks>
+/// Registrations are process-wide and cannot be undone, so each test uses its own subclass and its
+/// own discriminator.
+/// </remarks>
+[TestClass]
+public class CustomCredentialTypeTests
+{
+	public sealed class CredentialWithCertificate : Credential
+	{
+		public string Thumbprint { get; init; } = "";
+	}
+
+	public sealed class CredentialWithSshKey : Credential
+	{
+		public string KeyId { get; init; } = "";
+	}
+
+	public sealed class NeverRegisteredCredential : Credential
+	{
+		public string Value { get; init; } = "";
+	}
+
+	public sealed class TwiceRegisteredCredential : Credential { }
+
+	public sealed class ConflictingCredential : Credential { }
+
+	public abstract class AbstractCredential : Credential { }
+
+	private static PersonaGUID NewPersona() => PersonaGUID.Create(Guid.NewGuid().ToString());
+
+	[TestMethod]
+	public void ARegisteredSubclassRoundTripsThroughSerialization()
+	{
+		CredentialSerialization.RegisterDerivedType<CredentialWithCertificate>("Test.CredentialWithCertificate");
+
+		byte[] json = CredentialSerialization.Serialize(new CredentialWithCertificate { Thumbprint = "ab" });
+		Credential? read = CredentialSerialization.Deserialize(json);
+
+		CredentialWithCertificate certificate = Assert.IsInstanceOfType<CredentialWithCertificate>(read);
+		Assert.AreEqual("ab", certificate.Thumbprint);
+	}
+
+	[TestMethod]
+	public void ARegisteredSubclassRoundTripsThroughTheNativeBuffer()
+	{
+		CredentialSerialization.RegisterDerivedType<CredentialWithSshKey>("Test.CredentialWithSshKey");
+
+		using NativeSecretBuffer buffer = NativeSecretBuffer.OfCredential(new CredentialWithSshKey { KeyId = "key-1" });
+		Credential? read = NativeSecretBuffer.ReadCredential(buffer.Pointer);
+
+		CredentialWithSshKey key = Assert.IsInstanceOfType<CredentialWithSshKey>(read);
+		Assert.AreEqual("key-1", key.KeyId);
+	}
+
+	[TestMethod]
+	public void AddOrReplaceRefusesAnUnregisteredSubclassAndLeavesTheCacheUnchanged()
+	{
+		InMemoryCredentialStore store = new();
+		using CredentialCache cache = new(store);
+		PersonaGUID persona = NewPersona();
+		CredentialWithToken original = new() { Token = CredentialToken.Create("original") };
+		cache.AddOrReplace(persona, original);
+
+		CredentialStoreException exception = Assert.ThrowsExactly<CredentialStoreException>(
+			() => cache.AddOrReplace(persona, new NeverRegisteredCredential { Value = "x" }));
+
+		Assert.Contains(nameof(NeverRegisteredCredential), exception.Message);
+		Assert.IsTrue(cache.TryGet(persona, out Credential? cached));
+		Assert.AreSame(original, cached);
+		Assert.IsTrue(store.TryLoad(persona, out Credential? stored));
+		Assert.AreSame(original, stored);
+	}
+
+	[TestMethod]
+	public void RegisteringTheSameTypeAndDiscriminatorAgainDoesNothing()
+	{
+		CredentialSerialization.RegisterDerivedType<TwiceRegisteredCredential>("Test.TwiceRegisteredCredential");
+		CredentialSerialization.RegisterDerivedType<TwiceRegisteredCredential>("Test.TwiceRegisteredCredential");
+
+		_ = Assert.ThrowsExactly<ArgumentException>(
+			() => CredentialSerialization.RegisterDerivedType<TwiceRegisteredCredential>("Test.SomethingElse"));
+	}
+
+	[TestMethod]
+	public void ADiscriminatorThatAlreadyNamesAnotherTypeIsRefused()
+	{
+		_ = Assert.ThrowsExactly<ArgumentException>(
+			() => CredentialSerialization.RegisterDerivedType<ConflictingCredential>(nameof(CredentialWithToken)));
+
+		// The refusal left nothing behind.
+		using CredentialCache cache = new(new InMemoryCredentialStore());
+		_ = Assert.ThrowsExactly<CredentialStoreException>(
+			() => cache.AddOrReplace(NewPersona(), new ConflictingCredential()));
+	}
+
+	[TestMethod]
+	public void TheLibrarysOwnAndAbstractTypesCannotBeRegistered()
+	{
+		_ = Assert.ThrowsExactly<ArgumentException>(
+			() => CredentialSerialization.RegisterDerivedType<CredentialWithToken>("Test.Token"));
+		_ = Assert.ThrowsExactly<ArgumentException>(
+			() => CredentialSerialization.RegisterDerivedType<AbstractCredential>("Test.Abstract"));
+		_ = Assert.ThrowsExactly<ArgumentException>(
+			() => CredentialSerialization.RegisterDerivedType<CredentialWithCertificate>(" "));
+	}
+}

CredentialCache/CredentialCache.cs

@@ -9,7 +9,7 @@
 /// <summary>
 /// Represents a globally unique identifier for a persona.
 /// </summary>
 public sealed record class PersonaGUID : SemanticString<PersonaGUID> { }
 
 /// <summary>
 /// Caches <see cref="Credential"/> instances in memory and persists each one through
@@ -192,12 +192,21 @@
 	/// lock, so a concurrent <see cref="Remove"/> of the same persona cannot land
 	/// between them.
 	/// </remarks>
+	/// <exception cref="CredentialStoreException">
+	/// <paramref name="credential"/> is a type defined outside this library that was never
+	/// registered with <see cref="CredentialSerialization.RegisterDerivedType{T}(string)"/>. The
+	/// cache and the store are left unchanged.
+	/// </exception>
 	public void AddOrReplace(PersonaGUID persona, Credential credential)
 	{
 		ArgumentNullException.ThrowIfNull(persona);
 		ArgumentNullException.ThrowIfNull(credential);
 		ThrowIfDisposed();
 
+		// Checked here rather than left to the store, so every store refuses the same types. The
+		// in-memory store never serializes, and it accepted types every platform store rejects.
+		CredentialSerialization.ThrowIfUnknownType(credential);
+
 		lock (LockFor(persona))
 		{
 			Store.Save(persona, credential);

CredentialCache/Storage/CredentialSerialization.cs

@@ -2,8 +2,11 @@
 
 namespace ktsu.CredentialCache.Storage;
 
+using System.Reflection;
 using System.Security.Cryptography;
 using System.Text.Json;
+using System.Text.Json.Serialization;
+using System.Text.Json.Serialization.Metadata;
 using ktsu.RoundTripStringJsonConverter;
 
 /// <summary>
@@ -12,33 +15,158 @@ namespace ktsu.CredentialCache.Storage;
 /// through these helpers so polymorphic <see cref="Credential"/> subclasses are
 /// preserved.
 /// </summary>
+/// <remarks>
+/// The library's own subclasses are known up front. A subclass defined anywhere else has to be
+/// registered with <see cref="RegisterDerivedType{T}(string)"/> before it is saved or loaded.
+/// </remarks>
 public static class CredentialSerialization
 {
-	private static readonly JsonSerializerOptions Options = BuildOptions();
+	/// <summary>
+	/// The subclasses <see cref="Credential"/> itself declares, keyed by type.
+	/// </summary>
+	private static readonly Dictionary<Type, string> BuiltInTypes = typeof(Credential)
+		.GetCustomAttributes<JsonDerivedTypeAttribute>()
+		.ToDictionary(attribute => attribute.DerivedType, attribute => (string)attribute.TypeDiscriminator!);
+
+	private static readonly Lock RegistrationLock = new();
+
+	/// <summary>
+	/// The subclasses registered through <see cref="RegisterDerivedType{T}(string)"/>. Replaced
+	/// rather than mutated, under <see cref="RegistrationLock"/>, so a reader always sees a
+	/// complete set.
+	/// </summary>
+	private static volatile Dictionary<Type, string> registeredTypes = [];
 
-	private static JsonSerializerOptions BuildOptions()
+	/// <summary>
+	/// The options every helper serializes with. Rebuilt on each registration rather than
+	/// extended, because System.Text.Json fixes an options instance's polymorphic type set the
+	/// first time it is used, so a type added afterwards would otherwise never be seen.
+	/// </summary>
+	private static volatile JsonSerializerOptions options = BuildOptions(registeredTypes);
+
+	private static JsonSerializerOptions BuildOptions(Dictionary<Type, string> derivedTypes)
 	{
-		JsonSerializerOptions options = new()
+		DefaultJsonTypeInfoResolver resolver = new();
+		resolver.Modifiers.Add(typeInfo =>
+		{
+			if (typeInfo.Type == typeof(Credential) && typeInfo.PolymorphismOptions is not null)
+			{
+				foreach ((Type type, string discriminator) in derivedTypes)
+				{
+					typeInfo.PolymorphismOptions.DerivedTypes.Add(new JsonDerivedType(type, discriminator));
+				}
+			}
+		});
+
+		JsonSerializerOptions built = new()
 		{
 			WriteIndented = false,
+			TypeInfoResolver = resolver,
 		};
 		// Persuade System.Text.Json to use the SemanticString factory methods (Create / FromString)
 		// rather than treating SemanticString<T> as an IEnumerable<char> collection.
-		options.Converters.Add(new RoundTripStringJsonConverterFactory());
-		return options;
+		built.Converters.Add(new RoundTripStringJsonConverterFactory());
+		return built;
+	}
+
+	/// <summary>
+	/// Registers a <see cref="Credential"/> subclass defined outside this library, so the
+	/// platform stores can save and load it.
+	/// </summary>
+	/// <typeparam name="T">The subclass to register.</typeparam>
+	/// <param name="typeDiscriminator">
+	/// The name written into the stored JSON to identify <typeparamref name="T"/>. It is what
+	/// identifies the type when a stored credential is read back, so it must stay the same for
+	/// as long as credentials saved with it exist.
+	/// </param>
+	/// <remarks>
+	/// Register each type once, at start-up, before the first credential of that type is saved
+	/// or loaded. Registering the same type with the same discriminator again does nothing.
+	/// </remarks>
+	/// <exception cref="ArgumentException">
+	/// <paramref name="typeDiscriminator"/> is empty or whitespace, <typeparamref name="T"/> is
+	/// abstract or is one of the library's own subclasses, <typeparamref name="T"/> is already
+	/// registered under a different discriminator, or the discriminator already names another
+	/// type.
+	/// </exception>
+	public static void RegisterDerivedType<T>(string typeDiscriminator) where T : Credential
+	{
+		ArgumentException.ThrowIfNullOrWhiteSpace(typeDiscriminator);
+
+		Type type = typeof(T);
+		if (type.IsAbstract)
+		{
+			throw new ArgumentException($"{type} is abstract, so no stored credential can be one.");
+		}
+
+		if (BuiltInTypes.ContainsKey(type))
+		{
+			throw new ArgumentException($"{type} is one of the library's own credential types and is already known.");
+		}
+
+		lock (RegistrationLock)
+		{
+			if (registeredTypes.TryGetValue(type, out string? existing))
+			{
+				if (existing == typeDiscriminator)
+				{
+					return;
+				}
+
+				throw new ArgumentException($"{type} is already registered with the discriminator \"{existing}\".");
+			}
+
+			Type? owner = BuiltInTypes.Concat(registeredTypes)
+				.FirstOrDefault(entry => entry.Value == typeDiscriminator).Key;
+			if (owner is not null)
+			{
+				throw new ArgumentException($"The discriminator \"{typeDiscriminator}\" already identifies {owner}.", nameof(typeDiscriminator));
+			}
+
+			Dictionary<Type, string> updated = new(registeredTypes) { [type] = typeDiscriminator };
+			options = BuildOptions(updated);
+			registeredTypes = updated;
+		}
+	}
+
+	/// <summary>
+	/// Whether credentials of <paramref name="type"/> can be serialized, either because the
+	/// library declares it or because it was registered.
+	/// </summary>
+	internal static bool IsKnownType(Type type)
+	{
+		ArgumentNullException.ThrowIfNull(type);
+		return BuiltInTypes.ContainsKey(type) || registeredTypes.ContainsKey(type);
+	}
+
+	/// <summary>
+	/// Throws <see cref="CredentialStoreException"/> naming <paramref name="credential"/>'s type
+	/// when it is not one that can be serialized.
+	/// </summary>
+	internal static void ThrowIfUnknownType(Credential credential)
+	{
+		ArgumentNullException.ThrowIfNull(credential);
+
+		Type type = credential.GetType();
+		if (!IsKnownType(type))
+		{
+			throw new CredentialStoreException(
+				$"{type} is not a known credential type, so it cannot be stored. Register it first with "
+				+ $"{nameof(CredentialSerialization)}.{nameof(RegisterDerivedType)}.");
+		}
 	}
 
 	/// <summary>
 	/// Serializes the credential to a UTF-8 JSON byte array.
 	/// </summary>
 	public static byte[] Serialize(Credential credential) =>
-		JsonSerializer.SerializeToUtf8Bytes(credential, Options);
+		JsonSerializer.SerializeToUtf8Bytes(credential, options);
 
 	/// <summary>
 	/// Serializes the credential to a UTF-8 JSON string.
 	/// </summary>
 	public static string SerializeToString(Credential credential) =>
-		JsonSerializer.Serialize(credential, Options);
+		JsonSerializer.Serialize(credential, options);
 
 	/// <summary>
 	/// Deserializes a credential from a UTF-8 JSON byte array. Returns <c>null</c> if the bytes
@@ -53,7 +181,7 @@ public static string SerializeToString(Credential credential) =>
 
 		try
 		{
-			return JsonSerializer.Deserialize<Credential>(utf8Json, Options);
+			return JsonSerializer.Deserialize<Credential>(utf8Json, options);
 		}
 		catch (JsonException)
 		{
@@ -122,7 +250,7 @@ internal static void Zero(byte[] buffer)
 
 		try
 		{
-			return JsonSerializer.Deserialize<Credential>(utf8Json, Options);
+			return JsonSerializer.Deserialize<Credential>(utf8Json, options);
 		}
 		catch (JsonException)
 		{

README.md

@@ -88,7 +88,7 @@ The library ships with three concrete `Credential` subclasses:
 
 ### Adding your own credential type
 
-`Credential` is a polymorphic record class round-tripped through `System.Text.Json`. New subclasses need a `[JsonDerivedType]` on the base so deserialization can resolve them:
+`Credential` is a polymorphic abstract class round-tripped through `System.Text.Json`. A subclass defined outside the library has to be registered once, at start-up, before a credential of that type is saved or loaded:
 
 ```csharp
 // 1. Add the subclass.
@@ -97,9 +97,9 @@ public sealed class CredentialWithCertificate : Credential
     public string Thumbprint { get; init; } = "";
 }
 
-// 2. Register it on the base in Credential.cs.
-[JsonDerivedType(typeof(CredentialWithCertificate), nameof(CredentialWithCertificate))]
-public abstract class Credential { /* ... */ }
+// 2. Register it. The discriminator is written into every stored credential of this type and
+//    identifies it when one is read back, so keep it stable once credentials have been saved.
+CredentialSerialization.RegisterDerivedType<CredentialWithCertificate>("MyApp.CredentialWithCertificate");
 
 // 3. Optional: register a factory so TryCreate<T> works.
 public sealed class CertificateFactory : ICredentialFactory<CredentialWithCertificate>
@@ -111,6 +111,8 @@ cache.RegisterCredentialFactory(new CertificateFactory());
 
 If a subclass uses `SemanticString<T>` properties, they round-trip through `ktsu.RoundTripStringJsonConverter` automatically.
 
+`AddOrReplace` refuses a subclass that was never registered with a `CredentialStoreException` naming the type, and leaves the cache and the store unchanged. It does this for every store, `InMemoryCredentialStore` included, so a test that uses the in-memory store fails the same way production would.
+
 ## Customising the backing store
 
 `ICredentialStore` is a small CRUD interface (`TryLoad` / `Save` / `Remove`). Bring your own implementation when you need a different backend (HashiCorp Vault, an encrypted file, a test double):