Skip to content

Commit ea865bf

Browse files
jsed-nhsjsed-nhs
committed
Fix tests
1 parent 0fc1478 commit ea865bf

3 files changed

Lines changed: 72 additions & 96 deletions

File tree

‎src/api/Nhs.Appointments.Core/ReferenceNumber/Options.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
namespace Nhs.Appointments.Core.ReferenceNumber;
22

3-
public abstract class ReferenceNumberOptions
3+
public class ReferenceNumberOptions
44
{
55
public int HmacKeyVersion { get; set; }
66
public byte[] HmacKey { get; set; }

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

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public interface IProvider
1414

1515
bool IsValidBookingReference(string bookingReference);
1616

17-
int DeriveSequenceStride(string partitionKey);
17+
int DeriveSequenceMultiplier(string partitionKey);
1818
}
1919

2020
public interface IBookingReferenceDocumentStore
@@ -71,7 +71,7 @@ private string Generate(int sequenceNumber, DateTimeOffset utcNow)
7171
{
7272
var overflowedSequenceNumber = sequenceNumber % SequenceMax;
7373
var partitionKey = PartitionKey(utcNow);
74-
var sequenceBijection = GenerateSequenceBijection(overflowedSequenceNumber, partitionKey)
74+
var sequenceBijection = GenerateSequenceValue(overflowedSequenceNumber, partitionKey)
7575
.ToString("D8");
7676
var partitionAndSequenceBijection = partitionKey + sequenceBijection;
7777

@@ -84,72 +84,75 @@ private string Generate(int sequenceNumber, DateTimeOffset utcNow)
8484
}
8585

8686
/// <summary>
87-
/// A bijection is a 1-1 mapping between two sets of numbers.
8887
/// This generation adds an element of seeming randomness to the sequence numbers generated, while still being
89-
/// deterministic and avoiding collisions.
88+
/// deterministic and avoiding collisions within 100 million sequence numbers (mod 100 million).
9089
/// This method means all 100 million sequence numbers will each generate a different and distinct number in that range. (0-99999999)
91-
/// Using a dynamically generated stride/increment means this cannot be guessed, as it is generated using a secret key and will change with each partition change.
90+
/// Uses a dynamically generated sequence increment multiplier which means this cannot be inferred, as it is generated using a secret key and will change with each partition change.
9291
/// This is needed to protect against sequential attacks visible in the reference numbers (i.e people guessing the next booking is
9392
/// exactly 1 reference number higher)
9493
/// </summary>
95-
private int GenerateSequenceBijection(int sequence, string partitionKey)
94+
private int GenerateSequenceValue(int sequence, string partitionKey)
9695
{
97-
var sequenceStride = GetSequenceStride(partitionKey);
96+
var sequenceMultiplier = GetSequenceMultiplier(partitionKey);
9897

99-
// f(i) = ( S * i + O ) mod N : is a bijection on (0, ..., N-1)
100-
// when the stride (S) is coprime with N (i.e gcd(S,N) = 1), and O is a constant offset (set to zero for us as no benefit)
101-
return (int)((long)sequenceStride * sequence % SequenceMax);
98+
// A bijection is a 1-1 mapping between two sets of numbers.
99+
// f(i) = ( S * i ) mod N : is a bijection on (0, ..., N-1) when the multiplier (S) is coprime with N (i.e gcd(S,N) = 1)
100+
101+
return (int)((long)sequenceMultiplier * sequence % SequenceMax);
102102
}
103103

104-
private int GetSequenceStride(string partitionKey)
104+
private int GetSequenceMultiplier(string partitionKey)
105105
{
106106
var cacheKey = $"{Options.Value.HmacKeyVersion}:{partitionKey}";
107-
if (memoryCache.TryGetValue(cacheKey, out int sequenceStride))
107+
if (memoryCache.TryGetValue(cacheKey, out int sequenceMultiplier))
108108
{
109-
return sequenceStride;
109+
return sequenceMultiplier;
110110
}
111111

112-
sequenceStride = DeriveSequenceStride(partitionKey);
113-
memoryCache.Set(cacheKey, sequenceStride, TimeSpan.FromDays(PartitionBucketLengthInDays + 4)); // easily spans the partition length
114-
return sequenceStride;
112+
sequenceMultiplier = DeriveSequenceMultiplier(partitionKey);
113+
memoryCache.Set(cacheKey, sequenceMultiplier, TimeSpan.FromDays(PartitionBucketLengthInDays + 4)); // easily spans the partition length
114+
return sequenceMultiplier;
115115
}
116116

