From cff6350f90422d3a6583f4cd7bb3f658a70079fc Mon Sep 17 00:00:00 2001 From: Emily Harber Date: Thu, 8 Apr 2021 10:05:17 -0400 Subject: [PATCH 1/4] Pass required args to Session constructor. Fix undefined scopes. --- CHANGELOG.md | 3 +++ src/auth/oauth/oauth.ts | 5 +---- src/auth/oauth/test/oauth.test.ts | 6 ++++-- src/auth/scopes/index.ts | 4 ++-- src/auth/scopes/test/scopes.test.ts | 8 ++++++++ src/auth/session/session.ts | 19 +++++++++--------- src/auth/session/storage/custom.ts | 11 +++++++--- src/auth/session/storage/test/custom.test.ts | 12 +++++------ src/auth/session/storage/test/memory.test.ts | 2 +- src/utils/test/delete-current-session.test.ts | 13 ++++++++---- src/utils/test/delete-offline-session.test.ts | 2 +- src/utils/test/graphql_proxy.test.ts | 5 ++--- src/utils/test/load-current-session.test.ts | 15 +++++++++----- src/utils/test/load-offline-session.test.ts | 6 +++--- src/utils/test/store-session.test.ts | 2 +- src/utils/test/with-session.test.ts | 20 +++++-------------- 16 files changed, 72 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de492fb3f..388e02996 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,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 diff --git a/src/auth/oauth/oauth.ts b/src/auth/oauth/oauth.ts index 05a10657e..d4c06330c 100644 --- a/src/auth/oauth/oauth.ts +++ b/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); diff --git a/src/auth/oauth/test/oauth.test.ts b/src/auth/oauth/test/oauth.test.ts index 605e6af8f..08c2ba9ab 100644 --- a/src/auth/oauth/test/oauth.test.ts +++ b/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); diff --git a/src/auth/scopes/index.ts b/src/auth/scopes/index.ts index 036a8d750..8a1d3a7b5 100644 --- a/src/auth/scopes/index.ts +++ b/src/auth/scopes/index.ts @@ -4,11 +4,11 @@ class AuthScopes { private compressedScopes: Set; private expandedScopes: Set; - 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; } diff --git a/src/auth/scopes/test/scopes.test.ts b/src/auth/scopes/test/scopes.test.ts index e44353d8d..cb8b367a0 100644 --- a/src/auth/scopes/test/scopes.test.ts +++ b/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); diff --git a/src/auth/session/session.ts b/src/auth/session/session.ts index c4dc36f29..5253b6604 100644 --- a/src/auth/session/session.ts +++ b/src/auth/session/session.ts @@ -1,3 +1,4 @@ +// import {Context} from '../../context'; import {OnlineAccessInfo} from '../oauth/types'; import {Context} from '../../context'; @@ -8,29 +9,21 @@ 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) {} - public isActive(): boolean { const scopesUnchanged = Context.SCOPES.equals(this.scope); if (scopesUnchanged && this.accessToken && (!this.expires || this.expires >= new Date())) { @@ -38,6 +31,12 @@ class Session implements SessionInterface { } return false; } + constructor( + readonly id: string, + public shop: string, + public state: string, + public isOnline: boolean = false, + ) {} } export {Session}; diff --git a/src/auth/session/storage/custom.ts b/src/auth/session/storage/custom.ts index 44d351ccd..30a20af59 100644 --- a/src/auth/session/storage/custom.ts +++ b/src/auth/session/storage/custom.ts @@ -39,16 +39,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 ${result!.constructor.name}.`, diff --git a/src/auth/session/storage/test/custom.test.ts b/src/auth/session/storage/test/custom.test.ts index b556c3133..249f8d9d1 100644 --- a/src/auth/session/storage/test/custom.test.ts +++ b/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( diff --git a/src/auth/session/storage/test/memory.test.ts b/src/auth/session/storage/test/memory.test.ts index 8ac9e8930..2dc1443e3 100644 --- a/src/auth/session/storage/test/memory.test.ts +++ b/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); diff --git a/src/utils/test/delete-current-session.test.ts b/src/utils/test/delete-current-session.test.ts index 7adcacd8c..347596bdb 100644 --- a/src/utils/test/delete-current-session.test.ts +++ b/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); diff --git a/src/utils/test/delete-offline-session.test.ts b/src/utils/test/delete-offline-session.test.ts index 894bf11a4..49130c073 100644 --- a/src/utils/test/delete-offline-session.test.ts +++ b/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); }); diff --git a/src/utils/test/graphql_proxy.test.ts b/src/utils/test/graphql_proxy.test.ts index 5a15c63a7..d9d2dbbde 100644 --- a/src/utils/test/graphql_proxy.test.ts +++ b/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); diff --git a/src/utils/test/load-current-session.test.ts b/src/utils/test/load-current-session.test.ts index edd69c027..50a4f2e86 100644 --- a/src/utils/test/load-current-session.test.ts +++ b/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); diff --git a/src/utils/test/load-offline-session.test.ts b/src/utils/test/load-offline-session.test.ts index 44aa34e14..08377a40b 100644 --- a/src/utils/test/load-offline-session.test.ts +++ b/src/utils/test/load-offline-session.test.ts @@ -14,7 +14,7 @@ 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); @@ -22,7 +22,7 @@ describe('loadOfflineSession', () => { 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); diff --git a/src/utils/test/store-session.test.ts b/src/utils/test/store-session.test.ts index 1910fbead..f99654b18 100644 --- a/src/utils/test/store-session.test.ts +++ b/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); diff --git a/src/utils/test/with-session.test.ts b/src/utils/test/with-session.test.ts index d7225724f..5088793c1 100644 --- a/src/utils/test/with-session.test.ts +++ b/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); From 8bf068e58f50d16ff45b03411af5a992bc141962 Mon Sep 17 00:00:00 2001 From: Emily Harber Date: Thu, 29 Apr 2021 14:59:24 -0400 Subject: [PATCH 2/4] Rebase main. Add necessary changes to Session and AuthScope. --- src/auth/scopes/index.ts | 5 +++-- src/auth/session/session.ts | 15 ++++++++------- src/auth/session/test/session.test.ts | 11 ++++++----- src/auth/session/types.ts | 2 +- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/auth/scopes/index.ts b/src/auth/scopes/index.ts index 8a1d3a7b5..c12d6e322 100644 --- a/src/auth/scopes/index.ts +++ b/src/auth/scopes/index.ts @@ -1,3 +1,4 @@ + class AuthScopes { public static SCOPE_DELIMITER = ','; @@ -23,7 +24,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) { @@ -35,7 +36,7 @@ class AuthScopes { return other.toArray().filter((x) => !this.expandedScopes.has(x)).length === 0; } - public equals(otherScopes: string | string[] | AuthScopes) { + public equals(otherScopes: string | string[] | AuthScopes | undefined) { let other: AuthScopes; if (otherScopes instanceof AuthScopes) { diff --git a/src/auth/session/session.ts b/src/auth/session/session.ts index 5253b6604..38462b1f3 100644 --- a/src/auth/session/session.ts +++ b/src/auth/session/session.ts @@ -19,18 +19,19 @@ class Session implements SessionInterface { return newSession; } + public isActive(): boolean { + const scopesUnchanged = Context.SCOPES.equals(this.scope); + if (scopesUnchanged && this.accessToken && (!this.expires || this.expires >= new Date())) { + return true; + } + return false; + } + public scope?: string; public expires?: Date; public accessToken?: string; public onlineAccessInfo?: OnlineAccessInfo; - public isActive(): boolean { - const scopesUnchanged = Context.SCOPES.equals(this.scope); - if (scopesUnchanged && this.accessToken && (!this.expires || this.expires >= new Date())) { - return true; - } - return false; - } constructor( readonly id: string, public shop: string, diff --git a/src/auth/session/test/session.test.ts b/src/auth/session/test/session.test.ts index cf37463b6..07f327114 100644 --- a/src/auth/session/test/session.test.ts +++ b/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); @@ -19,16 +19,17 @@ describe('session', () => { describe('isActive', () => { it('returns true if session is active', () => { - const session = new Session('active'); - session.scope = 'test_scope'; + 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(); }); diff --git a/src/auth/session/types.ts b/src/auth/session/types.ts index 39a450c8d..d4e71df22 100644 --- a/src/auth/session/types.ts +++ b/src/auth/session/types.ts @@ -4,7 +4,7 @@ export interface SessionInterface { readonly id: string; shop: string; state: string; - scope: string; + scope?: string; expires?: Date; isOnline?: boolean; accessToken?: string; From 65226edb9e856403e250d24071b87ac35671d34e Mon Sep 17 00:00:00 2001 From: Emily Harber Date: Thu, 29 Apr 2021 15:00:52 -0400 Subject: [PATCH 3/4] Remove default value for isOnline --- CHANGELOG.md | 7 ++++--- src/auth/session/session.ts | 26 ++++++++++---------------- src/auth/session/test/session.test.ts | 2 +- src/auth/session/types.ts | 2 +- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 388e02996..effe8adec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,13 @@ 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) +### 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 - 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) +- Sessions no longer default to `false` for `isOnline` [#169](https://github.com/Shopify/shopify-node-api/pull/169) ## [1.2.0] - 2021-03-16 diff --git a/src/auth/session/session.ts b/src/auth/session/session.ts index 38462b1f3..6c17749ed 100644 --- a/src/auth/session/session.ts +++ b/src/auth/session/session.ts @@ -1,6 +1,5 @@ -// import {Context} from '../../context'; -import {OnlineAccessInfo} from '../oauth/types'; import {Context} from '../../context'; +import {OnlineAccessInfo} from '../oauth/types'; import {SessionInterface} from './types'; @@ -19,25 +18,20 @@ class Session implements SessionInterface { return newSession; } - public isActive(): boolean { - const scopesUnchanged = Context.SCOPES.equals(this.scope); - if (scopesUnchanged && this.accessToken && (!this.expires || this.expires >= new Date())) { - return true; - } - return false; - } - public scope?: string; public expires?: Date; public accessToken?: string; public onlineAccessInfo?: OnlineAccessInfo; - constructor( - readonly id: string, - public shop: string, - public state: string, - public isOnline: boolean = false, - ) {} + constructor(readonly id: string, public shop: string, public state: string, public isOnline: boolean) {} + + public isActive(): boolean { + const scopesUnchanged = Context.SCOPES.equals(this.scope); + if (scopesUnchanged && this.accessToken && (!this.expires || this.expires >= new Date())) { + return true; + } + return false; + } } export {Session}; diff --git a/src/auth/session/test/session.test.ts b/src/auth/session/test/session.test.ts index 07f327114..1b33b0cc1 100644 --- a/src/auth/session/test/session.test.ts +++ b/src/auth/session/test/session.test.ts @@ -20,7 +20,7 @@ describe('session', () => { describe('isActive', () => { it('returns true if session is active', () => { const session = new Session('active', 'active-shop', 'test_state', true); - session.scope = "test_scope" + session.scope = 'test_scope'; session.accessToken = 'indeed'; session.expires = new Date(Date.now() + 86400); diff --git a/src/auth/session/types.ts b/src/auth/session/types.ts index d4e71df22..30aa44850 100644 --- a/src/auth/session/types.ts +++ b/src/auth/session/types.ts @@ -4,9 +4,9 @@ export interface SessionInterface { readonly id: string; shop: string; state: string; + isOnline: boolean; scope?: string; expires?: Date; - isOnline?: boolean; accessToken?: string; onlineAccessInfo?: OnlineAccessInfo; isActive(): boolean; From 9c59dc2696c74ad68aa278227d28168de1f6c884 Mon Sep 17 00:00:00 2001 From: Emily Harber Date: Thu, 29 Apr 2021 15:29:07 -0400 Subject: [PATCH 4/4] Move changelog line to Unreleased --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index effe8adec..77092675b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ### Added - 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) @@ -16,7 +17,6 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [1.2.1] - 2021-03-26 ### Added - Added `April21` to `ApiVersion` [#149](https://github.com/Shopify/shopify-node-api/pull/149) -- Sessions no longer default to `false` for `isOnline` [#169](https://github.com/Shopify/shopify-node-api/pull/169) ## [1.2.0] - 2021-03-16