Fix touchAfter handling (#17)

- This was not working because `expires` is a field in `Item` but only `Item.sess` is available to the `touch` function
pwrdrvr · Jul 28, 2023 · 835ea53 · 835ea53
1 parent 8780bec
commit 835ea53
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 75 deletions.
diff --git a/README.md b/README.md
@@ -169,7 +169,9 @@ Disclaimer: perform your own pricing calculation, monitor your costs during and
 
 # Running Examples
 
-## [express](./examples/express)
+## express
+
+Source: [./examples/express.ts](./examples/express.ts)
 
 1. Create DynamoDB Table using AWS Console or any other method
    1. AWS CLI Example: ```aws dynamodb create-table --table-name dynamodb-session-store-test --attribute-definitions AttributeName=id,AttributeType=S --key-schema AttributeName=id,KeyType=HASH --billing-mode PAY_PER_REQUEST```
@@ -184,7 +186,9 @@ Disclaimer: perform your own pricing calculation, monitor your costs during and
 3. Load `http://localhost:3001/login` in a browser
 4. Observe that a cookie is returned and does not change
 
-## [cross-account](./examples/cross-account)
+## cross-account
+
+Source: [./examples/cross-account.ts](./examples/cross-account.ts)
 
 This example has the DynamoDB in one account and the express app using an IAM role from another account to access the DynamoDB Table using temporary credentials from an STS AssumeRole call (neatly encapsulated by the AWS SDK for JS v3).
 
@@ -194,7 +198,9 @@ This example is more involved than the others as it requires setting up an IAM r
 
 ![Session Store with DynamoDB Table in Another Account](https://github.com/pwrdrvr/dynamodb-session-store/assets/5617868/dbc8d07b-b2f3-42c8-96c9-2476007ed24c)
 
-## [express with dynamodb-connect module - for comparison](./examples/other)
+## express with dynamodb-connect module - for comparison
+
+Source: [./examples/other.ts](./examples/other.ts)
 
 1. Create DynamoDB Table using AWS Console or any other method
    1. AWS CLI Example: ```aws dynamodb create-table --table-name connect-dynamodb-test --attribute-definitions AttributeName=id,AttributeType=S --key-schema AttributeName=id,KeyType=HASH --billing-mode PAY_PER_REQUEST```

diff --git a/src/dynamodb-store.mock.spec.ts b/src/dynamodb-store.mock.spec.ts
@@ -168,7 +168,66 @@ describe('mock AWS API', () => {
   });
 
   describe('ttl', () => {
-    it('does not update the TTL if the session is not close to expiration', (done) => {
+    it('does not update the TTL if the session was recently modified', (done) => {
+      void (async () => {
+        dynamoClient
+          .onAnyCommand()
+          .callsFake((input) => {
+            console.log('dynamoClient.onAnyCommand', input);
+            throw new Error('unexpected call');
+          })
+          .rejects()
+          .on(dynamodb.DescribeTableCommand, {
+            TableName: tableName,
+          })
+          .resolves({
+            Table: {
+              TableName: tableName,
+            },
+          })
+          .on(dynamodb.CreateTableCommand, {
+            TableName: tableName,
+          })
+          .rejects();
+        ddbMock.onAnyCommand().callsFake((input) => {
+          console.log('ddbMock.onAnyCommand', input);
+          throw new Error('unexpected call');
+        });
+
+        const store = await DynamoDBStore.create({
+          tableName,
+          createTableOptions: {},
+        });
+
+        expect(store.tableName).toBe(tableName);
+        expect(dynamoClient.calls().length).toBe(1);
+        expect(ddbMock.calls().length).toBe(0);
+
+        store.touch(
+          '123',
+          {
+            // @ts-expect-error we know we have a cookie field
+            user: 'test',
+            lastModified: new Date().toISOString(),
+            cookie: {
+              originalMaxAge: 1000 * (14 * 24) * 60 * 60,
+              expires: new Date(Date.now() + 1000 * (14 * 24 - 0.9) * 60 * 60),
+            },
+          },
+          (err) => {
+            expect(err).toBeNull();
+
+            // Nothing should have happened
+            expect(dynamoClient.calls().length).toBe(1);
+            expect(ddbMock.calls().length).toBe(0);
+
+            done();
+          },
+        );
+      })();
+    });
+
+    it('does update the TTL if the session was last modified more than touchAfter seconds ago', (done) => {
       void (async () => {
         dynamoClient
           .onAnyCommand()
@@ -196,28 +255,19 @@ describe('mock AWS API', () => {
             throw new Error('unexpected call');
           })
           .on(
-            GetCommand,
+            UpdateCommand,
             {
-              TableName: tableName,
-              Key: {
-                id: {
-                  S: 'sess:123',
-                },
-              },
+              TableName: 'sessions-test',
+              Key: { id: 'session#123' },
+              UpdateExpression: 'set expires = :e, sess.lastModified = :lm',
+              // ExpressionAttributeValues: { ':e': 2898182909 },
+              ReturnValues: 'UPDATED_NEW',
             },
             false,
           )
-          .resolves({
-            Item: {
-              id: {
-                S: 'sess:123',
-              },
-              expires: {
-                N: '1598420000',
-              },
-              data: {
-                B: 'eyJ1c2VyIjoiYWRtaW4ifQ==',
-              },
+          .resolvesOnce({
+            Attributes: {
+              expires: 2898182909,
             },
           });
 
@@ -233,27 +283,30 @@ describe('mock AWS API', () => {
         store.touch(
           '123',
           {
-            user: 'test',
             // @ts-expect-error we know we have a cookie field
+            user: 'test',
+            expires: Math.floor((Date.now() + 1000 * (14 * 24 - 1.1) * 60 * 60) / 1000),
+            lastModified: '2021-08-01T00:00:00.000Z',
             cookie: {
-              expires: new Date(Date.now() + 1000 * (14 * 24 - 0.9) * 60 * 60),
+              maxAge: 1000 * (14 * 24 - 4) * 60 * 60,
+              originalMaxAge: 1000 * (14 * 24) * 60 * 60,
+              expires: new Date(Date.now() + 1000 * (14 * 24 - 4) * 60 * 60),
             },
-            expires: Math.floor(Date.now() + 1000 * (14 * 24 - 0.9) * 60 * 60),
           },
           (err) => {
             expect(err).toBeNull();
 
-            // Nothing should have happened
+            // We should have written to the DB
             expect(dynamoClient.calls().length).toBe(1);
-            expect(ddbMock.calls().length).toBe(0);
+            expect(ddbMock.calls().length).toBe(1);
 
             done();
           },
         );
       })();
     });
 
-    it('does update the TTL if the session was last touched more than touchAfter seconds ago', (done) => {
+    it('does update the TTL if the session has no lastModified field', (done) => {
       void (async () => {
         dynamoClient
           .onAnyCommand()
@@ -280,37 +333,12 @@ describe('mock AWS API', () => {
             console.log('ddbMock.onAnyCommand', input);
             throw new Error('unexpected call');
           })
-          .on(
-            GetCommand,
-            {
-              TableName: tableName,
-              Key: {
-                id: {
-                  S: 'sess:123',
-                },
-              },
-            },
-            false,
-          )
-          .resolves({
-            Item: {
-              id: {
-                S: 'sess:123',
-              },
-              expires: {
-                N: '1598420000',
-              },
-              data: {
-                B: 'eyJ1c2VyIjoiYWRtaW4ifQ==',
-              },
-            },
-          })
           .on(
             UpdateCommand,
             {
               TableName: 'sessions-test',
               Key: { id: 'session#123' },
-              UpdateExpression: 'set expires = :e',
+              UpdateExpression: 'set expires = :e, sess.lastModified = :lm',
               // ExpressionAttributeValues: { ':e': 2898182909 },
               ReturnValues: 'UPDATED_NEW',
             },

diff --git a/src/dynamodb-store.ts b/src/dynamodb-store.ts
@@ -469,6 +469,8 @@ export class DynamoDBStore extends session.Store {
               ...(session.cookie
                 ? { cookie: { ...JSON.parse(JSON.stringify(session.cookie)) } }
                 : {}),
+              // Add last-modified if touchAfter is set
+              ...(this.touchAfter > 0 ? { lastModified: new Date().toISOString() } : {}),
             },
           },
         });
@@ -497,21 +499,21 @@ export class DynamoDBStore extends session.Store {
     /**
      * Session data
      */
-    session: session.SessionData,
+    session: session.SessionData & { lastModified?: string },
     /**
      * Callback to return an error if the session TTL was not updated
      */
     callback?: (err?: unknown) => void,
   ): void {
     void (async () => {
       try {
-        // @ts-expect-error expires may exist
-        const expiresTimeSecs = session.expires ? session.expires : 0;
+        // The `expires` field from the DB `Item` is not available here
+        // when a session is loaded from the store
+        // We have to use a `lastModified` field within the user-visible session
         const currentTimeSecs = Math.floor(Date.now() / 1000);
-
-        // Compute how much time has passed since this session was last touched
-        const timePassedSecs =
-          currentTimeSecs + session.cookie.originalMaxAge / 1000 - expiresTimeSecs;
+        const lastModifiedSecs = session.lastModified
+          ? Math.floor(new Date(session.lastModified).getTime() / 1000)
+          : 0;
 
         // Update the TTL only if touchAfter
         // seconds have passed since the TTL was last updated
@@ -521,21 +523,33 @@ export class DynamoDBStore extends session.Store {
             ? Math.floor(0.1 * (session.cookie.originalMaxAge / 1000))
             : this._touchAfter;
 
-        if (timePassedSecs > touchAfterSecsCapped) {
-          const newExpires = this.newExpireSecondsSinceEpochUTC(session);
-
-          await this._ddbDocClient.update({
-            TableName: this._tableName,
-            Key: {
-              [this._hashKey]: `${this._prefix}${sid}`,
-            },
-            UpdateExpression: 'set expires = :e',
-            ExpressionAttributeValues: {
-              ':e': newExpires,
-            },
-            ReturnValues: 'UPDATED_NEW',
-          });
+        const timeElapsed = currentTimeSecs - lastModifiedSecs;
+        if (timeElapsed < touchAfterSecsCapped) {
+          debug(`Skip touching session=${sid}`);
+          if (callback) {
+            callback(null);
+          }
+          return;
         }
+
+        // We are going to touch the session, update the lastModified
+        session.lastModified = new Date().toISOString();
+
+        const newExpires = this.newExpireSecondsSinceEpochUTC(session);
+
+        await this._ddbDocClient.update({
+          TableName: this._tableName,
+          Key: {
+            [this._hashKey]: `${this._prefix}${sid}`,
+          },
+          UpdateExpression: 'set expires = :e, sess.lastModified = :lm',
+          ExpressionAttributeValues: {
+            ':e': newExpires,
+            ':lm': session.lastModified,
+          },
+          ReturnValues: 'UPDATED_NEW',
+        });
+
         if (callback) {
           callback(null);
         }
@@ -588,4 +602,10 @@ export class DynamoDBStore extends session.Store {
         : +new Date() + 60 * 60 * 24 * 1000;
     return Math.floor(expires / 1000);
   }
+
+  private getTTLSeconds(sess: session.SessionData) {
+    return sess && sess.cookie && sess.cookie.expires
+      ? Math.ceil((Number(new Date(sess.cookie.expires)) - Date.now()) / 1000)
+      : this._touchAfter;
+  }
 }