Ensure expired (but in db) doc registration record is updated.

CHANGELOG.md

@@ -1,5 +1,12 @@
 # bedrock-tokenization ChangeLog
 
+## 21.1.1 - 2024-mm-dd
+
+### Fixed
+- Ensure an expired (but still present in the database) document
+  registration record will be updated when a registration attempt using
+  a matching document is attempted.
+
 ## 21.1.0 - 2023-10-12
 
 ### Added

lib/documents.js

@@ -338,6 +338,11 @@ export async function register({
 
     // at this point the registration is known to be new/not exist, so
     // throw an error if it was expected to exist
+    // FIXME: current implementation could trigger a 404 when the update
+    // modifies no document because of an exactly simultaneous concurrent
+    // update; this should be handled gracefully externally but could perhaps
+    // be improved here by checking for `record` above instead of `updated`
+    // (if that logic is appropriate); this might also simplify logic below
     if(newRegistration === false) {
       const details = {
         httpStatusCode: 404,
@@ -348,18 +353,37 @@ export async function register({
         'NotFoundError', details);
     }
 
-    /* Note: At this point, either a document associated with the external ID
-    and document hash doesn't exist or the same expiration date *happened* to
-    be set by another concurrent process in the above `refresh` call, resulting
-    in this process not seeing an update to the existing record there.
+    /* Note: At this point, we expect that we need to insert a document
+    registration record because either:
+
+    1. A document registration record associated with the external ID and
+    document hash doesn't exist,
+
+    2. It does exist in the database but has been marked as expired (and the
+    external caller may or may not have set `newRegistration=true` on this
+    basis),
+
+    3. The same expiration date *happened* to be set by another concurrent
+    process in the above `refresh` call, resulting in this process not seeing
+    any records being modified in the above update call.
 
     We assume the more common case that there is no such document with the
     given `externalIdHash` and `documentHash` and go ahead and generate an
-    `internalId` for inserting the document and encrypt it. If incorrect,
-    an entity with the `internalId` will be inserted (and can be safely ignored
-    and allowed to expire), but the document registration record will not be
-    inserted due to a duplicate error (which also means that the encrypted data
-    won't be used and will be safely discarded). */
+    `internalId` for the document and encrypt it. We try to upsert an entity
+    record with that `internalId` and then try to insert the document
+    registration record.
+
+    Now, if our assumption is incorrect (and therefore some matching document
+    registration record exists), the entity record with the given `internalId`
+    that was inserted can be safely ignored and allowed to expire. The document
+    registration record will not be inserted due to a duplicate error (which
+    also means that the encrypted data won't be used and will be safely
+    discarded).
+
+    If this duplicate error occurs, we need to clear any assumption that a
+    new registration is being made, clear any `internalId` generated, and loop
+    to ensure to ensure that the document registration record (if expired) is
+    updated with a new expiration date. */
 
     // 4. Generate a new random internal ID to associate with `externalId` if
     //    none has been set yet.
@@ -402,7 +426,12 @@ export async function register({
         throw e;
       }
       // document already exists with matching `documentHash` and
-      // `externalIdHash`; loop to refresh it
+      // `externalIdHash`; loop to refresh it, but ensure it isn't treated
+      // as a new registration (this handles the case of an expired
+      // registration record that wasn't detected by an external caller of
+      // this API)
+      newRegistration = undefined;
+      internalId = undefined;
       continue;
     }
   }

lib/tokens/index.js

@@ -151,7 +151,8 @@ export async function registerDocumentAndCreate({
       })
     ]);
 
-    // if `internalId` does not match, then we must try again
+    // if `internalId` does not match, then we must try again to ensure the
+    // tokens match the registration record
     if(registrationResult.registration.internalId.compare(internalId) !== 0) {
       continue;
     }

test/mocha/20-tokens.js

@@ -868,6 +868,94 @@ describe('Tokens', function() {
       tokenResult.should.include.keys(['tokens', 'validUntil']);
       tokenResult.validUntil.should.be.a('Date');
     });
+  it('should register expired duplicate and create token concurrently',
+    async function() {
+      const dateOfBirth = '2000-05-01';
+      const expires = '2021-05-01';
+      const identifier = 'T99991234';
+      const issuer = 'VA';
+      const type = 'DriversLicense';
+      const recipients = [
+        {
+          header: {
+            kid: 'did:key:z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoA' +
+              'nwWsdvktH#z6LSbysY2xFMRpGMhb7tFTLMpeuPRaqaWM1yECx2AtzE3KCc',
+            alg: 'ECDH-ES+A256KW',
+          }
+        }
+      ];
+      const tokenCount = 1;
+      // canonicalize object then hash it then base58 encode it
+      const externalId = encode(crypto.createHash('sha256')
+        .update(canonicalize({dateOfBirth, identifier, issuer}))
+        .digest());
+
+      let registrationRecord;
+      {
+        let tokenResult;
+        let err;
+        try {
+          tokenResult = await tokens.registerDocumentAndCreate({
+            registerOptions: {
+              externalId,
+              document: {dateOfBirth, expires, identifier, issuer, type},
+              recipients,
+              ttl: 1209600000
+            },
+            tokenCount
+          });
+        } catch(e) {
+          err = e;
+        }
+        assertNoError(err);
+        should.exist(tokenResult);
+        tokenResult.should.include.keys(['tokens', 'validUntil']);
+        tokenResult.validUntil.should.be.a('Date');
+        ({registrationRecord} = tokenResult);
+      }
+
+      // now mark registration record expired and register again w/ success
+      const {registration} = registrationRecord;
+      const collection = database.collections['tokenization-registration'];
+      const yesterday = new Date();
+      yesterday.setDate(yesterday.getDate() - 1);
+      const updateResult = await collection.updateOne({
+        'registration.externalIdHash': registration.externalIdHash,
+        'registration.documentHash': registration.documentHash
+      }, {
+        $set: {'registration.expires': yesterday}
+      });
+      updateResult.result.nModified.should.equal(1);
+
+      // now re-registration should update expired registration record
+      {
+        let tokenResult;
+        let err;
+        try {
+          tokenResult = await tokens.registerDocumentAndCreate({
+            registerOptions: {
+              externalId,
+              document: {dateOfBirth, expires, identifier, issuer, type},
+              recipients,
+              ttl: 1209600000
+            },
+            tokenCount
+          });
+        } catch(e) {
+          err = e;
+        }
+        assertNoError(err);
+        should.exist(tokenResult);
+        tokenResult.should.include.keys(['tokens', 'validUntil']);
+        tokenResult.validUntil.should.be.a('Date');
+        tokenResult.registrationRecord.registration.externalIdHash.should
+          .deep.equal(registrationRecord.registration.externalIdHash);
+        tokenResult.registrationRecord.registration.documentHash.should
+          .deep.equal(registrationRecord.registration.documentHash);
+        tokenResult.registrationRecord.registration.expires.should.not.equal(
+          yesterday);
+      }
+    });
   it('should not resolve token from invalidated batch',
     async function() {
       // create tokens