Pass required args to Session constructor. Fix undefined scopes.

CHANGELOG.md

@@ -11,6 +11,9 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 ### Added
 - Added `April21` to `ApiVersion` [#149](https://github.com/Shopify/shopify-node-api/pull/149)
 
+### Fixed
+- Required `Session` arguments must be passed to the constructor [#169](https://github.com/Shopify/shopify-node-api/pull/169)
+
 ## [1.2.0] - 2021-03-16
 
 ### Added

src/auth/oauth/oauth.ts

@@ -48,10 +48,7 @@ const ShopifyOAuth = {
 
     const state = nonce();
 
-    const session = new Session(isOnline ? uuidv4() : this.getOfflineSessionId(shop));
-    session.shop = shop;
-    session.state = state;
-    session.isOnline = isOnline;
+    const session = new Session(isOnline ? uuidv4() : this.getOfflineSessionId(shop), shop, state, isOnline);
 
     const sessionStored = await Context.SESSION_STORAGE.storeSession(session);
 

src/auth/oauth/test/oauth.test.ts

@@ -56,7 +56,7 @@ describe('beginAuth', () => {
   test('throws SessionStorageErrors when storeSession returns false', async () => {
     const storage = new CustomSessionStorage(
       () => Promise.resolve(false),
-      () => Promise.resolve(new Session(shop)),
+      () => Promise.resolve(new Session('id', shop, 'state', true)),
       () => Promise.resolve(true),
     );
     Context.SESSION_STORAGE = storage;
@@ -268,7 +268,7 @@ describe('validateAuthCallback', () => {
 
   test('requests access token for valid callbacks with online access and updates session with expiration and onlineAccessInfo', async () => {
     await ShopifyOAuth.beginAuth(req, res, shop, '/some-callback', true);
-    const session = await Context.SESSION_STORAGE.loadSession(cookies.id);
+    let session = await Context.SESSION_STORAGE.loadSession(cookies.id);
     const testCallbackQuery: AuthQuery = {
       shop,
       state: session ? session.state : '',
@@ -305,6 +305,8 @@ describe('validateAuthCallback', () => {
 
     fetchMock.mockResponse(JSON.stringify(successResponse));
     await ShopifyOAuth.validateAuthCallback(req, res, testCallbackQuery);
+    session = await Context.SESSION_STORAGE.loadSession(cookies.id);
+
     expect(session?.accessToken).toBe(successResponse.access_token);
     expect(session?.expires).toBeInstanceOf(Date);
     expect(session?.onlineAccessInfo).toEqual(expectedOnlineAccessInfo);

src/auth/scopes/index.ts

@@ -4,11 +4,11 @@ class AuthScopes {
   private compressedScopes: Set<string>;
   private expandedScopes: Set<string>;
 
-  constructor(scopes: string | string[]) {
+  constructor(scopes: string | string[] | undefined) {
     let scopesArray: string[] = [];
     if (typeof scopes === 'string') {
       scopesArray = scopes.split(new RegExp(`${AuthScopes.SCOPE_DELIMITER}\\s*`));
-    } else {
+    } else if (scopes) {
       scopesArray = scopes;
     }
 

src/auth/scopes/test/scopes.test.ts

@@ -3,6 +3,14 @@ import '../../../test/test_helper';
 import {AuthScopes} from '../index';
 
 describe('AuthScopes', () => {
+  it('can be undefined', () => {
+    const scope = undefined;
+    const scopes = new AuthScopes(scope);
+
+    expect(scopes.toString()).toEqual('');
+    expect(scopes.has('read_something')).toBeFalsy();
+  });
+
   it('can parse and trim string scopes', () => {
     const scopeString = ' read_products, read_orders,,write_customers ';
     const scopes = new AuthScopes(scopeString);

src/auth/session/session.ts

@@ -1,32 +1,32 @@
+// import {Context} from '../../context';
 import {OnlineAccessInfo} from '../oauth/types';
 
 /**
  * Stores App information from logged in merchants so they can make authenticated requests to the Admin API.
  */
 class Session {
   public static cloneSession(session: Session, newId: string): Session {
-    const newSession = new Session(newId);
+    const newSession = new Session(newId, session.shop, session.state, session.isOnline);
 
-    newSession.shop = session.shop;
-    newSession.state = session.state;
     newSession.scope = session.scope;
     newSession.expires = session.expires;
-    newSession.isOnline = session.isOnline;
     newSession.accessToken = session.accessToken;
     newSession.onlineAccessInfo = session.onlineAccessInfo;
 
     return newSession;
   }
 
-  public shop: string;
-  public state: string;
-  public scope: string;
+  public scope?: string;
   public expires?: Date;
-  public isOnline?: boolean;
   public accessToken?: string;
   public onlineAccessInfo?: OnlineAccessInfo;
 
-  constructor(readonly id: string) {}
+  constructor(
+    readonly id: string,
+    public shop: string,
+    public state: string,
+    public isOnline: boolean = false,
+  ) {}
 }
 
 export {Session};

src/auth/session/storage/custom.ts

@@ -40,7 +40,12 @@ export class CustomSessionStorage implements SessionStorage {
 
         return result;
       } else if (result instanceof Object && 'id' in result) {
-        let session = new Session(result.id as string);
+        let session = new Session(
+          result.id as string,
+          result.shop as string,
+          result.state as string,
+          result.isOnline as boolean,
+        );
         session = {...session, ...result};
 
         if (session.expires && typeof session.expires === 'string') {

src/auth/session/test/custom.test.ts

@@ -7,7 +7,7 @@ import {SessionStorageError} from '../../../error';
 describe('custom session storage', () => {
   test('can perform actions', async () => {
     const sessionId = 'test_session';
-    let session: Session | undefined = new Session(sessionId);
+    let session: Session | undefined = new Session(sessionId, 'shop-url', 'state', true);
 
     let storeCalled = false;
     let loadCalled = false;
@@ -55,7 +55,7 @@ describe('custom session storage', () => {
 
   test('failures and exceptions are raised', () => {
     const sessionId = 'test_session';
-    const session = new Session(sessionId);
+    const session = new Session(sessionId, 'shop-url', 'state', true);
 
     let storage = new CustomSessionStorage(
       () => Promise.resolve(false),
@@ -99,7 +99,7 @@ describe('custom session storage', () => {
     const expiration = new Date();
     expiration.setDate(expiration.getDate() + 10);
 
-    let session: Session | undefined = new Session(sessionId);
+    let session: Session | undefined = new Session(sessionId, 'shop', 'state', true);
     session.expires = expiration;
 
     const storage = new CustomSessionStorage(
@@ -128,9 +128,7 @@ describe('custom session storage', () => {
     expiration.setDate(expiration.getDate() + 10);
 
     /* eslint-disable @typescript-eslint/naming-convention */
-    let session: Session | undefined = new Session(sessionId);
-    session.shop = 'test.myshopify.io';
-    session.state = '1234';
+    let session: Session | undefined = new Session(sessionId, 'test.myshopify.io', '1234', true);
     session.scope = 'read_products';
     session.expires = expiration;
     session.isOnline = true;
@@ -176,7 +174,7 @@ describe('custom session storage', () => {
   it('allows empty fields in serialized object', async () => {
     const sessionId = 'test_session';
 
-    let session: Session | undefined = new Session(sessionId);
+    let session: Session | undefined = new Session(sessionId, 'shop', 'state', true);
 
     let serializedSession = '';
     const storage = new CustomSessionStorage(

src/auth/session/test/memory.test.ts

@@ -6,7 +6,7 @@ import {MemorySessionStorage} from '../storage/memory';
 test('can store and delete sessions in memory', async () => {
   const sessionId = 'test_session';
   const storage = new MemorySessionStorage();
-  const session = new Session(sessionId);
+  const session = new Session(sessionId, 'shop', 'state', false);
 
   await expect(storage.storeSession(session)).resolves.toBe(true);
   await expect(storage.loadSession(sessionId)).resolves.toEqual(session);

src/utils/test/delete-current-session.test.ts

@@ -40,7 +40,7 @@ describe('deleteCurrenSession', () => {
 
     const cookieId = '1234-this-is-a-cookie-session-id';
 
-    const session = new Session(cookieId);
+    const session = new Session(cookieId, 'test-shop.myshopify.io', 'state', true);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     Cookies.prototype.get.mockImplementation(() => cookieId);
@@ -61,7 +61,7 @@ describe('deleteCurrenSession', () => {
     } as http.IncomingMessage;
     const res = {} as http.ServerResponse;
 
-    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`);
+    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`, 'test-shop.myshopify.io', 'state', true);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     await expect(deleteCurrentSession(req, res)).resolves.toBe(true);
@@ -77,7 +77,7 @@ describe('deleteCurrenSession', () => {
 
     const cookieId = ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io');
 
-    const session = new Session(cookieId);
+    const session = new Session(cookieId, 'test-shop.myshopify.io', 'state', false);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     Cookies.prototype.get.mockImplementation(() => cookieId);
@@ -98,7 +98,12 @@ describe('deleteCurrenSession', () => {
     } as http.IncomingMessage;
     const res = {} as http.ServerResponse;
 
-    const session = new Session(ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io'));
+    const session = new Session(
+      ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io'),
+      'test-shop.myshopify.io',
+      'state',
+      false,
+    );
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     await expect(deleteCurrentSession(req, res, false)).resolves.toBe(true);

src/utils/test/delete-offline-session.test.ts

@@ -11,7 +11,7 @@ describe('deleteOfflineSession', () => {
   const offlineId = OAuth.getOfflineSessionId(shop);
 
   beforeEach(() => {
-    const offlineSession = new Session(offlineId);
+    const offlineSession = new Session(offlineId, shop, 'state', false);
     Context.SESSION_STORAGE.storeSession(offlineSession);
   });
 

src/utils/test/graphql_proxy.test.ts

@@ -55,8 +55,7 @@ describe('GraphQL proxy with session', () => {
       sid: 'abc123',
     };
 
-    const session = new Session(`shop.myshopify.com_${jwtPayload.sub}`);
-    session.shop = shop;
+    const session = new Session(`shop.myshopify.com_${jwtPayload.sub}`, shop, 'state', true);
     session.accessToken = accessToken;
     await Context.SESSION_STORAGE.storeSession(session);
     token = jwt.sign(jwtPayload, Context.API_SECRET_KEY, {algorithm: 'HS256'});
@@ -121,7 +120,7 @@ describe('GraphQL proxy', () => {
       },
     } as http.IncomingMessage;
     const res = {} as http.ServerResponse;
-    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`);
+    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`, shop, 'state', true);
     Context.SESSION_STORAGE.storeSession(session);
 
     await expect(graphqlProxy(req, res)).rejects.toThrow(InvalidSession);

src/utils/test/load-current-session.test.ts

@@ -40,7 +40,7 @@ describe('loadCurrentSession', () => {
 
     const cookieId = '1234-this-is-a-cookie-session-id';
 
-    const session = new Session(cookieId);
+    const session = new Session(cookieId, 'test-shop.myshopify.io', 'state', true);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     Cookies.prototype.get.mockImplementation(() => cookieId);
@@ -72,7 +72,7 @@ describe('loadCurrentSession', () => {
     } as http.IncomingMessage;
     const res = {} as http.ServerResponse;
 
-    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`);
+    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`, 'test-shop.myshopify.io', 'state', true);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     await expect(loadCurrentSession(req, res)).resolves.toEqual(session);
@@ -130,7 +130,7 @@ describe('loadCurrentSession', () => {
 
     const cookieId = '1234-this-is-a-cookie-session-id';
 
-    const session = new Session(cookieId);
+    const session = new Session(cookieId, 'test-shop.myshopify.io', 'state', true);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     Cookies.prototype.get.mockImplementation(() => cookieId);
@@ -147,7 +147,7 @@ describe('loadCurrentSession', () => {
 
     const cookieId = ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io');
 
-    const session = new Session(cookieId);
+    const session = new Session(cookieId, 'test-shop.myshopify.io', 'state', false);
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     Cookies.prototype.get.mockImplementation(() => cookieId);
@@ -167,7 +167,12 @@ describe('loadCurrentSession', () => {
     } as http.IncomingMessage;
     const res = {} as http.ServerResponse;
 
-    const session = new Session(ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io'));
+    const session = new Session(
+      ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io'),
+      'test-shop.myshopify.io',
+      'state',
+      false,
+    );
     await expect(Context.SESSION_STORAGE.storeSession(session)).resolves.toEqual(true);
 
     await expect(loadCurrentSession(req, res, false)).resolves.toEqual(session);

src/utils/test/load-offline-session.test.ts

@@ -14,15 +14,15 @@ describe('loadOfflineSession', () => {
 
   it('loads offline sessions by shop', async () => {
     const offlineId = OAuth.getOfflineSessionId(shop);
-    const offlineSession = new Session(offlineId);
+    const offlineSession = new Session(offlineId, shop, 'state', false);
     Context.SESSION_STORAGE.storeSession(offlineSession);
 
     expect(await loadOfflineSession(shop)).toBe(offlineSession);
   });
 
   it('returns undefined for expired sessions by default', async () => {
     const offlineId = OAuth.getOfflineSessionId(shop);
-    const offlineSession = new Session(offlineId);
+    const offlineSession = new Session(offlineId, shop, 'state', false);
     offlineSession.expires = new Date('2020-01-01');
     Context.SESSION_STORAGE.storeSession(offlineSession);
 
@@ -31,7 +31,7 @@ describe('loadOfflineSession', () => {
 
   it('returns expired sessions when includeExpired is true', async () => {
     const offlineId = OAuth.getOfflineSessionId(shop);
-    const offlineSession = new Session(offlineId);
+    const offlineSession = new Session(offlineId, shop, 'state', false);
     offlineSession.expires = new Date('2020-01-01');
     Context.SESSION_STORAGE.storeSession(offlineSession);
 

src/utils/test/store-session.test.ts

@@ -34,7 +34,7 @@ describe('storeSession', () => {
     } as http.IncomingMessage;
     const res = {} as http.ServerResponse;
 
-    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`);
+    const session = new Session(`test-shop.myshopify.io_${jwtPayload.sub}`, 'test-shop.myshopify.io', 'state', false);
     await storeSession(session);
 
     let loadedSession = await loadCurrentSession(req, res);

src/utils/test/with-session.test.ts

@@ -37,9 +37,7 @@ describe('withSession', () => {
     hitting the clientType error */
 
     const offlineId = OAuth.getOfflineSessionId(shop);
-    const session = new Session(offlineId);
-    session.isOnline = false;
-    session.shop = shop;
+    const session = new Session(offlineId, shop, 'state', false);
     session.accessToken = 'gimme-access';
     await Context.SESSION_STORAGE.storeSession(session);
 
@@ -62,9 +60,7 @@ describe('withSession', () => {
 
   it('throws an error when the session is not yet authenticated', async () => {
     const offlineId = OAuth.getOfflineSessionId(shop);
-    const session = new Session(offlineId);
-    session.isOnline = false;
-    session.shop = shop;
+    const session = new Session(offlineId, shop, 'state', false);
     await Context.SESSION_STORAGE.storeSession(session);
 
     await expect(withSession({clientType: 'rest', isOnline: false, shop})).rejects.toThrow(
@@ -74,9 +70,7 @@ describe('withSession', () => {
 
   it('returns an object containing the appropriate client and session for offline sessions', async () => {
     const offlineId = OAuth.getOfflineSessionId(shop);
-    const session = new Session(offlineId);
-    session.isOnline = false;
-    session.shop = shop;
+    const session = new Session(offlineId, shop, 'state', false);
     session.accessToken = 'gimme-access';
     await Context.SESSION_STORAGE.storeSession(session);
 
@@ -105,9 +99,7 @@ describe('withSession', () => {
     Context.IS_EMBEDDED_APP = false;
     Context.initialize(Context);
 
-    const session = new Session(`12345`);
-    session.isOnline = true;
-    session.shop = shop;
+    const session = new Session(`12345`, shop, 'state', true);
     session.accessToken = 'gimme-access';
     await Context.SESSION_STORAGE.storeSession(session);
 
@@ -147,9 +139,7 @@ describe('withSession', () => {
 
     const sub = '1';
     const sessionId = ShopifyOAuth.getJwtSessionId(shop, sub);
-    const session = new Session(sessionId);
-    session.isOnline = true;
-    session.shop = shop;
+    const session = new Session(sessionId, shop, 'state', true);
     session.accessToken = 'gimme-access';
     await Context.SESSION_STORAGE.storeSession(session);