From 194ea8c32f1fdcad046c1c4d494a7c4686f74fbf Mon Sep 17 00:00:00 2001 From: daffl Date: Thu, 24 Nov 2022 13:28:11 -0800 Subject: [PATCH] fix(authentication-client): Do not cache authentication errors --- packages/authentication-client/src/core.ts | 2 +- .../authentication-client/test/index.test.ts | 26 ++- .../test/integration/commons.ts | 179 ++++++++---------- 3 files changed, 103 insertions(+), 104 deletions(-) diff --git a/packages/authentication-client/src/core.ts b/packages/authentication-client/src/core.ts index 07db6190f4..75f60a4875 100644 --- a/packages/authentication-client/src/core.ts +++ b/packages/authentication-client/src/core.ts @@ -134,7 +134,7 @@ export class AuthenticationClient { return type === 'logout' ? promise : promise.then(() => Promise.reject(error)) } - return Promise.reject(error) + return this.reset().then(() => Promise.reject(error)) } reAuthenticate(force = false, strategy?: string): Promise { diff --git a/packages/authentication-client/test/index.test.ts b/packages/authentication-client/test/index.test.ts index de8f9210e6..1720b45a4e 100644 --- a/packages/authentication-client/test/index.test.ts +++ b/packages/authentication-client/test/index.test.ts @@ -17,29 +17,29 @@ describe('@feathersjs/authentication-client', () => { app.configure(client()) app.use('/authentication', { - create(data: any) { + async create(data: any) { if (data.error) { - return Promise.reject(new Error('Did not work')) + throw new Error('Did not work') } - return Promise.resolve({ + return { accessToken, data, user - }) + } }, - remove(id) { + async remove(id) { if (!app.get('authentication')) { throw new NotAuthenticated('Not logged in') } - return Promise.resolve({ id }) + return { id } } }) app.use('dummy', { - find(params) { - return Promise.resolve(params) + async find(params) { + return params } }) }) @@ -147,6 +147,16 @@ describe('@feathersjs/authentication-client', () => { assert.strictEqual(at, accessToken) }) + it('resets after any error (#1947)', async () => { + await assert.rejects(() => app.authenticate({ strategy: 'testing', error: true }), { + message: 'Did not work' + }) + + const found = await app.service('dummy').find() + + assert.deepStrictEqual(found, {}) + }) + it('logout when not logged in without error', async () => { const result = await app.logout() diff --git a/packages/authentication-client/test/integration/commons.ts b/packages/authentication-client/test/integration/commons.ts index b9eef46587..4973530792 100644 --- a/packages/authentication-client/test/integration/commons.ts +++ b/packages/authentication-client/test/integration/commons.ts @@ -11,119 +11,108 @@ export default ( let client: Application let user: any - before(() => - getApp() - .service('users') - .create({ + before( + async () => + (user = await getApp().service('users').create({ email, password - }) - .then((result: any) => { - user = result - }) + })) ) beforeEach(() => { client = getClient() }) - it('authenticates with local strategy', () => { - return client - .authenticate({ - strategy: 'local', - email, - password - }) - .then((result) => { - assert.ok(result.accessToken) - assert.strictEqual(result.authentication.strategy, 'local') - assert.strictEqual(result.user.email, email) - }) + after(async () => { + await getApp().service('users').remove(user.id) }) - it('authentication with wrong credentials fails, does not maintain state', () => { - return client - .authenticate({ - strategy: 'local', - email, - password: 'blabla' - }) - .then(() => assert.fail('Should never get here')) - .catch((error) => { - assert.strictEqual(error.name, 'NotAuthenticated') - assert.strictEqual(error.message, 'Invalid login') - assert.ok(!client.get('authentication'), 'Reset client state') - }) + it('authenticates with local strategy', async () => { + const result = await client.authenticate({ + strategy: 'local', + email, + password + }) + + assert.ok(result.accessToken) + assert.strictEqual(result.authentication.strategy, 'local') + assert.strictEqual(result.user.email, email) }) - it('errors when not authenticated', () => { - return client - .service('dummy') - .find() - .then(() => assert.fail('Should never get here')) - .catch((error: any) => { - assert.strictEqual(error.name, 'NotAuthenticated') - assert.strictEqual(error.code, 401) - assert.strictEqual(error.message, 'Not authenticated') - }) + it('authentication with wrong credentials fails, does not maintain state', async () => { + await assert.rejects( + () => + client.authenticate({ + strategy: 'local', + email, + password: 'blabla' + }), + { + name: 'NotAuthenticated', + message: 'Invalid login' + } + ) + assert.ok(!client.get('authentication'), 'Reset client state') }) - it('authenticates and allows access', () => { - return client - .authenticate({ - strategy: 'local', - email, - password - }) - .then(() => client.service('dummy').find()) - .then((result) => { - assert.strictEqual(result.provider, provider) - assert.ok(result.authentication) - assert.ok(result.authentication.payload) - assert.strictEqual(result.user.email, user.email) - assert.strictEqual(result.user.id, user.id) - }) + it('errors when not authenticated', async () => { + await assert.rejects(() => client.service('dummy').find(), { + name: 'NotAuthenticated', + code: 401, + message: 'Not authenticated' + }) }) - it('re-authenticates', () => { - return client - .authenticate({ - strategy: 'local', - email, - password - }) - .then(() => client.authentication.reset()) - .then(() => client.authenticate()) - .then(() => client.service('dummy').find()) - .then((result) => { - assert.strictEqual(result.provider, provider) - assert.ok(result.authentication) - assert.ok(result.authentication.payload) - assert.strictEqual(result.user.email, user.email) - assert.strictEqual(result.user.id, user.id) - }) + it('authenticates and allows access', async () => { + await client.authenticate({ + strategy: 'local', + email, + password + }) + const result = await client.service('dummy').find() + + assert.strictEqual(result.provider, provider) + assert.ok(result.authentication) + assert.ok(result.authentication.payload) + assert.strictEqual(result.user.email, user.email) + assert.strictEqual(result.user.id, user.id) }) - it('after logout does not allow subsequent access', () => { - return client - .authenticate({ - strategy: 'local', - email, - password - }) - .then(() => client.logout()) - .then((result) => { - assert.ok(result.accessToken) - assert.ok(result.user) - - return client.service('dummy').find() - }) - .then(() => assert.fail('Should never get here')) - .catch((error) => { - assert.strictEqual(error.name, 'NotAuthenticated') - assert.strictEqual(error.code, 401) - assert.strictEqual(error.message, 'Not authenticated') - }) + it('re-authenticates', async () => { + await client.authenticate({ + strategy: 'local', + email, + password + }) + + client.authentication.reset() + client.authenticate() + const result = await client.service('dummy').find() + + assert.strictEqual(result.provider, provider) + assert.ok(result.authentication) + assert.ok(result.authentication.payload) + assert.strictEqual(result.user.email, user.email) + assert.strictEqual(result.user.id, user.id) + }) + + it('after logout does not allow subsequent access', async () => { + await client.authenticate({ + strategy: 'local', + email, + password + }) + + const result = await client.logout() + + assert.ok(result!.accessToken) + assert.ok(result!.user) + + assert.rejects(() => client.service('dummy').find(), { + name: 'NotAuthenticated', + code: 401, + message: 'Not authenticated' + }) }) }) }