diff --git a/CHANGELOG.md b/CHANGELOG.md index 04270411e..79396d850 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/auth/oauth/oauth.ts b/src/auth/oauth/oauth.ts index 1e9688a4f..14bc16b47 100644 --- a/src/auth/oauth/oauth.ts +++ b/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.', diff --git a/src/auth/oauth/test/oauth.test.ts b/src/auth/oauth/test/oauth.test.ts index f37ae3ef7..2feb6e3ae 100644 --- a/src/auth/oauth/test/oauth.test.ts +++ b/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); diff --git a/src/auth/scopes/index.ts b/src/auth/scopes/index.ts index d112360e2..4424e816a 100644 --- a/src/auth/scopes/index.ts +++ b/src/auth/scopes/index.ts @@ -4,13 +4,13 @@ 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; } @@ -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) { diff --git a/src/auth/scopes/test/scopes.test.ts b/src/auth/scopes/test/scopes.test.ts index 11f8ae3ca..3e5df5335 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 d7d2d7c24..8a59cc83b 100644 --- a/src/auth/session/session.ts +++ b/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); diff --git a/src/auth/session/storage/custom.ts b/src/auth/session/storage/custom.ts index 7df831942..e67103fcd 100644 --- a/src/auth/session/storage/custom.ts +++ b/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 ${ 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 8f6f48909..83059e0a0 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/auth/session/test/session.test.ts b/src/auth/session/test/session.test.ts index 79cb13829..5fa225d6e 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); @@ -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(); }); diff --git a/src/auth/session/types.ts b/src/auth/session/types.ts index 39a450c8d..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; - scope: string; + isOnline: boolean; + scope?: string; expires?: Date; - isOnline?: boolean; accessToken?: string; onlineAccessInfo?: OnlineAccessInfo; isActive(): boolean; diff --git a/src/utils/test/delete-current-session.test.ts b/src/utils/test/delete-current-session.test.ts index c42488761..ef88e0708 100644 --- a/src/utils/test/delete-current-session.test.ts +++ b/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), 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 38bfe5980..c79a8fd9e 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, { @@ -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); diff --git a/src/utils/test/load-current-session.test.ts b/src/utils/test/load-current-session.test.ts index 16f4cccd3..4e1c7e048 100644 --- a/src/utils/test/load-current-session.test.ts +++ b/src/utils/test/load-current-session.test.ts @@ -40,7 +40,12 @@ 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); @@ -76,7 +81,12 @@ 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); @@ -140,7 +150,12 @@ 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); @@ -159,7 +174,12 @@ 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); @@ -185,6 +205,9 @@ describe('loadCurrentSession', () => { const session = new Session( ShopifyOAuth.getOfflineSessionId('test-shop.myshopify.io'), + 'test-shop.myshopify.io', + 'state', + false, ); await expect( Context.SESSION_STORAGE.storeSession(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 9a317982b..27849c5ff 100644 --- a/src/utils/test/store-session.test.ts +++ b/src/utils/test/store-session.test.ts @@ -36,7 +36,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 9d1c99e23..b223aa4d0 100644 --- a/src/utils/test/with-session.test.ts +++ b/src/utils/test/with-session.test.ts @@ -36,9 +36,7 @@ describe('withSession', () => { /* we need to set up a session for this test, because errors will be thrown for a missing session before 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); @@ -61,9 +59,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( @@ -73,9 +69,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); @@ -104,9 +98,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); @@ -146,9 +138,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);