[Connectors] ConnectorTokenClient improvements

x-pack/plugins/actions/server/builtin_action_types/lib/connector_token_client.test.ts

@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import sinon from 'sinon';
 import { loggingSystemMock, savedObjectsClientMock } from '@kbn/core/server/mocks';
 import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
 import { ConnectorTokenClient } from './connector_token_client';
@@ -23,14 +24,21 @@ const encryptedSavedObjectsClient = encryptedSavedObjectsMock.createClient();
 
 let connectorTokenClient: ConnectorTokenClient;
 
+let clock: sinon.SinonFakeTimers;
+
+beforeAll(() => {
+  clock = sinon.useFakeTimers(new Date('2021-01-01T12:00:00.000Z'));
+});
 beforeEach(() => {
+  clock.reset();
   jest.resetAllMocks();
   connectorTokenClient = new ConnectorTokenClient({
     unsecuredSavedObjectsClient,
     encryptedSavedObjectsClient,
     logger,
   });
 });
+afterAll(() => clock.restore());
 
 describe('create()', () => {
   test('creates connector_token with all given properties', async () => {
@@ -131,7 +139,7 @@ describe('get()', () => {
     expect(result).toEqual({ connectorToken: null, hasErrors: false });
   });
 
-  test('return null and log the error if unsecuredSavedObjectsClient thows an error', async () => {
+  test('return null and log the error if unsecuredSavedObjectsClient throws an error', async () => {
     unsecuredSavedObjectsClient.find.mockRejectedValueOnce(new Error('Fail'));
 
     const result = await connectorTokenClient.get({
@@ -145,7 +153,7 @@ describe('get()', () => {
     expect(result).toEqual({ connectorToken: null, hasErrors: true });
   });
 
-  test('return null and log the error if encryptedSavedObjectsClient decrypt method thows an error', async () => {
+  test('return null and log the error if encryptedSavedObjectsClient decrypt method throws an error', async () => {
     const expectedResult = {
       total: 1,
       per_page: 10,
@@ -178,6 +186,47 @@ describe('get()', () => {
     ]);
     expect(result).toEqual({ connectorToken: null, hasErrors: true });
   });
+
+  test('return null and log the error if expiresAt is NaN', async () => {
+    const expectedResult = {
+      total: 1,
+      per_page: 10,
+      page: 1,
+      saved_objects: [
+        {
+          id: '1',
+          type: 'connector_token',
+          attributes: {
+            connectorId: '123',
+            tokenType: 'access_token',
+            createdAt: new Date().toISOString(),
+            expiresAt: 'yo',
+          },
+          score: 1,
+          references: [],
+        },
+      ],
+    };
+    unsecuredSavedObjectsClient.find.mockResolvedValueOnce(expectedResult);
+    encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockResolvedValueOnce({
+      id: '1',
+      type: 'connector_token',
+      references: [],
+      attributes: {
+        token: 'testtokenvalue',
+      },
+    });
+
+    const result = await connectorTokenClient.get({
+      connectorId: '123',
+      tokenType: 'access_token',
+    });
+
+    expect(logger.error.mock.calls[0]).toMatchObject([
+      `Failed to get connector_token for connectorId "123" and tokenType: "access_token". Error: expiresAt is not a valid Date "yo"`,
+    ]);
+    expect(result).toEqual({ connectorToken: null, hasErrors: true });
+  });
 });
 
 describe('update()', () => {
@@ -375,12 +424,60 @@ describe('updateOrReplace()', () => {
       connectorId: '1',
       token: null,
       newToken: 'newToken',
+      tokenRequestDate: undefined as unknown as number,
       expiresInSec: 1000,
       deleteExisting: false,
     });
     expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1);
-    expect((unsecuredSavedObjectsClient.create.mock.calls[0][1] as ConnectorToken).token).toBe(
-      'newToken'
+    expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith(
+      'connector_token',
+      {
+        connectorId: '1',
+        createdAt: '2021-01-01T12:00:00.000Z',
+        expiresAt: '2021-01-01T12:16:40.000Z',
+        token: 'newToken',
+        tokenType: 'access_token',
+        updatedAt: '2021-01-01T12:00:00.000Z',
+      },
+      { id: 'mock-saved-object-id' }
+    );
+
+    expect(unsecuredSavedObjectsClient.find).not.toHaveBeenCalled();
+    expect(unsecuredSavedObjectsClient.delete).not.toHaveBeenCalled();
+  });
+
+  test('uses tokenRequestDate to determine expire time if provided', async () => {
+    unsecuredSavedObjectsClient.create.mockResolvedValueOnce({
+      id: '1',
+      type: 'connector_token',
+      attributes: {
+        connectorId: '123',
+        tokenType: 'access_token',
+        token: 'testtokenvalue',
+        expiresAt: new Date('2021-01-01T08:00:00.000Z').toISOString(),
+      },
+      references: [],
+    });
+    await connectorTokenClient.updateOrReplace({
+      connectorId: '1',
+      token: null,
+      newToken: 'newToken',
+      tokenRequestDate: new Date('2021-03-03T00:00:00.000Z').getTime(),
+      expiresInSec: 1000,
+      deleteExisting: false,
+    });
+    expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledTimes(1);
+    expect(unsecuredSavedObjectsClient.create).toHaveBeenCalledWith(
+      'connector_token',
+      {
+        connectorId: '1',
+        createdAt: '2021-01-01T12:00:00.000Z',
+        expiresAt: '2021-03-03T00:16:40.000Z',
+        token: 'newToken',
+        tokenType: 'access_token',
+        updatedAt: '2021-01-01T12:00:00.000Z',
+      },
+      { id: 'mock-saved-object-id' }
     );
 
     expect(unsecuredSavedObjectsClient.find).not.toHaveBeenCalled();
@@ -434,6 +531,7 @@ describe('updateOrReplace()', () => {
       connectorId: '1',
       token: null,
       newToken: 'newToken',
+      tokenRequestDate: Date.now(),
       expiresInSec: 1000,
       deleteExisting: true,
     });
@@ -483,6 +581,7 @@ describe('updateOrReplace()', () => {
         expiresAt: new Date().toISOString(),
       },
       newToken: 'newToken',
+      tokenRequestDate: Date.now(),
       expiresInSec: 1000,
       deleteExisting: true,
     });

x-pack/plugins/actions/server/builtin_action_types/lib/connector_token_client.ts

@@ -38,6 +38,7 @@ interface UpdateOrReplaceOptions {
   token: ConnectorToken | null;
   newToken: string;
   expiresInSec: number;
+  tokenRequestDate: number;
   deleteExisting: boolean;
 }
 export class ConnectorTokenClient {
@@ -195,6 +196,7 @@ export class ConnectorTokenClient {
       return { hasErrors: false, connectorToken: null };
     }
 
+    let accessToken: string;
     try {
       const {
         attributes: { token },
@@ -203,14 +205,7 @@ export class ConnectorTokenClient {
         connectorTokensResult[0].id
       );
 
-      return {
-        hasErrors: false,
-        connectorToken: {
-          id: connectorTokensResult[0].id,
-          ...connectorTokensResult[0].attributes,
-          token,
-        },
-      };
+      accessToken = token;
     } catch (err) {
       this.logger.error(
         `Failed to decrypt connector_token for connectorId "${connectorId}" and tokenType: "${
@@ -219,6 +214,24 @@ export class ConnectorTokenClient {
       );
       return { hasErrors: true, connectorToken: null };
     }
+
+    if (isNaN(Date.parse(connectorTokensResult[0].attributes.expiresAt))) {
+      this.logger.error(
+        `Failed to get connector_token for connectorId "${connectorId}" and tokenType: "${
+          tokenType ?? 'access_token'
+        }". Error: expiresAt is not a valid Date "${connectorTokensResult[0].attributes.expiresAt}"`
+      );
+      return { hasErrors: true, connectorToken: null };
+    }
+
+    return {
+      hasErrors: false,
+      connectorToken: {
+        id: connectorTokensResult[0].id,
+        ...connectorTokensResult[0].attributes,
+        token: accessToken,
+      },
+    };
   }
 
   /**
@@ -258,9 +271,11 @@ export class ConnectorTokenClient {
     token,
     newToken,
     expiresInSec,
+    tokenRequestDate,
     deleteExisting,
   }: UpdateOrReplaceOptions) {
     expiresInSec = expiresInSec ?? 3600;
+    tokenRequestDate = tokenRequestDate ?? Date.now();
     if (token === null) {
       if (deleteExisting) {
         await this.deleteConnectorTokens({
@@ -272,14 +287,14 @@ export class ConnectorTokenClient {
       await this.create({
         connectorId,
         token: newToken,
-        expiresAtMillis: new Date(Date.now() + expiresInSec * 1000).toISOString(),
+        expiresAtMillis: new Date(tokenRequestDate + expiresInSec * 1000).toISOString(),
         tokenType: 'access_token',
       });
     } else {
       await this.update({
         id: token.id!.toString(),
         token: newToken,
-        expiresAtMillis: new Date(Date.now() + expiresInSec * 1000).toISOString(),
+        expiresAtMillis: new Date(tokenRequestDate + expiresInSec * 1000).toISOString(),
         tokenType: 'access_token',
       });
     }

x-pack/plugins/actions/server/builtin_action_types/lib/get_oauth_client_credentials_access_token.test.ts

@@ -4,6 +4,7 @@
  * 2.0; you may not use this file except in compliance with the Elastic License
  * 2.0.
  */
+import sinon from 'sinon';
 import { Logger } from '@kbn/core/server';
 import { asyncForEach } from '@kbn/std';
 import { loggingSystemMock } from '@kbn/core/server/mocks';
@@ -20,7 +21,15 @@ const logger = loggingSystemMock.create().get() as jest.Mocked<Logger>;
 const configurationUtilities = actionsConfigMock.create();
 const connectorTokenClient = connectorTokenClientMock.create();
 
+let clock: sinon.SinonFakeTimers;
+
 describe('getOAuthClientCredentialsAccessToken', () => {
+  beforeAll(() => {
+    clock = sinon.useFakeTimers(new Date('2021-01-01T12:00:00.000Z'));
+  });
+  beforeEach(() => clock.reset());
+  afterAll(() => clock.restore());
+
   const getOAuthClientCredentialsAccessTokenOpts = {
     connectorId: '123',
     logger,
@@ -52,8 +61,8 @@ describe('getOAuthClientCredentialsAccessToken', () => {
         connectorId: '123',
         tokenType: 'access_token',
         token: 'testtokenvalue',
-        createdAt: new Date().toISOString(),
-        expiresAt: new Date(Date.now() + 10000000000).toISOString(),
+        createdAt: new Date('2021-01-01T08:00:00.000Z').toISOString(),
+        expiresAt: new Date('2021-01-02T13:00:00.000Z').toISOString(),
       },
     });
     const accessToken = await getOAuthClientCredentialsAccessToken(
@@ -95,14 +104,15 @@ describe('getOAuthClientCredentialsAccessToken', () => {
       connectorId: '123',
       token: null,
       newToken: 'access_token brandnewaccesstoken',
+      tokenRequestDate: 1609502400000,
       expiresInSec: 1000,
       deleteExisting: false,
     });
   });
 
   test('creates new assertion if stored access token exists but is expired', async () => {
-    const createdAt = new Date().toISOString();
-    const expiresAt = new Date(Date.now() - 100).toISOString();
+    const createdAt = new Date('2021-01-01T08:00:00.000Z').toISOString();
+    const expiresAt = new Date('2021-01-01T09:00:00.000Z').toISOString();
     connectorTokenClient.get.mockResolvedValueOnce({
       hasErrors: false,
       connectorToken: {
@@ -147,6 +157,7 @@ describe('getOAuthClientCredentialsAccessToken', () => {
         expiresAt,
       },
       newToken: 'access_token brandnewaccesstoken',
+      tokenRequestDate: 1609502400000,
       expiresInSec: 1000,
       deleteExisting: false,
     });
@@ -210,6 +221,7 @@ describe('getOAuthClientCredentialsAccessToken', () => {
     (requestOAuthClientCredentialsToken as jest.Mock).mockResolvedValueOnce({
       tokenType: 'access_token',
       accessToken: 'brandnewaccesstoken',
+      tokenRequestDate: 1609502400000,
       expiresIn: 1000,
     });
     connectorTokenClient.updateOrReplace.mockRejectedValueOnce(new Error('updateOrReplace error'));
@@ -268,6 +280,7 @@ describe('getOAuthClientCredentialsAccessToken', () => {
     (requestOAuthClientCredentialsToken as jest.Mock).mockResolvedValueOnce({
       tokenType: 'access_token',
       accessToken: 'brandnewaccesstoken',
+      tokenRequestDate: 1609502400000,
       expiresIn: 1000,
     });
 

x-pack/plugins/actions/server/builtin_action_types/lib/get_oauth_client_credentials_access_token.ts

@@ -62,6 +62,9 @@ export const getOAuthClientCredentialsAccessToken = async ({
   }
 
   if (connectorToken === null || Date.parse(connectorToken.expiresAt) <= Date.now()) {
+    // Save the time before requesting token so we can use it to calculate expiration
+    const requestTokenStart = Date.now();
+
     // request access token with jwt assertion
     const tokenResult = await requestOAuthClientCredentialsToken(
       tokenUrl,
@@ -82,6 +85,7 @@ export const getOAuthClientCredentialsAccessToken = async ({
           connectorId,
           token: connectorToken,
           newToken: accessToken,
+          tokenRequestDate: requestTokenStart,
           expiresInSec: tokenResult.expiresIn,
           deleteExisting: hasErrors,
         });