diff --git a/CredentialCache.Test/CredentialCacheTests.cs b/CredentialCache.Test/CredentialCacheTests.cs index 1a85fbc..f18bb79 100644 --- a/CredentialCache.Test/CredentialCacheTests.cs +++ b/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(nameof(AnotherCredential)); using CredentialCache cache = NewCache(); CredentialWithNothingFactory factory1 = new(); AnotherCredentialFactory factory2 = new(); diff --git a/CredentialCache.Test/CustomCredentialTypeTests.cs b/CredentialCache.Test/CustomCredentialTypeTests.cs new file mode 100644 index 0000000..6fb842a --- /dev/null +++ b/CredentialCache.Test/CustomCredentialTypeTests.cs @@ -0,0 +1,116 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.CredentialCache.Test; + +using ktsu.CredentialCache.Storage; + +/// +/// Tests that a 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. +/// +/// +/// Registrations are process-wide and cannot be undone, so each test uses its own subclass and its +/// own discriminator. +/// +[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("Test.CredentialWithCertificate"); + + byte[] json = CredentialSerialization.Serialize(new CredentialWithCertificate { Thumbprint = "ab" }); + Credential? read = CredentialSerialization.Deserialize(json); + + CredentialWithCertificate certificate = Assert.IsInstanceOfType(read); + Assert.AreEqual("ab", certificate.Thumbprint); + } + + [TestMethod] + public void ARegisteredSubclassRoundTripsThroughTheNativeBuffer() + { + CredentialSerialization.RegisterDerivedType("Test.CredentialWithSshKey"); + + using NativeSecretBuffer buffer = NativeSecretBuffer.OfCredential(new CredentialWithSshKey { KeyId = "key-1" }); + Credential? read = NativeSecretBuffer.ReadCredential(buffer.Pointer); + + CredentialWithSshKey key = Assert.IsInstanceOfType(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( + () => 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("Test.TwiceRegisteredCredential"); + CredentialSerialization.RegisterDerivedType("Test.TwiceRegisteredCredential"); + + _ = Assert.ThrowsExactly( + () => CredentialSerialization.RegisterDerivedType("Test.SomethingElse")); + } + + [TestMethod] + public void ADiscriminatorThatAlreadyNamesAnotherTypeIsRefused() + { + _ = Assert.ThrowsExactly( + () => CredentialSerialization.RegisterDerivedType(nameof(CredentialWithToken))); + + // The refusal left nothing behind. + using CredentialCache cache = new(new InMemoryCredentialStore()); + _ = Assert.ThrowsExactly( + () => cache.AddOrReplace(NewPersona(), new ConflictingCredential())); + } + + [TestMethod] + public void TheLibrarysOwnAndAbstractTypesCannotBeRegistered() + { + _ = Assert.ThrowsExactly( + () => CredentialSerialization.RegisterDerivedType("Test.Token")); + _ = Assert.ThrowsExactly( + () => CredentialSerialization.RegisterDerivedType("Test.Abstract")); + _ = Assert.ThrowsExactly( + () => CredentialSerialization.RegisterDerivedType(" ")); + } +} diff --git a/CredentialCache/CredentialCache.cs b/CredentialCache/CredentialCache.cs index d8160bb..b5ffca9 100644 --- a/CredentialCache/CredentialCache.cs +++ b/CredentialCache/CredentialCache.cs @@ -192,12 +192,21 @@ public bool TryGet(PersonaGUID persona, out Credential? credential) /// lock, so a concurrent of the same persona cannot land /// between them. /// + /// + /// is a type defined outside this library that was never + /// registered with . The + /// cache and the store are left unchanged. + /// 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); diff --git a/CredentialCache/Storage/CredentialSerialization.cs b/CredentialCache/Storage/CredentialSerialization.cs index 1f462b3..0faf895 100644 --- a/CredentialCache/Storage/CredentialSerialization.cs +++ b/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; /// @@ -12,33 +15,158 @@ namespace ktsu.CredentialCache.Storage; /// through these helpers so polymorphic subclasses are /// preserved. /// +/// +/// The library's own subclasses are known up front. A subclass defined anywhere else has to be +/// registered with before it is saved or loaded. +/// public static class CredentialSerialization { - private static readonly JsonSerializerOptions Options = BuildOptions(); + /// + /// The subclasses itself declares, keyed by type. + /// + private static readonly Dictionary BuiltInTypes = typeof(Credential) + .GetCustomAttributes() + .ToDictionary(attribute => attribute.DerivedType, attribute => (string)attribute.TypeDiscriminator!); + + private static readonly Lock RegistrationLock = new(); + + /// + /// The subclasses registered through . Replaced + /// rather than mutated, under , so a reader always sees a + /// complete set. + /// + private static volatile Dictionary registeredTypes = []; - private static JsonSerializerOptions BuildOptions() + /// + /// 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. + /// + private static volatile JsonSerializerOptions options = BuildOptions(registeredTypes); + + private static JsonSerializerOptions BuildOptions(Dictionary 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 as an IEnumerable collection. - options.Converters.Add(new RoundTripStringJsonConverterFactory()); - return options; + built.Converters.Add(new RoundTripStringJsonConverterFactory()); + return built; + } + + /// + /// Registers a subclass defined outside this library, so the + /// platform stores can save and load it. + /// + /// The subclass to register. + /// + /// The name written into the stored JSON to identify . 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. + /// + /// + /// 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. + /// + /// + /// is empty or whitespace, is + /// abstract or is one of the library's own subclasses, is already + /// registered under a different discriminator, or the discriminator already names another + /// type. + /// + public static void RegisterDerivedType(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 updated = new(registeredTypes) { [type] = typeDiscriminator }; + options = BuildOptions(updated); + registeredTypes = updated; + } + } + + /// + /// Whether credentials of can be serialized, either because the + /// library declares it or because it was registered. + /// + internal static bool IsKnownType(Type type) + { + ArgumentNullException.ThrowIfNull(type); + return BuiltInTypes.ContainsKey(type) || registeredTypes.ContainsKey(type); + } + + /// + /// Throws naming 's type + /// when it is not one that can be serialized. + /// + 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)}."); + } } /// /// Serializes the credential to a UTF-8 JSON byte array. /// public static byte[] Serialize(Credential credential) => - JsonSerializer.SerializeToUtf8Bytes(credential, Options); + JsonSerializer.SerializeToUtf8Bytes(credential, options); /// /// Serializes the credential to a UTF-8 JSON string. /// public static string SerializeToString(Credential credential) => - JsonSerializer.Serialize(credential, Options); + JsonSerializer.Serialize(credential, options); /// /// Deserializes a credential from a UTF-8 JSON byte array. Returns null if the bytes @@ -53,7 +181,7 @@ public static string SerializeToString(Credential credential) => try { - return JsonSerializer.Deserialize(utf8Json, Options); + return JsonSerializer.Deserialize(utf8Json, options); } catch (JsonException) { @@ -122,7 +250,7 @@ internal static void Zero(byte[] buffer) try { - return JsonSerializer.Deserialize(utf8Json, Options); + return JsonSerializer.Deserialize(utf8Json, options); } catch (JsonException) { diff --git a/README.md b/README.md index f6fa590..0138425 100644 --- a/README.md +++ b/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("MyApp.CredentialWithCertificate"); // 3. Optional: register a factory so TryCreate works. public sealed class CertificateFactory : ICredentialFactory @@ -111,6 +111,8 @@ cache.RegisterCredentialFactory(new CertificateFactory()); If a subclass uses `SemanticString` 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):