Refuse a null non-nullable credential property when reading or saving [patch]

CredentialCache.Test/NullPropertyTests.cs

@@ -0,0 +1,84 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.CredentialCache.Test;
+
+using System.Text;
+using ktsu.CredentialCache.Storage;
+using ktsu.Semantics.Strings;
+
+/// <summary>
+/// Tests that a credential whose non-nullable property is null is neither read back as found
+/// nor written to a store, so no caller is handed a credential that throws on first use.
+/// </summary>
+[TestClass]
+public class NullPropertyTests
+{
+	private const string NullToken = "{\"$type\":\"CredentialWithToken\",\"Token\":null}";
+	private const string NullUsername = "{\"$type\":\"CredentialWithUsernamePassword\",\"Username\":null,\"Password\":\"hunter2\"}";
+	private const string NullPassword = "{\"$type\":\"CredentialWithUsernamePassword\",\"Username\":\"alice\",\"Password\":null}";
+
+	[TestMethod]
+	[DataRow(NullToken)]
+	[DataRow(NullUsername)]
+	[DataRow(NullPassword)]
+	public void DeserializeReturnsNullForANullNonNullableProperty(string json) =>
+		Assert.IsNull(CredentialSerialization.Deserialize(Encoding.UTF8.GetBytes(json)));
+
+	[TestMethod]
+	[DataRow(NullToken)]
+	[DataRow(NullUsername)]
+	[DataRow(NullPassword)]
+	public void DeserializeFromStringReturnsNullForANullNonNullableProperty(string json) =>
+		Assert.IsNull(CredentialSerialization.DeserializeFromString(json));
+
+	[TestMethod]
+	[DataRow("{\"$type\":\"CredentialWithToken\",\"Token\":\"abc\"}")]
+	[DataRow("{\"$type\":\"CredentialWithUsernamePassword\",\"Username\":\"alice\",\"Password\":\"hunter2\"}")]
+	[DataRow("{\"$type\":\"CredentialWithNothing\"}")]
+	public void DeserializeStillReadsCompleteCredentials(string json) =>
+		Assert.IsNotNull(CredentialSerialization.Deserialize(Encoding.UTF8.GetBytes(json)));
+
+	[TestMethod]
+	[DataRow(NullToken)]
+	[DataRow(NullUsername)]
+	[DataRow(NullPassword)]
+	public void TryGetReturnsFalseForAStoredEntryWithANullNonNullableProperty(string json)
+	{
+		RawBlobCredentialStore store = new();
+		using CredentialCache cache = new(store);
+		PersonaGUID persona = CredentialCache.CreatePersonaGUID();
+		store.Blobs[persona] = Encoding.UTF8.GetBytes(json);
+
+		bool found = cache.TryGet(persona, out Credential? credential);
+
+		Assert.IsFalse(found);
+		Assert.IsNull(credential);
+	}
+
+	[TestMethod]
+	public void SerializeRejectsANullNonNullableProperty()
+	{
+		CredentialWithToken credential = new() { Token = null! };
+
+		Assert.ThrowsExactly<ArgumentException>(() => CredentialSerialization.Serialize(credential));
+		Assert.ThrowsExactly<ArgumentException>(() => CredentialSerialization.SerializeToString(credential));
+	}
+
+	[TestMethod]
+	public void AddOrReplaceRejectsANullNonNullablePropertyAndLeavesTheStoreAndCacheUnchanged()
+	{
+		RawBlobCredentialStore store = new();
+		using CredentialCache cache = new(store);
+		PersonaGUID persona = CredentialCache.CreatePersonaGUID();
+		CredentialWithUsernamePassword credential = new()
+		{
+			Username = SemanticString<CredentialUsername>.Create("alice"),
+			Password = null!,
+		};
+
+		Assert.ThrowsExactly<ArgumentException>(() => cache.AddOrReplace(persona, credential));
+
+		Assert.IsFalse(store.Blobs.ContainsKey(persona));
+		Assert.IsFalse(cache.TryGet(persona, out _));
+	}
+}

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
@@ -197,6 +197,10 @@
 	/// registered with <see cref="CredentialSerialization.RegisterDerivedType{T}(string)"/>. The
 	/// cache and the store are left unchanged.
 	/// </exception>
+	/// <exception cref="ArgumentException">
+	/// The store serializes credentials and <paramref name="credential"/> has a non-nullable
+	/// property set to <c>null</c>. The cache and the store are left unchanged.
+	/// </exception>
 	public void AddOrReplace(PersonaGUID persona, Credential credential)
 	{
 		ArgumentNullException.ThrowIfNull(persona);

CredentialCache/Storage/CredentialSerialization.cs

@@ -72,6 +72,10 @@ private static JsonSerializerOptions BuildOptions(Dictionary<Type, string> deriv
 		// Persuade System.Text.Json to use the SemanticString factory methods (Create / FromString)
 		// rather than treating SemanticString<T> as an IEnumerable<char> collection.
 		built.Converters.Add(new RoundTripStringJsonConverterFactory());
+		// Refuse a JSON null for a non-nullable property, in both directions. Otherwise a stored
+		// "Token":null reads back as a found credential whose Token throws on first use, and a
+		// credential with a null! property is persisted and round-trips.
+		built.RespectNullableAnnotations = true;
 		return built;
 	}
 
@@ -169,10 +173,22 @@ internal static void ThrowIfUnknownType(Credential credential)
 	/// The credential's type is neither one of the library's own nor registered with
 	/// <see cref="RegisterDerivedType{T}(string)"/>.
 	/// </exception>
+	/// <exception cref="ArgumentException">
+	/// <paramref name="credential"/> has a non-nullable property set to <c>null</c>, so it could
+	/// not be read back.
+	/// </exception>
 	public static byte[] Serialize(Credential credential)
 	{
 		ThrowIfUnknownType(credential);
-		return JsonSerializer.SerializeToUtf8Bytes(credential, options);
+
+		try
+		{
+			return JsonSerializer.SerializeToUtf8Bytes(credential, options);
+		}
+		catch (JsonException exception)
+		{
+			throw NotSerializable(credential, exception);
+		}
 	}
 
 	/// <summary>
@@ -182,12 +198,27 @@ public static byte[] Serialize(Credential credential)
 	/// The credential's type is neither one of the library's own nor registered with
 	/// <see cref="RegisterDerivedType{T}(string)"/>.
 	/// </exception>
+	/// <exception cref="ArgumentException">
+	/// <paramref name="credential"/> has a non-nullable property set to <c>null</c>, so it could
+	/// not be read back.
+	/// </exception>
 	public static string SerializeToString(Credential credential)
 	{
 		ThrowIfUnknownType(credential);
-		return JsonSerializer.Serialize(credential, options);
+
+		try
+		{
+			return JsonSerializer.Serialize(credential, options);
+		}
+		catch (JsonException exception)
+		{
+			throw NotSerializable(credential, exception);
+		}
 	}
 
+	private static ArgumentException NotSerializable(Credential credential, JsonException exception) =>
+		new($"{credential.GetType()} cannot be stored: {exception.Message}", nameof(credential), exception);
+
 	/// <summary>
 	/// Deserializes a credential from a UTF-8 JSON byte array. Returns <c>null</c> if the bytes
 	/// do not represent a known credential.