Luhn invalid check logs warning

Author: jsed-nhs

src/api/Nhs.Appointments.Core/ReferenceNumber/V2/Provider.cs

@@ -4,6 +4,7 @@
 using System.Text.RegularExpressions;
 using LuhnNet;
 using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 
 namespace Nhs.Appointments.Core.ReferenceNumber.V2;
@@ -26,12 +27,15 @@ public class Provider(
     IOptions<ReferenceNumberOptions> options,
     IBookingReferenceDocumentStore bookingReferenceDocumentStore,
     IMemoryCache memoryCache,
+    ILogger<Provider> logger,
     TimeProvider timeProvider)
     : IProvider
 {
-    private Regex BookingReferenceV2Regex => new (@"^\d{4}-\d{5}-\d{4}$");
+    //needed for backwards compatibility since we are now checking correctly formatted strings
     private Regex BookingReferenceV1Regex => new (@"^\d{2}-\d{2}-\d{6}$");
 
+    private Regex BookingReferenceV2Regex => new (@"^\d{4}-\d{5}-\d{4}$");
+    
     private IOptions<ReferenceNumberOptions> Options { get; } = options;
 
     //in generating the partition key, splitting the current day of the year into X buckets of this length
@@ -111,7 +115,7 @@ private int GetSequenceMultiplier(string partitionKey)
         }
 
         sequenceMultiplier = DeriveSequenceMultiplier(partitionKey);
-        memoryCache.Set(cacheKey, sequenceMultiplier, TimeSpan.FromDays(PartitionBucketLengthInDays + 4)); // easily spans the partition length
+        memoryCache.Set(cacheKey, sequenceMultiplier, TimeSpan.FromDays(PartitionBucketLengthInDays + 2)); // easily spans the partition length
         return sequenceMultiplier;
     }
 
@@ -147,9 +151,13 @@ public int DeriveSequenceMultiplier(string partitionKey)
         //confirmation of GCD(multiplier,SequenceMax) == 1, without this, the entire bijection pattern fails!!
         if ((int)BigInteger.GreatestCommonDivisor(multiplier, SequenceMax) != 1)
         {
+            var message =
+                $"CRITICAL ERROR - Derived sequence multiplier does not pass logical requirement of GCD(multiplier,SequenceMax) == 1. " +
+                $"Failure for multiplier: '{multiplier}' and sequenceMax: '{SequenceMax}'";            
+            
             //fail fast - logically this should NEVER happen and something has gone VERY WRONG!
-            throw new InvalidOperationException($"CRITICAL ERROR - Derived sequence multiplier does not pass logical requirement of GCD(multiplier,SequenceMax) == 1. " +
-                                                $"Failure for multiplier: '{multiplier}' and sequenceMax: '{SequenceMax}'");
+            logger.LogCritical(message);
+            throw new InvalidOperationException(message);
         }
 
         //GCD(multiplier,SequenceMax) == 1 guaranteed
@@ -181,8 +189,11 @@ public bool IsValidBookingReference(string bookingReference)
 
         if (!Luhn.IsValid(digitReference))
         {
-            // Logger.LogWarning
-            // checksum fail indicates someone tried to guess a booking reference format using the valid format
+            // checksum fail indicates someone may have tried to guess a booking reference format using the valid format
+            // this could however also be a user typing error
+            
+            //if there are a lot of these warnings, could this suggest someone trying to brute force guess a valid reference...?
+            logger.LogWarning("Booking Reference '{BookingReference}' does not pass the valid Luhn digit requirement.", bookingReference);
             return false;
         }
 

tests/Nhs.Appointments.Core.UnitTests/ReferenceNumber/V2/ProviderTests.cs

@@ -1,4 +1,5 @@
 using Microsoft.Extensions.Caching.Memory;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Nhs.Appointments.Core.ReferenceNumber.V2;
 
@@ -16,14 +17,15 @@ public class ProviderTests
 
     private readonly Mock<IBookingReferenceDocumentStore> _bookingReferenceDocumentStore = new();
     private readonly Mock<TimeProvider> _timeProvider = new();
+    private readonly Mock<ILogger<Provider>> _logger = new();
     private readonly Mock<IOptions<ReferenceNumberOptions>> _options = new();
 
     private readonly IProvider _sut;
 
     public ProviderTests()
     {
         _options.Setup(x => x.Value).Returns(new ReferenceNumberOptions { HmacKeyVersion = 1, HmacKey = HmacTestSecretKey});
-        _sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, new MemoryCache(new MemoryCacheOptions()), _timeProvider.Object);
+        _sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, new MemoryCache(new MemoryCacheOptions()), _logger.Object, _timeProvider.Object);
     }
 
     [Fact]
@@ -135,26 +137,60 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_MinSequen
     }
 
     [Fact]
-    public async Task GetReferenceNumber_ChecksumDigitExists_MeansOnlyOneReferenceIsValid()
+    public async Task GetReferenceNumber_LuhnChecksumDigitExists_MeansOnlyOneReferenceIsValid()
     {
         _timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2016, 6, 06, 17, 23, 00));
         _bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(76543789);
 
         var result = await _sut.GetReferenceNumber();
         result.Should().Be("4016-90927-6174");
         Assert.True(_sut.IsValidBookingReference(result));
+        
+        _logger.Verify(x => x.Log(
+                LogLevel.Warning,
+                It.IsAny<EventId>(),
+                It.Is<It.IsAnyType>((state, t) =>
+                    state.ToString().Contains("Booking Reference '4016-90927-6174' does not pass the valid Luhn digit requirement.")
+                ),
+                It.IsAny<Exception>(),
+                It.IsAny<Func<It.IsAnyType, Exception, string>>()
+            ), Times.Never
+        );
 
         //the other 9 numbers with a different final digit should NOT be valid references
         Assert.False(_sut.IsValidBookingReference("4016-90927-6171"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6172"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6173"));
-
         Assert.False(_sut.IsValidBookingReference("4016-90927-6175"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6176"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6177"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6178"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6179"));
         Assert.False(_sut.IsValidBookingReference("4016-90927-6170"));
+        
+        VerifyLuhnDigitInvalidLogged("4016-90927-6171");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6172");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6173");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6175");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6176");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6177");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6178");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6179");
+        VerifyLuhnDigitInvalidLogged("4016-90927-6170");
+    }
+
+    private void VerifyLuhnDigitInvalidLogged(string bookingReference)
+    {
+        _logger.Verify(x => x.Log(
+                LogLevel.Warning,
+                It.IsAny<EventId>(),
+                It.Is<It.IsAnyType>((state, t) =>
+                    state.ToString().Contains($"Booking Reference '{bookingReference}' does not pass the valid Luhn digit requirement.")
+                ),
+                It.IsAny<Exception>(),
+                It.IsAny<Func<It.IsAnyType, Exception, string>>()
+            ), Times.Once
+        );
     }
 
     [Fact]