117117
/// <summary>
118-
/// Derives the sequence stride used for the bijection.
119-
/// Generates a sequence stride that is COPRIME with SequenceMax, using the partitionKey and a secret key, and some manual manipulation to guarantee the value is coprime.
118+
/// Derives the sequence multiplier used for the bijection.
119+
/// Generates a sequence multiplier that is COPRIME with SequenceMax, using the partitionKey and a secret key, and some manual manipulation to guarantee the value is coprime.
120120
/// This doesn't have to generate unique results, two different partition keys and secrets can generate the same result.
121121
/// This just needs to be deterministic and non-guessable and guarded by a secret.
122122
/// It is used for obfuscation of the sequencing, not for security.
123123
/// </summary>
124-
public int DeriveSequenceStride(string partitionKey)
124+
public int DeriveSequenceMultiplier(string partitionKey)
125125
{
126+
//generate a hmacMultiplier that uses both the partition key and the hmac secret key
127+
//this can be any 64-bit integer
126128
using var h = new HMACSHA256(Options.Value.HmacKey);
127129
var mac = h.ComputeHash(Encoding.ASCII.GetBytes(partitionKey));
128-
var hmacStride = BitConverter.ToUInt64(mac, 0);
130+
var hmacMultiplier = BitConverter.ToUInt64(mac, 0);
129131

130-
// Base stride is (0,...,N-1)/10 - so multiplying by 10 still stays < N
131-
var baseStride = (int)(hmacStride % (SequenceMax / 10)); // 0,..., N-1
132+
//next make the hmacMultiplier be a value between 0 - 10 million (we're going to generate the final digit manually to make a number that is coprime with 100 million)
133+
var baseMultiplier = (int)(hmacMultiplier % (SequenceMax / 10));
132134

133135
//the following logic is dependent on SequenceMax being a number of format 10^x
134136

135137
// Pick a last digit from {1,3,7,9} deterministically (since any number ending in any of these are coprime to 100 million (sequence max))
136-
int[] tail = [1, 3, 7, 9];
138+
int[] lastDigitOptions = [1, 3, 7, 9];
137139

138-
//takes the last two bits of v (so a value 0–3) to pick the last digit deterministically
139-
var lastDigit = tail[(int)(hmacStride & 3)];
140+
//takes the last two bits of hmacMultiplier (so a value 0–3) to pick the last digit deterministically
141+
var lastDigit = lastDigitOptions[(int)(hmacMultiplier & 3)];
140142

141-
var stride = (baseStride * 10) + lastDigit; // 0,...,N-1 with last digit ending in either {1,3,7,9}
143+
// 0,...,999,999,999 with the last digit guaranteed to end in either 1,3,7,9. (i.e 672,509,093 where 67250909 was the hmacMultiplier)
144+
var multiplier = (baseMultiplier * 10) + lastDigit;
142145

143-
//confirmation of GCD(S,N) == 1, without this, the entire bijection pattern fails!!
144-
if ((int)BigInteger.GreatestCommonDivisor(stride, SequenceMax) != 1)
146+
//confirmation of GCD(multiplier,SequenceMax) == 1, without this, the entire bijection pattern fails!!
147+
if ((int)BigInteger.GreatestCommonDivisor(multiplier, SequenceMax) != 1)
145148
{
146-
//fail fast - logically this should NEVER happen
147-
throw new InvalidOperationException($"CRITICAL ERROR - Derived sequence stride does not pass logical requirement of GCD(stride,SequenceMax) == 1. " +
148-
$"Failure for stride: '{stride}' and sequenceMax: '{SequenceMax}'");
149+
//fail fast - logically this should NEVER happen and something has gone VERY WRONG!
150+
throw new InvalidOperationException($"CRITICAL ERROR - Derived sequence multiplier does not pass logical requirement of GCD(multiplier,SequenceMax) == 1. " +
151+
$"Failure for multiplier: '{multiplier}' and sequenceMax: '{SequenceMax}'");
149152
}
150153

151-
//GCD(S,N) == 1 guaranteed
152-
return stride;
154+
//GCD(multiplier,SequenceMax) == 1 guaranteed
155+
return multiplier;
153156
}
154157

