Skip to content

Commit

Permalink
Refactor the internals of InstallProvider for better null safety (#1411)
Browse files Browse the repository at this point in the history 
* Refactor the internals of InstallProvider for better null safety
* A minor fix thanks to Fil
  • Loading branch information
@seratch
seratch authored Jan 12, 2022
1 parent b30ae11 commit 4ad3827
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 13 deletions.
96 changes: 96 additions & 0 deletions packages/oauth/src/index.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,102 @@ describe('OAuth', async () => {
// fsReaddirSync returns ['app-latest', 'user-userId-latest']
expect(fsUnlink.callCount).equals(1);
});

it('should run authorize with triage-bot\'s MongoDB data', async () => {
// Refer to https://github.com/slackapi/bolt-js/issues/1265 to learn the context
const storedInstallation = {
"_id": "6.....",
"id": "T....",
"__v": 0,
"appId": "A...",
"authVersion": "v2",
"bot": {
"scopes": [
"channels:history",
"channels:join",
"channels:read",
"chat:write",
"commands",
"files:write"
],
"token": "xoxb-...",
"userId": "U...",
"id": "B02SS7QU407"
},
"db_record_created_at": "2022-01-08T02:24:40.470Z",
"db_record_updated_at": "2022-01-08T02:24:40.470Z",
"enterprise": null,
"isEnterpriseInstall": false,
"name": "My Team",
"tokenType": "bot",
"user": {
"scopes": null,
"id": "U..."
}
};
const installationStore = {
fetchInstallation: (_) => {
return new Promise((resolve) => {
resolve(storedInstallation);
});
},
storeInstallation: () => {},
deleteInstallation: (_) => {},
}
const installer = new InstallProvider({ clientId, clientSecret, stateSecret, installationStore });
const authorizeResult = await installer.authorize({ teamId: 'T111' });
assert.deepEqual(authorizeResult, {
"teamId": "T111",
"botId": "B02SS7QU407",
"botUserId": "U...",
"botToken": "xoxb-...",
"userToken": undefined,
});
});
it('should run authorize even if there are null objects in data', async () => {
const storedInstallation = {
// https://github.com/slackapi/template-triage-bot/blob/c1e54fb9d760b46cc8809c57e307061fdb3e0a91/app.js#L51-L55
id: "T999", // template-triage-bot specific
name: 'My Team', // template-triage-bot specific
appId: 'A111',
tokenType: 'bot',
authVersion: 'v2',
bot: {
id: 'B111',
userId: 'U111',
scopes: [
'channels:history',
'channels:join',
'channels:read',
'chat:write',
'commands',
'files:write',
],
token: 'xoxb-____',
},
enterprise: null,
team: null, // v2.3 does not work with this data due to "Error: Cannot read property 'id' of null"
isEnterpriseInstall: false,
user: null,
}
const installationStore = {
fetchInstallation: (_) => {
return new Promise((resolve) => {
resolve(storedInstallation);
});
},
storeInstallation: () => {},
deleteInstallation: (_) => {},
}
const installer = new InstallProvider({ clientId, clientSecret, stateSecret, installationStore });
const authorizeResult = await installer.authorize({ teamId: 'T111' });
assert.deepEqual(authorizeResult, {
"teamId": "T111",
"botId": "B111",
"botUserId": "U111",
"botToken": "xoxb-____",
});
});
});

