Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix eslint warnings and TODOs in OAuth module #1425

Merged
merged 1 commit into from Feb 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 21 additions & 53 deletions examples/oauth-v2/app.js
@@ -1,72 +1,30 @@
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,
});
// 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'));
Expand All @@ -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',
})

Expand Down Expand Up @@ -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);
seratch marked this conversation as resolved.
Show resolved Hide resolved
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({
Expand All @@ -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`))
12 changes: 12 additions & 0 deletions 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

2 changes: 1 addition & 1 deletion examples/oauth-v2/package.json
Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions examples/oauth-v2/unlink.sh
@@ -0,0 +1,4 @@
#!/bin/bash

rm -rf node_modules/
npm i @slack/oauth
2 changes: 1 addition & 1 deletion packages/oauth/src/install-provider.spec.js
Expand Up @@ -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"})');
}
});

Expand Down
84 changes: 56 additions & 28 deletions packages/oauth/src/install-provider.ts
Expand Up @@ -12,6 +12,8 @@ import {
MissingCodeError,
GenerateInstallUrlError,
AuthorizationError,
CodedError,
ErrorCode,
} from './errors';
import { Installation, OrgInstallation } from './installation';
import { InstallationQuery } from './installation-query';
Expand Down Expand Up @@ -119,7 +121,9 @@ export class InstallProvider {
* Fetches data from the installationStore
*/
public async authorize(source: InstallationQuery<boolean>): Promise<AuthorizeResult> {
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.
Expand All @@ -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 = {};
Expand Down Expand Up @@ -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
Expand All @@ -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})`);
}
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -604,12 +618,26 @@ interface AuthTestResult extends WebAPICallResult {
async function runAuthTest(token: string, clientOptions: WebClientOptions): Promise<AuthTestResult> {
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.
*
Expand Down
17 changes: 15 additions & 2 deletions packages/oauth/src/stores/file-store.ts
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented


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');
Expand Down