Applying comments from review

packages/apps/shopify-api/docs/guides/session-storage.md

@@ -1,22 +1,16 @@
 # Storing sessions
 
-As of v6 of the library, there are no `SessionStorage` implementations included and the responsibility for implementing session storage is now delegated to the application.
+In order to be able to load user data without using cookies, your app will need to store its access tokens and other relevant data.
 
-The previous implementations of `SessionStorage` are now available in their own packages, the source of which is available in the [respective directory](../../../session-storage#readme).
+This package provides a `SessionStorage` interface that makes it easy to plug in a new storage strategy to your app.
+You can use of one the [packages we provide](/packages/apps/session-storage/README.md), or implement your own following the instructions in this page.
 
-|                      Package                      |  Session storage object  | Notes                                    |
-| :-----------------------------------------------: | :----------------------: | ---------------------------------------- |
-|   `@shopify/shopify-app-session-storage-memory`   |   MemorySessionStorage   |                                          |
-|  `@shopify/shopify-app-session-storage-mongodb`   |  MongoDBSessionStorage   |                                          |
-|   `@shopify/shopify-app-session-storage-mysql`    |   MySQLSessionStorage    |                                          |
-| `@shopify/shopify-app-session-storage-postgresql` | PostgreSQLSessionStorage |                                          |
-|   `@shopify/shopify-app-session-storage-redis`    |   RedisSessionStorage    |                                          |
-|   `@shopify/shopify-app-session-storage-sqlite`   |   SQLiteSessionStorage   |                                          |
-|  `@shopify/shopify-app-session-storage-dynamodb`  |  DynamoDBSessionStorage  |                                          |
-|     `@shopify/shopify-app-session-storage-kv`     |     KVSessionStorage     |                                          |
-|      `@shopify/shopify-app-session-storage`       |      SessionStorage      | Abstract class used by the classes above |
+In this page, you'll file:
 
-## Basics
+- [What data is in a `Session` object?](#what-data-is-in-a-session-object)
+- [Save a session to storage](#save-a-session-to-storage)
+- [Load a session from storage](#load-a-session-from-storage)
+- [Encrypting data for storage](#encrypting-data-for-storage)
 
 ### What data is in a `Session` object?
 
@@ -294,12 +288,35 @@ const session = Session.fromPropertyArray(sessionProperties);
 
 ### Encrypting data for storage
 
-If you want to encrypt your access tokens before storing them, the `Session` class provides two methods: `fromEncryptedPropertyArray` and `toEncryptedPropertyArray`.
+If you want to encrypt your sessions before storing them, the `Session` class provides two methods: `fromEncryptedPropertyArray` and `toEncryptedPropertyArray`.
 
-These behave the same as their non-encrypted counterparts, and take in a `CryptoKey` object. If a session is currently not encrypted in storage, these methods will still load it normally, and will encrypt them when saving them.
-Currently, only the `accessToken` is encrypted.
+These behave the same as their non-encrypted counterparts, but are `async` and take in a `CryptoKey` object.
+If a session is currently not encrypted in storage, these methods will still load it normally, and will encrypt them when saving them.
 
 `fromEncryptedPropertyArray` will return ciphers for the encrypted fields, prefixed with `encrypted#`.
 That way, storage providers can progressively update sessions as they're loaded and saved, or run a migration script that simply loads and saves every session.
 
+By default, only the access token is encrypted, but you can pass in any fields (including custom ones not from the `Session` class) you want to encrypt.
+Note that some fields are not allowed for encryption: `id`, `expires`, and any of the boolean fields.
+
+You can use the static `Session.DEFAULT_ENCRYPTED_PROPERTIES` to encrypt the default properties, in addition to your own.
+That enables apps to continue using the default behaviour while customizing it.
+
+```ts
+// Get encrypted data array
+const sessionProperties = await session.toEncryptedPropertyArray({
+  cryptoKey,
+  // Use the default encrypted columns
+  propertiesToEncrypt: [...Session.DEFAULT_ENCRYPTED_PROPERTIES, 'customField'],
+});
+
+// Store and load data
+// ...
+
+// Create session from encrypted data
+const newSession = Session.fromEncryptedPropertyArray(sessionProperties, {
+  cryptoKey,
+});
+```
+
 [Back to guide index](../../README.md#guides)

packages/apps/shopify-api/lib/session/__tests__/session.test.ts

@@ -240,8 +240,8 @@ const testSessions = [
       ['state', 'offline-session-state'],
       ['isOnline', false],
       ['scope', 'offline-session-scope'],
-      ['expires', expiresNumber],
       ['accessToken', 'offline-session-token'],
+      ['expires', expiresNumber],
     ],
     returnUserData: false,
   },
@@ -341,8 +341,8 @@ const testSessions = [
       ['state', 'online-session-state'],
       ['isOnline', true],
       ['scope', 'online-session-scope'],
-      ['expires', expiresNumber],
       ['accessToken', 'online-session-token'],
+      ['expires', expiresNumber],
       ['onlineAccessInfo', 1],
     ],
     returnUserData: false,
@@ -393,8 +393,8 @@ const testSessions = [
       ['state', 'offline-session-state'],
       ['isOnline', false],
       ['scope', 'offline-session-scope'],
-      ['expires', expiresNumber],
       ['accessToken', 'offline-session-token'],
+      ['expires', expiresNumber],
     ],
     returnUserData: true,
   },
@@ -428,8 +428,8 @@ const testSessions = [
       ['state', 'online-session-state'],
       ['isOnline', true],
       ['scope', 'online-session-scope'],
-      ['expires', expiresNumber],
       ['accessToken', 'online-session-token'],
+      ['expires', expiresNumber],
       ['userId', 1],
       ['firstName', 'online-session-first-name'],
       ['lastName', 'online-session-last-name'],
@@ -464,8 +464,8 @@ const testSessions = [
       ['state', 'online-session-state'],
       ['isOnline', true],
       ['scope', 'online-session-scope'],
-      ['expires', expiresNumber],
       ['accessToken', 'online-session-token'],
+      ['expires', expiresNumber],
       ['userId', 1],
     ],
     returnUserData: true,
@@ -639,12 +639,12 @@ describe('toPropertyArray and fromPropertyArray', () => {
 });
 
 describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
-  let key: CryptoKey;
+  let cryptoKey: CryptoKey;
 
   beforeEach(async () => {
     const cryptoLib = getCryptoLib();
 
-    key = await cryptoLib.subtle.generateKey(
+    cryptoKey = await cryptoLib.subtle.generateKey(
       {name: 'AES-GCM', length: 256},
       true,
       ['encrypt', 'decrypt'],
@@ -669,10 +669,10 @@ describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
       const testProps = [...test.propertyArray];
 
       // WHEN
-      const actualProps = await session.toEncryptedPropertyArray(
-        key,
-        test.returnUserData,
-      );
+      const actualProps = await session.toEncryptedPropertyArray({
+        cryptoKey,
+        returnUserData: test.returnUserData,
+      });
 
       // THEN
 
@@ -698,9 +698,11 @@ describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
 
       // WHEN
       const actualSession = await Session.fromEncryptedPropertyArray(
-        await session.toEncryptedPropertyArray(key, test.returnUserData),
-        key,
-        test.returnUserData,
+        await session.toEncryptedPropertyArray({
+          cryptoKey,
+          returnUserData: test.returnUserData,
+        }),
+        {cryptoKey, returnUserData: test.returnUserData},
       );
 
       // THEN
@@ -743,9 +745,11 @@ describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
 
       // WHEN
       const actualSession = await Session.fromEncryptedPropertyArray(
-        await session.toEncryptedPropertyArray(key, test.returnUserData),
-        key,
-        test.returnUserData,
+        await session.toEncryptedPropertyArray({
+          cryptoKey,
+          returnUserData: test.returnUserData,
+        }),
+        {cryptoKey, returnUserData: test.returnUserData},
       );
 
       // THEN
@@ -769,7 +773,10 @@ describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
       expires: expiresDate,
     });
 
-    const props = await session.toEncryptedPropertyArray(key, false);
+    const props = await session.toEncryptedPropertyArray({
+      cryptoKey,
+      returnUserData: false,
+    });
 
     // WHEN
     const tamperedProps = props.map((derp) => {
@@ -781,7 +788,128 @@ describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
 
     // THEN
     await expect(async () =>
-      Session.fromEncryptedPropertyArray(tamperedProps, key, false),
+      Session.fromEncryptedPropertyArray(tamperedProps, {
+        cryptoKey,
+        returnUserData: false,
+      }),
     ).rejects.toThrow('The provided data is too small.');
   });
+
+  describe('encrypting multiple fields', () => {
+    let session: Session;
+
+    beforeEach(() => {
+      session = new Session({
+        id: 'offline_session_id',
+        shop: 'example.myshopify.io',
+        state: 'offline-session-state',
+        isOnline: true,
+        scope: 'test_scope',
+        accessToken: 'offline-session-token',
+        expires: expiresDate,
+        onlineAccessInfo: {
+          expires_in: 1,
+          associated_user_scope: 'user_scope',
+          associated_user: {
+            id: 1,
+            first_name: 'first-name',
+            last_name: 'last-name',
+            email: 'email',
+            locale: 'locale',
+            email_verified: true,
+            account_owner: true,
+            collaborator: false,
+          },
+        },
+      });
+    });
+
+    it('can encrypt and decrypt all fields', async () => {
+      // GIVEN
+      const encryptFields = [
+        'shop',
+        'state',
+        'scope',
+        'accessToken',
+        'userId',
+        'firstName',
+        'lastName',
+        'email',
+        'locale',
+      ];
+
+      // WHEN
+      const encryptedProps = await session.toEncryptedPropertyArray({
+        cryptoKey,
+        propertiesToEncrypt: encryptFields,
+        returnUserData: true,
+      });
+      const newSession = await Session.fromEncryptedPropertyArray(
+        encryptedProps,
+        {cryptoKey, returnUserData: true},
+      );
+
+      // THEN
+      expect(encryptedProps).toMatchObject([
+        ['id', 'offline_session_id'],
+        ['shop', expect.stringMatching(/^encrypted#/)],
+        ['state', expect.stringMatching(/^encrypted#/)],
+        ['isOnline', true],
+        ['scope', expect.stringMatching(/^encrypted#/)],
+        ['accessToken', expect.stringMatching(/^encrypted#/)],
+        ['expires', expect.any(Number)],
+        ['userId', expect.stringMatching(/^encrypted#/)],
+        ['firstName', expect.stringMatching(/^encrypted#/)],
+        ['lastName', expect.stringMatching(/^encrypted#/)],
+        ['email', expect.stringMatching(/^encrypted#/)],
+        ['locale', expect.stringMatching(/^encrypted#/)],
+        ['emailVerified', true],
+        ['accountOwner', true],
+        ['collaborator', false],
+      ]);
+      expect(newSession.equals(session)).toBeTruthy();
+    });
+
+    it('can encrypt and decrypt custom fields', async () => {
+      // GIVEN
+      const sessionWithCustomFields = new Session({
+        ...session.toObject(),
+        customField: 'custom',
+      });
+
+      // WHEN
+      const encryptedProps =
+        await sessionWithCustomFields.toEncryptedPropertyArray({
+          cryptoKey,
+          propertiesToEncrypt: ['customField'],
+          returnUserData: true,
+        });
+      const newSession = await Session.fromEncryptedPropertyArray(
+        encryptedProps,
+        {cryptoKey, returnUserData: true},
+      );
+
+      // THEN
+      const index = encryptedProps.findIndex(([key]) => key === 'customField');
+      expect(index).toBeGreaterThan(-1);
+      expect(encryptedProps[index][1]).toMatch(/^encrypted#/);
+      expect((newSession as any).customField).toEqual(
+        (sessionWithCustomFields as any).customField,
+      );
+    });
+
+    it.each(['id', 'expires', 'emailVerified', 'accountOwner', 'collaborator'])(
+      "can't encrypt '%s' field",
+      async (field) => {
+        // WHEN
+        await expect(
+          session.toEncryptedPropertyArray({
+            cryptoKey,
+            propertiesToEncrypt: [field],
+            returnUserData: true,
+          }),
+        ).rejects.toThrow(`Can't encrypt fields: [${field}]`);
+      },
+    );
+  });
 });