describe('ClearStateStore', async () => {
Expand Down
34 changes: 21 additions & 13 deletions packages/oauth/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,26 +134,32 @@ export class InstallProvider {
*/
public async authorize(source: InstallationQuery<boolean>): Promise<AuthorizeResult> {
try {
// 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.
let queryResult;
if (source.isEnterpriseInstall) {
queryResult = await this.installationStore.fetchInstallation(source as InstallationQuery<true>, this.logger);
} else {
queryResult = await this.installationStore.fetchInstallation(source as InstallationQuery<false>, this.logger);
}

if (queryResult === undefined) {
if (queryResult === undefined || queryResult === null) {
throw new Error('Failed fetching data from the Installation Store');
}

const authResult: AuthorizeResult = {};
authResult.userToken = queryResult.user.token;

if (queryResult.team !== undefined) {
if (queryResult.user) {
authResult.userToken = queryResult.user.token;
}

if (queryResult.team?.id) {
authResult.teamId = queryResult.team.id;
} else if (source.teamId !== undefined) {
} else if (source?.teamId) {
/**
* since queryResult is a org installation, it won't have team.id. If one was passed in via source,
* we should add it to the authResult
* Since queryResult is a org installation, it won't have team.id.
* If one was passed in via source, we should add it to the authResult.
*/
authResult.teamId = source.teamId;
}
Expand All @@ -162,20 +168,20 @@ export class InstallProvider {
authResult.enterpriseId = queryResult?.enterprise?.id || source?.enterpriseId;
}

if (queryResult.bot !== undefined) {
if (queryResult.bot) {
authResult.botToken = queryResult.bot.token;
authResult.botId = queryResult.bot.id;
authResult.botUserId = queryResult.bot.userId;

// Token Rotation Enabled (Bot Token)
if (queryResult.bot.refreshToken !== undefined) {
if (queryResult.bot.refreshToken) {
authResult.botRefreshToken = queryResult.bot.refreshToken;
authResult.botTokenExpiresAt = queryResult.bot.expiresAt; // utc, seconds
}
}

// Token Rotation Enabled (User Token)
if (queryResult.user.refreshToken !== undefined) {
if (queryResult.user?.refreshToken) {
authResult.userRefreshToken = queryResult.user.refreshToken;
authResult.userTokenExpiresAt = queryResult.user.expiresAt; // utc, seconds
}
Expand All @@ -186,7 +192,7 @@ export class InstallProvider {
* If the token has expired, or will expire within 2 hours, the token is refreshed and
* the `authResult` and `Installation` are updated with the new values.
*/
if (authResult.botRefreshToken !== undefined || authResult.userRefreshToken !== undefined) {
if (authResult.botRefreshToken || authResult.userRefreshToken) {
const currentUTCSec = Math.floor(Date.now() / 1000); // seconds
const tokensToRefresh: string[] = detectExpiredOrExpiringTokens(authResult, currentUTCSec);

Expand Down Expand Up @@ -271,7 +277,7 @@ export class InstallProvider {
public async generateInstallUrl(options: InstallURLOptions, stateVerification: boolean = true): Promise<string> {
const slackURL = new URL(this.authorizationUrl);

if (options.scopes === undefined) {
if (options.scopes === undefined || options.scopes === null) {
throw new GenerateInstallUrlError('You must provide a scope parameter when calling generateInstallUrl');
}

Expand Down Expand Up @@ -809,14 +815,16 @@ function detectExpiredOrExpiringTokens(authResult: AuthorizeResult, currentUTCSe
const tokensToRefresh: string[] = [];
const EXPIRY_WINDOW: number = 7200; // 2 hours

if (authResult.botRefreshToken !== undefined && authResult.botTokenExpiresAt !== undefined) {
if (authResult.botRefreshToken &&
(authResult.botTokenExpiresAt !== undefined && authResult.botTokenExpiresAt !== null)) {
const botTokenExpiresIn = authResult.botTokenExpiresAt - currentUTCSec;
if (botTokenExpiresIn <= EXPIRY_WINDOW) {
tokensToRefresh.push(authResult.botRefreshToken);
}
}

if (authResult.userRefreshToken !== undefined && authResult.userTokenExpiresAt !== undefined) {
if (authResult.userRefreshToken &&
(authResult.userTokenExpiresAt !== undefined && authResult.userTokenExpiresAt !== null)) {
const userTokenExpiresIn = authResult.userTokenExpiresAt - currentUTCSec;
if (userTokenExpiresIn <= EXPIRY_WINDOW) {
tokensToRefresh.push(authResult.userRefreshToken);
Expand Down

0 comments on commit 4ad3827

Please sign in to comment.