From a596f22c058789d966645283ba9413c2b4b64efc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Nov 2026 22:27:57 +0000 Subject: [PATCH] Refuse an unregistered Credential subclass in every store's Save, not only AddOrReplace [patch] #194 added ThrowIfUnknownType, but only CredentialCache.AddOrReplace called it. A store used directly, through CredentialStoreFactory or CredentialCache.Instance.Store, skipped the check: each platform store's Save let System.Text.Json's raw NotSupportedException escape from Serialize, while InMemoryCredentialStore.Save accepted the credential. Tests against the in-memory store passed and production threw an exception type nothing around a store call expects. CredentialSerialization.Serialize and SerializeToString now call ThrowIfUnknownType first, so the Windows, macOS and Linux stores and any custom store built on those helpers refuse an unregistered subclass with the same CredentialStoreException and message as AddOrReplace. InMemoryCredentialStore.Save calls it too, and keeps nothing. Fixes ktsu-dev/CredentialCache#206 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW46SewAZ45qMGEBCjL --- .../CustomCredentialTypeTests.cs | 34 +++++++++++++++++++ .../Storage/CredentialSerialization.cs | 22 +++++++++--- .../Storage/InMemoryCredentialStore.cs | 4 +++ README.md | 2 +- 4 files changed, 57 insertions(+), 5 deletions(-) diff --git a/CredentialCache.Test/CustomCredentialTypeTests.cs b/CredentialCache.Test/CustomCredentialTypeTests.cs index 6fb842a..9934d77 100644 --- a/CredentialCache.Test/CustomCredentialTypeTests.cs +++ b/CredentialCache.Test/CustomCredentialTypeTests.cs @@ -81,6 +81,40 @@ public void AddOrReplaceRefusesAnUnregisteredSubclassAndLeavesTheCacheUnchanged( Assert.AreSame(original, stored); } + [TestMethod] + public void EveryStoreRefusesAnUnregisteredSubclassWithTheSameException() + { + NeverRegisteredCredential credential = new() { Value = "x" }; + using CredentialCache cache = new(new InMemoryCredentialStore()); + string expected = Assert.ThrowsExactly( + () => cache.AddOrReplace(NewPersona(), credential)).Message; + + InMemoryCredentialStore inMemory = new(); + PersonaGUID persona = NewPersona(); + CredentialStoreException fromInMemory = Assert.ThrowsExactly( + () => inMemory.Save(persona, credential)); + Assert.AreEqual(expected, fromInMemory.Message); + Assert.IsFalse(inMemory.TryLoad(persona, out _), "The refused credential should not have been kept."); + + // The platform store serializes before any native call, so this needs no keyring. Comparing + // the message tells the type refusal apart from a store that is merely unavailable. + ICredentialStore platform = CredentialStoreFactory.CreateDefault("ktsu.CredentialCache.Test.Unregistered"); + CredentialStoreException fromPlatform = Assert.ThrowsExactly( + () => platform.Save(NewPersona(), credential)); + Assert.AreEqual(expected, fromPlatform.Message); + } + + [TestMethod] + public void TheSerializationHelpersRefuseAnUnregisteredSubclass() + { + NeverRegisteredCredential credential = new() { Value = "x" }; + + Assert.Contains(nameof(NeverRegisteredCredential), Assert.ThrowsExactly( + () => CredentialSerialization.Serialize(credential)).Message); + Assert.Contains(nameof(NeverRegisteredCredential), Assert.ThrowsExactly( + () => CredentialSerialization.SerializeToString(credential)).Message); + } + [TestMethod] public void RegisteringTheSameTypeAndDiscriminatorAgainDoesNothing() { diff --git a/CredentialCache/Storage/CredentialSerialization.cs b/CredentialCache/Storage/CredentialSerialization.cs index 0faf895..2970627 100644 --- a/CredentialCache/Storage/CredentialSerialization.cs +++ b/CredentialCache/Storage/CredentialSerialization.cs @@ -159,14 +159,28 @@ internal static void ThrowIfUnknownType(Credential credential) /// /// Serializes the credential to a UTF-8 JSON byte array. /// - public static byte[] Serialize(Credential credential) => - JsonSerializer.SerializeToUtf8Bytes(credential, options); + /// + /// The credential's type is neither one of the library's own nor registered with + /// . + /// + public static byte[] Serialize(Credential credential) + { + ThrowIfUnknownType(credential); + return JsonSerializer.SerializeToUtf8Bytes(credential, options); + } /// /// Serializes the credential to a UTF-8 JSON string. /// - public static string SerializeToString(Credential credential) => - JsonSerializer.Serialize(credential, options); + /// + /// The credential's type is neither one of the library's own nor registered with + /// . + /// + public static string SerializeToString(Credential credential) + { + ThrowIfUnknownType(credential); + return JsonSerializer.Serialize(credential, options); + } /// /// Deserializes a credential from a UTF-8 JSON byte array. Returns null if the bytes diff --git a/CredentialCache/Storage/InMemoryCredentialStore.cs b/CredentialCache/Storage/InMemoryCredentialStore.cs index 29dfdfa..9d60696 100644 --- a/CredentialCache/Storage/InMemoryCredentialStore.cs +++ b/CredentialCache/Storage/InMemoryCredentialStore.cs @@ -27,6 +27,10 @@ public void Save(PersonaGUID persona, Credential credential) { ArgumentNullException.ThrowIfNull(persona); ArgumentNullException.ThrowIfNull(credential); + + // Refuse what a platform store could not serialize, so code tested against this store + // sees the same failure it would in production. + CredentialSerialization.ThrowIfUnknownType(credential); _items[persona] = credential; } diff --git a/README.md b/README.md index 0138425..765b966 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ 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. +`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. Calling `Save` on a store directly refuses it the same way, with the same exception and message, from the in-memory store and each platform store. So do `CredentialSerialization.Serialize` and `SerializeToString`, which means a custom store built on them refuses it too. ## Customising the backing store