From 593b2ceaa989aebec309840b536a60bcf819ae58 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Tue, 15 Feb 2022 23:34:50 +0900 Subject: [PATCH] Fix eslint warnings and TODOs in OAuth module --- examples/oauth-v2/app.js | 74 ++++++------------ examples/oauth-v2/link.sh | 12 +++ examples/oauth-v2/package.json | 2 +- examples/oauth-v2/unlink.sh | 4 + packages/oauth/src/install-provider.spec.js | 2 +- packages/oauth/src/install-provider.ts | 84 ++++++++++++++------- packages/oauth/src/stores/file-store.ts | 17 ++++- 7 files changed, 110 insertions(+), 85 deletions(-) create mode 100755 examples/oauth-v2/link.sh create mode 100755 examples/oauth-v2/unlink.sh diff --git a/examples/oauth-v2/app.js b/examples/oauth-v2/app.js index 6701009f2..ea1fc559f 100644 --- a/examples/oauth-v2/app.js +++ b/examples/oauth-v2/app.js @@ -1,15 +1,11 @@ -const { InstallProvider } = require('@slack/oauth'); +const { InstallProvider, LogLevel, FileInstallationStore } = require('@slack/oauth'); const { createEventAdapter } = require('@slack/events-api'); const { WebClient } = require('@slack/web-api'); const express = require('express'); -// Using Keyv as an interface to our database -// see https://github.com/lukechilds/keyv for more info -const Keyv = require('keyv'); const app = express(); const port = 3000; - // Initialize slack events adapter const slackEvents = createEventAdapter(process.env.SLACK_SIGNING_SECRET, { includeBody: true, @@ -17,56 +13,18 @@ const slackEvents = createEventAdapter(process.env.SLACK_SIGNING_SECRET, { // Set path to receive events app.use('/slack/events', slackEvents.requestListener()); -// can use different keyv db adapters here -// ex: const keyv = new Keyv('redis://user:pass@localhost:6379'); -// using the basic in-memory one below -const keyv = new Keyv(); - -keyv.on('error', err => console.log('Connection Error', err)); +const scopes = ['channels:read', 'groups:read', 'channels:manage', 'chat:write', 'incoming-webhook']; +const userScopes = ['chat:write']; const installer = new InstallProvider({ clientId: process.env.SLACK_CLIENT_ID, clientSecret: process.env.SLACK_CLIENT_SECRET, authVersion: 'v2', stateSecret: 'my-state-secret', - installationStore: { - storeInstallation: async (installation) => { - if (installation.isEnterpriseInstall) { - // storing org installation - return await keyv.set(installation.enterprise.id, installation); - } else if (installation.team !== null && installation.team.id !== undefined) { - // storing single team installation - return await keyv.set(installation.team.id, installation);; - } - throw new Error('Failed saving installation data to installationStore'); - }, - fetchInstallation: async (installQuery) => { - if (installQuery.isEnterpriseInstall) { - if (installQuery.enterpriseId !== undefined) { - // fetching org installation - return await keyv.get(installQuery.enterpriseId) - } - } - if (installQuery.teamId !== undefined) { - // fetching single team installation - return await keyv.get(installQuery.teamId); - } - throw new Error('Failed fetching installation'); - }, - deleteInstallation: async (installQuery) => { - if (installQuery.isEnterpriseInstall) { - if (installQuery.enterpriseId !== undefined) { - // delete org installation - return await keyv.delete(installQuery.enterpriseId) - } - } - if (installQuery.teamId !== undefined) { - // delete single team installation - return await keyv.delete(installQuery.teamId); - } - throw new Error('Failed to delete installation'); - }, - }, + scopes, + userScopes, + installationStore: new FileInstallationStore(), + logLevel: LogLevel.DEBUG, }); app.get('/', (req, res) => res.send('go to /slack/install')); @@ -75,7 +33,8 @@ app.get('/slack/install', async (req, res, next) => { try { // feel free to modify the scopes const url = await installer.generateInstallUrl({ - scopes: ['channels:read', 'groups:read', 'channels:manage', 'chat:write', 'incoming-webhook'], + scopes, + userScopes, metadata: 'some_metadata', }) @@ -108,15 +67,25 @@ app.get('/slack/oauth_redirect', async (req, res) => { // When a user navigates to the app home, grab the token from our database and publish a view slackEvents.on('app_home_opened', async (event, body) => { + console.log(event); try { if (event.tab === 'home') { let DBInstallData; if (body.authorizations !== undefined && body.authorizations[0].is_enterprise_install) { //org wide installation - DBInstallData = await installer.authorize({enterpriseId: body.enterprise_id, isEnterpriseInstall: true}); + DBInstallData = await installer.authorize({ + enterpriseId: body.enterprise_id, + userId: event.user, + isEnterpriseInstall: true, + }); } else { // non org wide installation - DBInstallData = await installer.authorize({teamId:body.team_id, isEnterpriseInstall: false}); + DBInstallData = await installer.authorize({ + enterpriseId: body.enterprise_id, + teamId: body.team_id, + userId: event.user, + isEnterpriseInstall: false, + }); } const web = new WebClient(DBInstallData.botToken); await web.views.publish({ @@ -141,5 +110,4 @@ slackEvents.on('app_home_opened', async (event, body) => { console.error(error); } }); - app.listen(port, () => console.log(`Example app listening on port ${port}! Go to http://localhost:3000/slack/install to initiate oauth flow`)) diff --git a/examples/oauth-v2/link.sh b/examples/oauth-v2/link.sh new file mode 100755 index 000000000..1074d6dbc --- /dev/null +++ b/examples/oauth-v2/link.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +current_dir=`dirname $0` +cd ${current_dir} +npm unlink @slack/oauth \ + && npm i \ + && cd ../../packages/oauth \ + && npm link \ + && cd - \ + && npm i \ + && npm link @slack/oauth + diff --git a/examples/oauth-v2/package.json b/examples/oauth-v2/package.json index 103935c8c..fbd0c495a 100644 --- a/examples/oauth-v2/package.json +++ b/examples/oauth-v2/package.json @@ -11,7 +11,7 @@ "repository": "slackapi/node-slack-sdk", "dependencies": { "@slack/events-api": "^2.3.2", - "@slack/oauth": "feat-org-apps", + "@slack/oauth": "^2.4.0", "@slack/web-api": "^5.8.0", "express": "^4.17.1", "keyv": "^4.0.0" diff --git a/examples/oauth-v2/unlink.sh b/examples/oauth-v2/unlink.sh new file mode 100755 index 000000000..d18501200 --- /dev/null +++ b/examples/oauth-v2/unlink.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +rm -rf node_modules/ +npm i @slack/oauth diff --git a/packages/oauth/src/install-provider.spec.js b/packages/oauth/src/install-provider.spec.js index e3d050e07..911ff2208 100644 --- a/packages/oauth/src/install-provider.spec.js +++ b/packages/oauth/src/install-provider.spec.js @@ -403,7 +403,7 @@ describe('InstallProvider', async () => { assert.fail('Should have failed'); } catch (error) { assert.equal(error.code, ErrorCode.AuthorizationError); - assert.equal(error.message, 'Failed fetching data from the Installation Store'); + assert.equal(error.message, 'Failed fetching data from the Installation Store (source: {"teamId":"non_existing_team_id"})'); } }); diff --git a/packages/oauth/src/install-provider.ts b/packages/oauth/src/install-provider.ts index af9359a0d..845ccf2c0 100644 --- a/packages/oauth/src/install-provider.ts +++ b/packages/oauth/src/install-provider.ts @@ -12,6 +12,8 @@ import { MissingCodeError, GenerateInstallUrlError, AuthorizationError, + CodedError, + ErrorCode, } from './errors'; import { Installation, OrgInstallation } from './installation'; import { InstallationQuery } from './installation-query'; @@ -119,7 +121,9 @@ export class InstallProvider { * Fetches data from the installationStore */ public async authorize(source: InstallationQuery): Promise { + const sourceForLogging = JSON.stringify(source); try { + this.logger.debug(`Starting authorize() execution (source: ${sourceForLogging})`); // Note that `queryResult` may unexpectedly include null values for some properties. // For example, MongoDB can often save properties as null for some reasons. // Inside this method, we should alwayss check if a value is either undefined or null. @@ -131,7 +135,7 @@ export class InstallProvider { } if (queryResult === undefined || queryResult === null) { - throw new Error('Failed fetching data from the Installation Store'); + throw new Error(`Failed fetching data from the Installation Store (source: ${sourceForLogging})`); } const authResult: AuthorizeResult = {}; @@ -183,14 +187,17 @@ export class InstallProvider { const tokensToRefresh: string[] = detectExpiredOrExpiringTokens(authResult, currentUTCSec); if (tokensToRefresh.length > 0) { - const installationUpdates: any = { ...queryResult }; // TODO :: TS - const refreshResponses = await this.refreshExpiringTokens(tokensToRefresh); - - // TODO: perhaps this for..of loop could introduce an async delay due to await'ing once for each refreshResp? - // Could we rewrite to be more performant and not trigger the eslint warning? Perhaps a concurrent async - // map/reduce? But will the return value be the same? Does order of this array matter? - // eslint-disable-next-line no-restricted-syntax - for (const refreshResp of refreshResponses) { + if (queryResult.authVersion !== 'v2') { + const errorMessage = 'Unexpected data structure detected. ' + + 'The data returned by your InstallationStore#fetchInstallation() method must have "authVersion": "v2" ' + + 'if it has a refresh token'; + throw new UnknownError(errorMessage); + } + const installationUpdates: TokenRefreshInstallationUpdates = { + ...queryResult as Installation<'v2', boolean>, + }; + const refreshResponses: OAuthV2TokenRefreshResponse[] = await this.refreshExpiringTokens(tokensToRefresh); + refreshResponses.forEach((refreshResp) => { const tokenType = refreshResp.token_type; // Update Authorization @@ -207,25 +214,28 @@ export class InstallProvider { } // Update Installation - installationUpdates[tokenType].token = refreshResp.access_token; - installationUpdates[tokenType].refreshToken = refreshResp.refresh_token; - installationUpdates[tokenType].expiresAt = currentUTCSec + refreshResp.expires_in; - - const updatedInstallation = { - ...installationUpdates, - [tokenType]: { ...queryResult[tokenType], ...installationUpdates[tokenType] }, - }; - - // TODO: related to the above TODO comment as well - // eslint-disable-next-line no-await-in-loop - await this.installationStore.storeInstallation(updatedInstallation); - } + const botOrUser = installationUpdates[tokenType]; + if (botOrUser !== undefined) { + this.logger.debug(`Saving ${tokenType} token and its refresh token in InstallationStore`); + botOrUser.token = refreshResp.access_token; + botOrUser.refreshToken = refreshResp.refresh_token; + botOrUser.expiresAt = currentUTCSec + refreshResp.expires_in; + } else { + const errorMessage = `Unexpected data structure detected. The data returned by your InstallationStore#fetchInstallation() method must have ${tokenType} at top-level`; + throw new UnknownError(errorMessage); + } + }); + await this.installationStore.storeInstallation(installationUpdates); + this.logger.debug('Refreshed tokens have been saved in InstallationStore'); } } return authResult; - } catch (error: any) { - throw new AuthorizationError(error.message); + } catch (error) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + throw new AuthorizationError((error as any).message); + } finally { + this.logger.debug(`Completed authorize() execution (source: ${sourceForLogging})`); } } @@ -504,7 +514,7 @@ export class InstallProvider { this.logger.debug('run built-in success function'); defaultCallbackSuccess(installation, installOptions, req, res); } - } catch (error: any) { + } catch (error) { this.logger.error(error); if (!installOptions) { @@ -515,12 +525,16 @@ export class InstallProvider { } // Call the failure callback + const codedError: CodedError = error as CodedError; + if (codedError.code === undefined) { + codedError.code = ErrorCode.UnknownError; + } if (options !== undefined && options.failure !== undefined) { this.logger.debug('calling passed in options.failure'); - options.failure(error, installOptions, req, res); + options.failure(codedError, installOptions, req, res); } else { this.logger.debug('run built-in failure function'); - defaultCallbackFailure(error, installOptions, req, res); + defaultCallbackFailure(codedError, installOptions, req, res); } } } @@ -604,12 +618,26 @@ interface AuthTestResult extends WebAPICallResult { async function runAuthTest(token: string, clientOptions: WebClientOptions): Promise { const client = new WebClient(token, clientOptions); const authResult = await client.auth.test(); - return authResult as any as AuthTestResult; + return authResult as unknown as AuthTestResult; } // --------------------- // token rotation +// This type is used only in this source file +type TokenRefreshInstallationUpdates = Installation<'v2', boolean> & { + bot?: { + token: string, + refreshToken?: string; + expiresAt?: number; + }; + user: { + token?: string, + refreshToken?: string; + expiresAt?: number; + }; +}; + /** * detectExpiredOrExpiringTokens determines access tokens' eligibility for refresh. * diff --git a/packages/oauth/src/stores/file-store.ts b/packages/oauth/src/stores/file-store.ts index d54b00fdc..4becf259f 100644 --- a/packages/oauth/src/stores/file-store.ts +++ b/packages/oauth/src/stores/file-store.ts @@ -63,11 +63,24 @@ export default class FileInstallationStore implements InstallationStore { throw new Error('enterpriseId is required to fetch data of an enterprise installation'); } - // TODO :: introduce support for user tokens + querying using userId - try { const data = fs.readFileSync(path.resolve(`${installationDir}/app-latest`)); const installation: Installation = JSON.parse(data.toString()); + if (query.userId && installation.user.id !== query.userId) { + try { + const userData = fs.readFileSync(path.resolve(`${installationDir}/user-${query.userId}-latest`)); + if (userData !== undefined && userData !== null) { + const userInstallation: Installation = JSON.parse(userData.toString()); + installation.user = userInstallation.user; + } + } catch (err) { + logger?.debug(`The user-token installation for the request user (user_id: ${query.userId}) was not found.`); + delete installation.user.token; + delete installation.user.refreshToken; + delete installation.user.expiresAt; + delete installation.user.scopes; + } + } return installation; } catch (err) { throw new Error('Failed to fetch data from FileInstallationStore');