155158
/// <summary>

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

Lines changed: 35 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ public class ProviderTests
2222

2323
public ProviderTests()
2424
{
25-
_options.Setup(x => x.Value.HmacKey).Returns(HmacTestSecretKey);
26-
_options.Setup(x => x.Value.HmacKeyVersion).Returns(1);
27-
28-
using var cache = new MemoryCache(new MemoryCacheOptions());
29-
_sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
25+
_options.Setup(x => x.Value).Returns(new ReferenceNumberOptions { HmacKeyVersion = 1, HmacKey = HmacTestSecretKey});
26+
_sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, new MemoryCache(new MemoryCacheOptions()), _timeProvider.Object);
3027
}
3128

3229
[Fact]
@@ -45,10 +42,7 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_MinDate()
4542
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(1970, 1, 1, 00, 00, 00));
4643
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(1);
4744

48-
using var cache = new MemoryCache(new MemoryCacheOptions());
49-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
50-
51-
var result = await sut.GetReferenceNumber();
45+
var result = await _sut.GetReferenceNumber();
5246
result.Should().Be("0170-73123-6791");
5347
}
5448

@@ -70,10 +64,7 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_FirstTwoD
7064
initialDate = initialDate.AddDays(1);
7165
_timeProvider.Setup(x => x.GetUtcNow()).Returns(initialDate);
7266

73-
using var cache = new MemoryCache(new MemoryCacheOptions());
74-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
75-
76-
var result = await sut.GetReferenceNumber();
67+
var result = await _sut.GetReferenceNumber();
7768

7869
Assert.True(_sut.IsValidBookingReference(result));
7970
result.Should().StartWith($"{dayStep:00}25");
@@ -85,11 +76,8 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_MaxDate_L
8576
{
8677
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2024, 12, 31, 23, 59, 59));
8778
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(99999999);
88-
89-
using var cache = new MemoryCache(new MemoryCacheOptions());
90-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
91-
92-
var result = await sut.GetReferenceNumber();
79+
80+
var result = await _sut.GetReferenceNumber();
9381

9482
Assert.True(_sut.IsValidBookingReference(result));
9583
result.Should().Be("9224-44976-9071");
@@ -100,11 +88,8 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_MinSequen
10088
{
10189
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2077, 1, 01, 17, 23, 00));
10290
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(0);
103-
104-
using var cache = new MemoryCache(new MemoryCacheOptions());
105-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
106-
107-
var result = await sut.GetReferenceNumber();
91+
92+
var result = await _sut.GetReferenceNumber();
10893
result.Should().Be("0177-00000-0006");
10994
}
11095

@@ -114,10 +99,7 @@ public async Task GetReferenceNumber_ChecksumDigitExists_MeansOnlyOneReferenceIs
11499
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2016, 6, 06, 17, 23, 00));
115100
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(76543789);
116101

117-
using var cache = new MemoryCache(new MemoryCacheOptions());
118-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
119-
120-
var result = await sut.GetReferenceNumber();
102+
var result = await _sut.GetReferenceNumber();
121103
result.Should().Be("4016-90927-6174");
122104
Assert.True(_sut.IsValidBookingReference(result));
123105

