Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to encrypt session data #818

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/olive-eggs-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@shopify/shopify-api": minor
---

Added the ability to encrypt Session access tokens using AES-GCM with a 128-bit tag and a 12-byte random IV.
16 changes: 12 additions & 4 deletions packages/apps/shopify-api/docs/guides/session-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,9 @@ const sessionCopy = new Session(callbackResponse.session.toObject());
// sessionCopy is an identical copy of the callbackResponse.session instance
```

Now that the app has a JavaScript object containing the data of a `Session`, it can convert the data into whatever means necessary to store it in the apps preferred storage mechanism. Various implementations of session storage can be found in the [`session-storages` folder](../../../session-storage#readme).
When converting the data for storage, the `Session` class also includes an instance method called `.toPropertyArray` that returns an array of key-value pairs constructed from the result of `toObject`. `toPropertyArray` has an optional parameter `returnUserData`, defaulted to false, when set to true it will return the associated user data as part of the property array object.

The `Session` class also includes an instance method called `.toPropertyArray` that returns an array of key-value pairs, e.g.,

`toPropertyArray` has an optional parameter `returnUserData`, defaulted to false, when set to true it will return the associated user data as part of the property array object.
With that object containing the data of a `Session`, the app can convert the data into whatever it needs to store using its preferred mechanism. Various implementations of session storage can be found in the [`session-storages` folder](../../../session-storage#readme).

```ts
const {session, headers} = shopify.auth.callback({
Expand Down Expand Up @@ -294,4 +292,14 @@ const session = Session.fromPropertyArray(sessionProperties);
>
> The existing [SQL-based implementations](../../../session-storage#readme), i.e., MySQL, PostgreSQL and SQLite, convert it from seconds from storage. The remaining implementations do not change the retrieved `expires` property.

### Encrypting data for storage

If you want to encrypt your access tokens 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Should we be making it flexible to allow folks to encrypt more than just the accessToken?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought of that too, but haven't thought of how the app would convey that information. I think it would need to be a direct argument to to/fromEncryptedArray that the app would have to pass in to the session storage.

That would mean the app could do something like:

// Evergreen way of encrypting every column we allow
new PrismaSessionStorage(prisma, {encryptionKey: true, encryptColumns: 'all'});

// Explicitly select which columns
new PrismaSessionStorage(prisma, {encryptionKey: true, encryptColumns: ['accessToken', 'userId']});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think it would have to be something like that. I think it would be a nice to have, but does go beyond the scope of this required work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fairly easy to implement though, I'm going to timebox it and see if I can get it off the ground.


`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.

[Back to guide index](../../README.md#guides)
159 changes: 154 additions & 5 deletions packages/apps/shopify-api/lib/session/__tests__/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {Session} from '../session';
import {testConfig} from '../../__tests__/test-config';
import {shopifyApi} from '../..';
import {AuthScopes} from '../../auth';
import {getCryptoLib} from '../../../runtime';

describe('session', () => {
it('can create a session from another session', () => {
Expand Down Expand Up @@ -239,8 +240,8 @@ const testSessions = [
['state', 'offline-session-state'],
['isOnline', false],
['scope', 'offline-session-scope'],
['accessToken', 'offline-session-token'],
['expires', expiresNumber],
['accessToken', 'offline-session-token'],
],
returnUserData: false,
},
Expand Down Expand Up @@ -340,8 +341,8 @@ const testSessions = [
['state', 'online-session-state'],
['isOnline', true],
['scope', 'online-session-scope'],
['accessToken', 'online-session-token'],
['expires', expiresNumber],
['accessToken', 'online-session-token'],
['onlineAccessInfo', 1],
],
returnUserData: false,
Expand Down Expand Up @@ -392,8 +393,8 @@ const testSessions = [
['state', 'offline-session-state'],
['isOnline', false],
['scope', 'offline-session-scope'],
['accessToken', 'offline-session-token'],
['expires', expiresNumber],
['accessToken', 'offline-session-token'],
],
returnUserData: true,
},
Expand Down Expand Up @@ -427,8 +428,8 @@ const testSessions = [
['state', 'online-session-state'],
['isOnline', true],
['scope', 'online-session-scope'],
['accessToken', 'online-session-token'],
['expires', expiresNumber],
['accessToken', 'online-session-token'],
['userId', 1],
['firstName', 'online-session-first-name'],
['lastName', 'online-session-last-name'],
Expand Down Expand Up @@ -463,8 +464,8 @@ const testSessions = [
['state', 'online-session-state'],
['isOnline', true],
['scope', 'online-session-scope'],
['accessToken', 'online-session-token'],
['expires', expiresNumber],
['accessToken', 'online-session-token'],
['userId', 1],
],
returnUserData: true,
Expand Down Expand Up @@ -636,3 +637,151 @@ describe('toPropertyArray and fromPropertyArray', () => {
});
});
});

describe('toEncryptedPropertyArray and fromEncryptedPropertyArray', () => {
let key: CryptoKey;

beforeEach(async () => {
const cryptoLib = getCryptoLib();

key = await cryptoLib.subtle.generateKey(
{name: 'AES-GCM', length: 256},
true,
['encrypt', 'decrypt'],
);
});

testSessions.forEach((test) => {
const onlineOrOffline = test.session.isOnline ? 'online' : 'offline';
const userData = test.returnUserData ? 'with' : 'without';

it(`returns a property array of an ${onlineOrOffline} session ${userData} user data`, async () => {
// GIVEN
const getPropIndex = (object: any, prop: string, check = true) => {
const index = object.findIndex((property: any) => property[0] === prop);

if (check) expect(index).toBeGreaterThan(-1);

return index;
};

const session = new Session(test.session);
const testProps = [...test.propertyArray];

// WHEN
const actualProps = await session.toEncryptedPropertyArray(
key,
test.returnUserData,
);

// THEN

// The token is encrypted, so the values will be different
const tokenIndex = getPropIndex(testProps, 'accessToken', false);
const actualTokenIndex = getPropIndex(actualProps, 'accessToken', false);

if (actualTokenIndex > -1 && tokenIndex > -1) {
expect(
actualProps[actualTokenIndex][1].toString().startsWith('encrypted#'),
).toBeTruthy();

actualProps.splice(actualTokenIndex, 1);
testProps.splice(tokenIndex, 1);
}

expect(actualProps).toStrictEqual(testProps);
});

it(`recreates a Session from a property array of an ${onlineOrOffline} session ${userData} user data`, async () => {
// GIVEN
const session = new Session(test.session);

// WHEN
const actualSession = await Session.fromEncryptedPropertyArray(
await session.toEncryptedPropertyArray(key, test.returnUserData),
key,
test.returnUserData,
);

// THEN
expect(actualSession.id).toStrictEqual(session.id);
expect(actualSession.shop).toStrictEqual(session.shop);
expect(actualSession.state).toStrictEqual(session.state);
expect(actualSession.isOnline).toStrictEqual(session.isOnline);
expect(actualSession.scope).toStrictEqual(session.scope);
expect(actualSession.accessToken).toStrictEqual(session.accessToken);
expect(actualSession.expires).toStrictEqual(session.expires);

const user = session.onlineAccessInfo?.associated_user;
const actualUser = actualSession.onlineAccessInfo?.associated_user;
expect(actualUser?.id).toStrictEqual(user?.id);

if (test.returnUserData) {
if (user && actualUser) {
expect(actualUser).toMatchObject(user);
} else {
expect(actualUser).toBeUndefined();
expect(user).toBeUndefined();
}
} else {
expect(actualUser?.first_name).toBeUndefined();
expect(actualUser?.last_name).toBeUndefined();
expect(actualUser?.email).toBeUndefined();
expect(actualUser?.locale).toBeUndefined();
expect(actualUser?.email_verified).toBeUndefined();
expect(actualUser?.account_owner).toBeUndefined();
expect(actualUser?.collaborator).toBeUndefined();
}
});

const describe = test.session.isOnline ? 'Does' : 'Does not';
const isOnline = test.session.isOnline ? 'online' : 'offline';

it(`${describe} have online access info when the token is ${isOnline}`, async () => {
// GIVEN
const session = new Session(test.session);

// WHEN
const actualSession = await Session.fromEncryptedPropertyArray(
await session.toEncryptedPropertyArray(key, test.returnUserData),
key,
test.returnUserData,
);

// THEN
if (test.session.isOnline) {
expect(actualSession.onlineAccessInfo).toBeDefined();
} else {
expect(actualSession.onlineAccessInfo).toBeUndefined();
}
});
});

it('fails to decrypt an invalid token', async () => {
// GIVEN
const session = new Session({
id: 'offline_session_id',
shop: 'offline-session-shop',
state: 'offline-session-state',
isOnline: false,
scope: 'offline-session-scope',
accessToken: 'offline-session-token',
expires: expiresDate,
});

const props = await session.toEncryptedPropertyArray(key, false);

// WHEN
const tamperedProps = props.map((derp) => {
return [
derp[0],
derp[0] === 'accessToken' ? 'encrypted#invalid token' : derp[1],
] as [string, string | number | boolean];
});

// THEN
await expect(async () =>
Session.fromEncryptedPropertyArray(tamperedProps, key, false),
).rejects.toThrow('The provided data is too small.');
});
});
89 changes: 83 additions & 6 deletions packages/apps/shopify-api/lib/session/session.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
/* eslint-disable no-fallthrough */

import {InvalidSession} from '../error';
import {OnlineAccessInfo} from '../auth/oauth/types';
import {AuthScopes} from '../auth/scopes';
import {
decryptString,
encryptString,
generateIV,
asBase64,
fromBase64,
} from '../../runtime/crypto';

import {SessionParams} from './types';

type SessionParamsArray = [string, string | number | boolean][];

const propertiesToSave = [
'id',
'shop',
Expand All @@ -20,8 +30,10 @@ const propertiesToSave = [
* Stores App information from logged in merchants so they can make authenticated requests to the Admin API.
*/
export class Session {
private static CIPHER_PREFIX = 'encrypted#';

public static fromPropertyArray(
entries: [string, string | number | boolean][],
entries: SessionParamsArray,
returnUserData = false,
): Session {
if (!Array.isArray(entries)) {
Expand Down Expand Up @@ -134,6 +146,48 @@ export class Session {
return session;
}

public static async fromEncryptedPropertyArray(
entries: SessionParamsArray,
cryptoKey: CryptoKey,
returnUserData = false,
) {
const decryptedEntries: SessionParamsArray = [];
for (const [key, value] of entries) {
switch (key) {
case 'accessToken':
decryptedEntries.push([
key,
await this.decryptValue(value as string, cryptoKey),
]);
break;
default:
decryptedEntries.push([key, value]);
break;
}
}

return this.fromPropertyArray(decryptedEntries, returnUserData);
}

private static async encryptValue(value: string, key: CryptoKey) {
const iv = generateIV();
const cipher = await encryptString(value, {key, iv});

return `${Session.CIPHER_PREFIX}${asBase64(iv)}${cipher}`;
}

private static async decryptValue(value: string, key: CryptoKey) {
if (!value.startsWith(Session.CIPHER_PREFIX)) {
return value;
}

const keyString = value.slice(Session.CIPHER_PREFIX.length);
paulomarg marked this conversation as resolved.
Show resolved Hide resolved
const iv = new Uint8Array(fromBase64(keyString.slice(0, 16)));
const cipher = keyString.slice(16);

return decryptString(cipher, {key, iv});
}

/**
* The unique identifier for the session.
*/
Expand Down Expand Up @@ -208,7 +262,7 @@ export class Session {
}

/**
* Converts an object with data into a Session.
* Converts a Session into an object with its data, that can be used to construct another Session.
*/
public toObject(): SessionParams {
const object: SessionParams = {
Expand Down Expand Up @@ -259,19 +313,42 @@ export class Session {
/**
* Converts the session into an array of key-value pairs.
*/
public toPropertyArray(
public toPropertyArray(returnUserData = false): SessionParamsArray {
return this.flattenProperties(this.toObject(), returnUserData);
}

/**
* Converts the session into an array of key-value pairs, encrypting sensitive data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could explicitly say it is the access token that is getting encrypted.

*
* The encrypted string will contain both the IV and the encrypted value.
*/
public async toEncryptedPropertyArray(
key: CryptoKey,
returnUserData = false,
): [string, string | number | boolean][] {
): Promise<SessionParamsArray> {
const object = this.toObject();

if (object.accessToken) {
object.accessToken = await Session.encryptValue(object.accessToken, key);
}

return this.flattenProperties(object, returnUserData);
}

private flattenProperties(
params: SessionParams,
returnUserData: boolean,
): SessionParamsArray {
return (
Object.entries(this)
Object.entries(params)
.filter(
([key, value]) =>
propertiesToSave.includes(key) &&
value !== undefined &&
value !== null,
)
// Prepare values for db storage
.flatMap(([key, value]): [string, string | number | boolean][] => {
.flatMap(([key, value]): SessionParamsArray => {
switch (key) {
case 'expires':
return [[key, value ? value.getTime() : undefined]];
Expand Down
1 change: 1 addition & 0 deletions packages/apps/shopify-api/runtime/__tests__/all.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import '../crypto/__tests__/encrypt.test';
import '../crypto/__tests__/hmac.test';
import '../http/__tests__/http.test';
import '../platform/__tests__/platform.test';
Loading
Loading