Pass required params to Session constructor. Allow undefined in AuthScopes

CHANGELOG.md

@@ -27,7 +27,11 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 - Added Storefront API client under `Shopify.Clients.Storefront`
 - Add `isActive()` method to `Session` class to check if session is active, replace `Session` with `SessionInterface` when used as a type [#153](https://github.com/Shopify/shopify-node-api/pull/153)
+- Sessions no longer default to `false` for `isOnline` [#169](https://github.com/Shopify/shopify-node-api/pull/169)
 
+### Fixed
+- Required `Session` arguments must be passed to the constructor [#169](https://github.com/Shopify/shopify-node-api/pull/169)
+- Allow `undefined` in `AuthScopes` [#169](https://github.com/Shopify/shopify-node-api/pull/169)
 ## [1.2.1] - 2021-03-26
 
 ### Added

src/auth/oauth/oauth.ts

@@ -56,10 +56,10 @@ const ShopifyOAuth = {
 
     const session = new Session(
       isOnline ? uuidv4() : this.getOfflineSessionId(shop),
+      shop,
+      state,
+      isOnline,
     );
-    session.shop = shop;
-    session.state = state;
-    session.isOnline = isOnline;
 
     const sessionStored = await Context.SESSION_STORAGE.storeSession(session);
 
@@ -173,7 +173,9 @@ const ShopifyOAuth = {
         );
         const jwtSession = Session.cloneSession(currentSession, jwtSessionId);
 
-        const sessionDeleted = await Context.SESSION_STORAGE.deleteSession(currentSession.id);
+        const sessionDeleted = await Context.SESSION_STORAGE.deleteSession(
+          currentSession.id,
+        );
         if (!sessionDeleted) {
           throw new ShopifyErrors.SessionStorageError(
             'OAuth Session could not be deleted. Please check your session storage functionality.',
@@ -195,7 +197,9 @@ const ShopifyOAuth = {
       secure: true,
     });
 
-    const sessionStored = await Context.SESSION_STORAGE.storeSession(currentSession);
+    const sessionStored = await Context.SESSION_STORAGE.storeSession(
+      currentSession,
+    );
     if (!sessionStored) {
       throw new ShopifyErrors.SessionStorageError(
         'OAuth Session could not be saved. Please check your session storage functionality.',

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

@@ -58,7 +58,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;
@@ -294,7 +294,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 : '',
@@ -331,6 +331,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,13 +4,13 @@ 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;
     }
 
@@ -29,7 +29,7 @@ class AuthScopes {
     this.expandedScopes = new Set([...scopeSet, ...impliedSet]);
   }
 
-  public has(scope: string | string[] | AuthScopes) {
+  public has(scope: string | string[] | AuthScopes | undefined) {
     let other: AuthScopes;
 
     if (scope instanceof AuthScopes) {
@@ -43,7 +43,7 @@ class AuthScopes {
     );
   }
 
-  public equals(otherScopes: string | string[] | AuthScopes) {
+  public equals(otherScopes: string | string[] | AuthScopes | undefined) {
     let other: AuthScopes;
 
     if (otherScopes instanceof AuthScopes) {

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,5 +1,5 @@
-import {OnlineAccessInfo} from '../oauth/types';
 import {Context} from '../../context';
+import {OnlineAccessInfo} from '../oauth/types';
 
 import {SessionInterface} from './types';
 
@@ -8,28 +8,22 @@ import {SessionInterface} from './types';
  */
 class Session implements SessionInterface {
   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) {}
 
   public isActive(): boolean {
     const scopesUnchanged = Context.SCOPES.equals(this.scope);

src/auth/session/storage/custom.ts

@@ -41,16 +41,21 @@ export class CustomSessionStorage implements SessionStorage {
           result.expires = new Date(result.expires);
         }
 
-        return result;
+        return result as SessionInterface;
       } 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 as SessionInterface)};
 
         if (session.expires && typeof session.expires === 'string') {
           session.expires = new Date(session.expires);
         }
 
-        return session;
+        return session as SessionInterface;
       } else {
         throw new ShopifyErrors.SessionStorageError(
           `Expected return to be instanceof Session, but received instanceof ${

src/auth/session/storage/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/storage/test/memory.test.ts

@@ -6,7 +6,7 @@ import {MemorySessionStorage} from '../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/auth/session/test/session.test.ts

@@ -3,7 +3,7 @@ import {Session} from '..';
 
 describe('session', () => {
   it('can clone a session', () => {
-    const session = new Session('original');
+    const session = new Session('original', 'original-shop', 'original-state', true);
     const sessionClone = Session.cloneSession(session, 'new');
 
     expect(session.id).not.toEqual(sessionClone.id);
@@ -21,16 +21,17 @@ describe('session', () => {
 
 describe('isActive', () => {
   it('returns true if session is active', () => {
-    const session = new Session('active');
+    const session = new Session('active', 'active-shop', 'test_state', true);
     session.scope = 'test_scope';
     session.accessToken = 'indeed';
     session.expires = new Date(Date.now() + 86400);
+
     expect(session.isActive()).toBeTruthy();
   });
 
   it('returns false if session is not active', () => {
-    const session = new Session('not_active');
-    session.scope = 'not_same';
+    const session = new Session('not_active', 'inactive-shop', 'not_same', true);
+    session.scope = 'test_scope';
     session.expires = new Date(Date.now() - 1);
     expect(session.isActive()).toBeFalsy();
   });

src/auth/session/types.ts

@@ -4,9 +4,9 @@ export interface SessionInterface {
   readonly id: string;
   shop: string;
   state: string;
-  scope: string;
+  isOnline: boolean;
+  scope?: string;
   expires?: Date;
-  isOnline?: boolean;
   accessToken?: string;
   onlineAccessInfo?: OnlineAccessInfo;
   isActive(): boolean;

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

@@ -40,7 +40,12 @@ 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);
@@ -65,7 +70,12 @@ 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);
@@ -83,7 +93,12 @@ 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);
@@ -110,6 +125,9 @@ describe('deleteCurrenSession', () => {
 
     const session = new Session(
       ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io'),
+      'test-shop.myshopify.io',
+      'state',
+      false,
     );
     await expect(
       Context.SESSION_STORAGE.storeSession(session),

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, {
@@ -128,7 +127,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);