@@ -139,11 +121,8 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_MinSequen
139121
{
140122
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2077, 1, 01, 17, 23, 00));
141123
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(1);
142-
143-
using var cache = new MemoryCache(new MemoryCacheOptions());
144-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
145-
146-
var result = await sut.GetReferenceNumber();
124+
125+
var result = await _sut.GetReferenceNumber();
147126
result.Should().Be("0177-40950-2016");
148127
}
149128

@@ -152,17 +131,14 @@ public async Task GetReferenceNumber_GeneratesCorrectlyFormattedNumber_SameSeque
152131
{
153132
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(356035);
154133
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2025, 7, 13, 17, 23, 00));
155-
156-
using var cache = new MemoryCache(new MemoryCacheOptions());
157-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
158-
159-
var firstResult = await sut.GetReferenceNumber();
134+
135+
var firstResult = await _sut.GetReferenceNumber();
160136
firstResult.Should().Be("4925-52301-3450");
161137

162138
//sequence has passed 100 million and reset
163139
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(356035 + 100000000);
164140
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2025, 7, 17, 17, 23, 00));
165-
var secondResult = await sut.GetReferenceNumber();
141+
var secondResult = await _sut.GetReferenceNumber();
166142
secondResult.Should().Be("5025-82949-7652");
167143
}
168144

@@ -177,16 +153,13 @@ public async Task GetReferenceNumber_GeneratesSameReferenceNumber_SameSequenceNu
177153
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(356035);
178154
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2025, 7, 13, 17, 23, 00));
179155

180-
using var cache = new MemoryCache(new MemoryCacheOptions());
181-
var sut = new Provider(_options.Object, _bookingReferenceDocumentStore.Object, cache, _timeProvider.Object);
182-
183-
var firstResult = await sut.GetReferenceNumber();
156+
var firstResult = await _sut.GetReferenceNumber();
184157
firstResult.Should().Be("4925-52301-3450");
185158

186159
//sequence has passed 100 million and reset
187160
_bookingReferenceDocumentStore.Setup(x => x.GetNextSequenceNumber()).ReturnsAsync(356035 + 100000000);
188161
_timeProvider.Setup(x => x.GetUtcNow()).Returns(new DateTime(2025, 7, 13, 17, 23, 00));
189-
var secondResult = await sut.GetReferenceNumber();
162+
var secondResult = await _sut.GetReferenceNumber();
190163

191164
//NOT DESIRABLE!!
192165
secondResult.Should().Be(firstResult);
@@ -211,12 +184,12 @@ public void PartitionBucketLengthInDays_Should_Equal_Four()
211184
}
212185

213186
/// <summary>
214-
/// It is possible, but unlikely, that two different hmac keys produce the same stride value for the same partition key
187+
/// It is possible, but unlikely, that two different hmac keys produce the same multiplier value for the same partition key
215188
/// This is known and does not cause any issues with the logic.
216189
/// The hard-coded data for this test that proves this scenario was found by trial and error iterations.
217190
/// </summary>
218191
[Fact]
219-
public void DeriveSequenceStride_DifferentHmacKeys_CanProduce_SameStride_For_SamePartitionKey()
192+
public void DeriveSequenceMultiplier_DifferentHmacKeys_CanProduce_SameMultiplier_For_SamePartitionKey()
220193
{
221194
const string samePartitionKey = "3125";
222195

@@ -292,23 +265,23 @@ public void DeriveSequenceStride_DifferentHmacKeys_CanProduce_SameStride_For_Sam
292265
49
293266
];
294267

295-
_options.Setup(x => x.Value.HmacKey).Returns(hmacKey1);
296-
var stride1 = _sut.DeriveSequenceStride(samePartitionKey);
268+
_options.Setup(x => x.Value).Returns(new ReferenceNumberOptions { HmacKeyVersion = 1, HmacKey = hmacKey1});
269+
var multiplier1 = _sut.DeriveSequenceMultiplier(samePartitionKey);
297270

298-
_options.Setup(x => x.Value.HmacKey).Returns(hmacKey2);
299-
var stride2 = _sut.DeriveSequenceStride(samePartitionKey);
271+
_options.Setup(x => x.Value).Returns(new ReferenceNumberOptions { HmacKeyVersion = 1, HmacKey = hmacKey2});
272+
var multiplier2 = _sut.DeriveSequenceMultiplier(samePartitionKey);
300273

301274
//this is fine
302-
Assert.Equal(stride1, stride2);
275+
Assert.Equal(multiplier1, multiplier2);
303276
}
304277

305278
/// <summary>
306-
/// It is possible, but unlikely, that two different hmac keys produce the same stride value for the same partition key
279+
/// It is possible, but unlikely, that two different hmac keys produce the same multiplier value for the same partition key
307280
/// This is known and does not cause any issues with the logic.
308281
/// The hard-coded data for this test that proves this scenario was found by trial and error iterations.
309282
/// </summary>
310283
[Fact]
311-
public void DeriveSequenceStride_DifferentHmacKeys_CanProduce_SameStride_For_DifferentPartitionKeys()
284+
public void DeriveSequenceMultiplier_DifferentHmacKeys_CanProduce_SameMultiplier_For_DifferentPartitionKeys()
312285
{
313286
byte[] hmacKey1 =
314287
[
@@ -346,9 +319,9 @@ public void DeriveSequenceStride_DifferentHmacKeys_CanProduce_SameStride_For_Dif
346319
6
347320
];
348321

349-
_options.Setup(x => x.Value.HmacKey).Returns(hmacKey1);
322+
_options.Setup(x => x.Value).Returns(new ReferenceNumberOptions { HmacKeyVersion = 1, HmacKey = hmacKey1});
350323
var partitionKey1 = "5728"; //around about October 15th 2028
351-
var stride1 = _sut.DeriveSequenceStride(partitionKey1);
324+
var multiplier1 = _sut.DeriveSequenceMultiplier(partitionKey1);
352325

353326
byte[] hmacKey2 =
354327
[
@@ -386,29 +359,29 @@ public void DeriveSequenceStride_DifferentHmacKeys_CanProduce_SameStride_For_Dif
386359
107
387360
];
388361

389-
_options.Setup(x => x.Value.HmacKey).Returns(hmacKey2);
362+
_options.Setup(x => x.Value).Returns(new ReferenceNumberOptions { HmacKeyVersion = 1, HmacKey = hmacKey2});
390363
var partitionKey2 = "5837"; //around about August 20th 2037
391-
var stride2 = _sut.DeriveSequenceStride(partitionKey2);
364+
var multiplier2 = _sut.DeriveSequenceMultiplier(partitionKey2);
392365

393-
Assert.Equal(stride1, stride2);
366+
Assert.Equal(multiplier1, multiplier2);
394367
}
395368

396369
/// <summary>
397-
/// It is possible, but unlikely, that two different partition keys produce the same stride value for the same hmac key
370+
/// It is possible, but unlikely, that two different partition keys produce the same multiplier value for the same hmac key
398371
/// This is known and does not cause any issues with the logic.
399372
/// The hard-coded data for this test that proves this scenario was found by trial and error iterations.
400373
/// </summary>
401374
[Fact]
402-
public void DeriveSequenceStride_SameHmacKey_CanProduce_SameStride_For_DifferentPartitionKeys()
375+
public void DeriveSequenceMultiplier_SameHmacKey_CanProduce_SameMultiplier_For_DifferentPartitionKeys()
403376
{
404377
var partitionKey1 = "1947"; //around about March 18th 2047
405378
var partitionKey2 = "2538"; //around about April 10th 2038
406379

407-
var stride1 = _sut.DeriveSequenceStride(partitionKey1);
408-
var stride2 = _sut.DeriveSequenceStride(partitionKey2);
380+
var multiplier1 = _sut.DeriveSequenceMultiplier(partitionKey1);
381+
var multiplier2 = _sut.DeriveSequenceMultiplier(partitionKey2);
409382

410383
//this is fine
411-
Assert.Equal(stride1, stride2);
384+
Assert.Equal(multiplier1, multiplier2);
412385
}
413386

414387
/// <summary>

0 commit comments

Comments
 (0)