-
Notifications
You must be signed in to change notification settings - Fork 399
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 #248 by adding a new option deferInitialization #1303
Merged
Merged
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9edef5b
Fix #248 by adding a new option deferInitialization
seratch 0a67221
Move private methods in code
seratch cd57645
Improve a lot
seratch e837905
Update more
seratch fa695ae
Fix
seratch 3b7b6aa
Fix examples
seratch f4d52c9
Update src/App.spec.ts
seratch ebb7da2
Merge branch 'main' into issue-248-auth-test-failure
seratch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,6 +105,7 @@ export interface AppOptions { | |
socketMode?: boolean; | ||
developerMode?: boolean; | ||
tokenVerificationEnabled?: boolean; | ||
deferInitialization?: boolean; | ||
extendedErrorHandler?: boolean; | ||
} | ||
|
||
|
@@ -241,6 +242,17 @@ export default class App { | |
|
||
private hasCustomErrorHandler: boolean; | ||
|
||
// used for the deferred initialization | ||
srajiang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private argToken?: string; | ||
|
||
// used for the deferred initialization | ||
srajiang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private argAuthorize?: Authorize; | ||
|
||
// used for the deferred initialization | ||
private argAuthorization?: Authorization; | ||
|
||
private tokenVerificationEnabled: boolean; | ||
|
||
public constructor({ | ||
signingSecret = undefined, | ||
endpoints = undefined, | ||
|
@@ -272,6 +284,7 @@ export default class App { | |
developerMode = false, | ||
tokenVerificationEnabled = true, | ||
extendedErrorHandler = false, | ||
deferInitialization = false, | ||
}: AppOptions = {}) { | ||
// this.logLevel = logLevel; | ||
|
||
|
@@ -369,101 +382,45 @@ export default class App { | |
} | ||
|
||
/* --------------------- Initialize receiver ---------------------- */ | ||
srajiang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (receiver !== undefined) { | ||
// Custom receiver supplied | ||
if (this.socketMode === true) { | ||
// socketMode = true should result in SocketModeReceiver being used as receiver | ||
// TODO: Add case for when socketMode = true and receiver = SocketModeReceiver | ||
// as this should not result in an error | ||
throw new AppInitializationError('You cannot supply a custom receiver when socketMode is set to true.'); | ||
} | ||
this.receiver = receiver; | ||
} else if (this.socketMode === true) { | ||
if (appToken === undefined) { | ||
throw new AppInitializationError('You must provide an appToken when socketMode is set to true. To generate an appToken see: https://api.slack.com/apis/connections/socket#token'); | ||
} | ||
this.logger.debug('Initializing SocketModeReceiver'); | ||
this.receiver = new SocketModeReceiver({ | ||
appToken, | ||
clientId, | ||
clientSecret, | ||
stateSecret, | ||
redirectUri, | ||
installationStore, | ||
scopes, | ||
logger, | ||
logLevel: this.logLevel, | ||
installerOptions: this.installerOptions, | ||
customRoutes, | ||
}); | ||
} else if (signatureVerification === true && signingSecret === undefined) { | ||
// Using default receiver HTTPReceiver, signature verification enabled, missing signingSecret | ||
throw new AppInitializationError( | ||
'signingSecret is required to initialize the default receiver. Set signingSecret or use a ' + | ||
'custom receiver. You can find your Signing Secret in your Slack App Settings.', | ||
); | ||
} else { | ||
this.logger.debug('Initializing HTTPReceiver'); | ||
this.receiver = new HTTPReceiver({ | ||
signingSecret: signingSecret || '', | ||
endpoints, | ||
port, | ||
customRoutes, | ||
processBeforeResponse, | ||
signatureVerification, | ||
clientId, | ||
clientSecret, | ||
stateSecret, | ||
redirectUri, | ||
installationStore, | ||
scopes, | ||
logger, | ||
logLevel: this.logLevel, | ||
installerOptions: this.installerOptions, | ||
}); | ||
} | ||
this.receiver = this.initReceiver( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is so nice! 💯 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, I like splitting up the initialization into discrete functions! |
||
receiver, | ||
signingSecret, | ||
endpoints, | ||
port, | ||
customRoutes, | ||
processBeforeResponse, | ||
signatureVerification, | ||
clientId, | ||
clientSecret, | ||
stateSecret, | ||
redirectUri, | ||
installationStore, | ||
scopes, | ||
appToken, | ||
logger, | ||
); | ||
|
||
/* ------------------------ Set authorize ----------------------------- */ | ||
let usingOauth = false; | ||
const httpReceiver = (this.receiver as HTTPReceiver); | ||
if ( | ||
httpReceiver.installer !== undefined && | ||
httpReceiver.installer.authorize !== undefined | ||
) { | ||
// This supports using the built in HTTPReceiver, declaring your own HTTPReceiver | ||
// and theoretically, doing a fully custom (non express) receiver that implements OAuth | ||
usingOauth = true; | ||
} | ||
this.tokenVerificationEnabled = tokenVerificationEnabled; | ||
let argAuthorization: Authorization | undefined; | ||
if (token !== undefined) { | ||
// If a token is supplied, the app is installed in at least one workspace | ||
if (usingOauth || authorize !== undefined) { | ||
throw new AppInitializationError( | ||
`You cannot provide a token along with either oauth installer options or authorize. ${tokenUsage}`, | ||
); | ||
} | ||
this.authorize = singleAuthorization( | ||
this.client, | ||
{ | ||
botId, | ||
botUserId, | ||
botToken: token, | ||
}, | ||
tokenVerificationEnabled, | ||
); | ||
} else if (authorize === undefined && !usingOauth) { | ||
throw new AppInitializationError( | ||
`${tokenUsage} \n\nSince you have not provided a token or authorize, you might be missing one or more required oauth installer options. See https://slack.dev/bolt-js/concepts#authenticating-oauth for these required fields.\n`, | ||
); | ||
} else if (authorize !== undefined && usingOauth) { | ||
throw new AppInitializationError(`You cannot provide both authorize and oauth installer options. ${tokenUsage}`); | ||
} else if (authorize === undefined && usingOauth) { | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
this.authorize = httpReceiver.installer!.authorize; | ||
} else if (authorize !== undefined && !usingOauth) { | ||
this.authorize = authorize; | ||
argAuthorization = { | ||
botId, | ||
botUserId, | ||
botToken: token, | ||
}; | ||
} | ||
if (deferInitialization) { | ||
this.argToken = token; | ||
this.argAuthorize = authorize; | ||
this.argAuthorization = argAuthorization; | ||
// You need to run `await app.init();` on your own | ||
} else { | ||
this.logger.error('Something has gone wrong. Please report this issue to the maintainers. https://github.com/slackapi/bolt-js/issues'); | ||
assertNever(); | ||
this.initInConstructor( | ||
token, | ||
authorize, | ||
argAuthorization, | ||
); | ||
} | ||
|
||
// Conditionally use a global middleware that ignores events (including messages) that are sent from this app | ||
|
@@ -483,6 +440,37 @@ export default class App { | |
this.receiver.init(this); | ||
} | ||
|
||
public async init(): Promise<void> { | ||
this.initAuthorizeIfNoTokenIsGiven( | ||
this.argToken, | ||
this.argAuthorize, | ||
); | ||
if (this.authorize !== undefined) { | ||
return; | ||
} | ||
if (this.argToken !== undefined && this.argAuthorization !== undefined) { | ||
let authorization = this.argAuthorization; | ||
if (this.tokenVerificationEnabled) { | ||
const authTestResult = await this.client.auth.test({ token: this.argToken }); | ||
if (authTestResult.ok) { | ||
authorization = { | ||
botUserId: authTestResult.user_id as string, | ||
botId: authTestResult.bot_id as string, | ||
botToken: this.argToken, | ||
}; | ||
} | ||
} | ||
this.authorize = singleAuthorization( | ||
this.client, | ||
authorization, | ||
this.tokenVerificationEnabled, | ||
); | ||
} else { | ||
this.logger.error('Something has gone wrong. Please report this issue to the maintainers. https://github.com/slackapi/bolt-js/issues'); | ||
assertNever(); | ||
} | ||
} | ||
|
||
/** | ||
* Register a new middleware, processed in the order registered. | ||
* | ||
|
@@ -1040,6 +1028,145 @@ export default class App { | |
this.errorHandler({ error: asCodedError(error), ...rest }) : | ||
this.errorHandler(asCodedError(error)); | ||
} | ||
|
||
// --------------------- | ||
// Private methods for initialization | ||
// --------------------- | ||
|
||
private initReceiver( | ||
receiver?: Receiver, | ||
signingSecret?: HTTPReceiverOptions['signingSecret'], | ||
endpoints?: HTTPReceiverOptions['endpoints'], | ||
port?: HTTPReceiverOptions['port'], | ||
customRoutes?: HTTPReceiverOptions['customRoutes'], | ||
processBeforeResponse?: HTTPReceiverOptions['processBeforeResponse'], | ||
signatureVerification?: HTTPReceiverOptions['signatureVerification'], | ||
clientId?: HTTPReceiverOptions['clientId'], | ||
clientSecret?: HTTPReceiverOptions['clientSecret'], | ||
stateSecret?: HTTPReceiverOptions['stateSecret'], | ||
redirectUri?: HTTPReceiverOptions['redirectUri'], | ||
installationStore?: HTTPReceiverOptions['installationStore'], | ||
scopes?: HTTPReceiverOptions['scopes'], | ||
appToken?: string, | ||
logger?: Logger, | ||
): Receiver { | ||
if (receiver !== undefined) { | ||
// Custom receiver supplied | ||
if (this.socketMode === true) { | ||
// socketMode = true should result in SocketModeReceiver being used as receiver | ||
// TODO: Add case for when socketMode = true and receiver = SocketModeReceiver | ||
// as this should not result in an error | ||
throw new AppInitializationError('You cannot supply a custom receiver when socketMode is set to true.'); | ||
} | ||
return receiver; | ||
} | ||
if (this.socketMode === true) { | ||
if (appToken === undefined) { | ||
throw new AppInitializationError('You must provide an appToken when socketMode is set to true. To generate an appToken see: https://api.slack.com/apis/connections/socket#token'); | ||
} | ||
this.logger.debug('Initializing SocketModeReceiver'); | ||
return new SocketModeReceiver({ | ||
appToken, | ||
clientId, | ||
clientSecret, | ||
stateSecret, | ||
redirectUri, | ||
installationStore, | ||
scopes, | ||
logger, | ||
logLevel: this.logLevel, | ||
installerOptions: this.installerOptions, | ||
customRoutes, | ||
}); | ||
} | ||
if (signatureVerification === true && signingSecret === undefined) { | ||
// Using default receiver HTTPReceiver, signature verification enabled, missing signingSecret | ||
throw new AppInitializationError( | ||
'signingSecret is required to initialize the default receiver. Set signingSecret or use a ' + | ||
'custom receiver. You can find your Signing Secret in your Slack App Settings.', | ||
); | ||
} | ||
this.logger.debug('Initializing HTTPReceiver'); | ||
return new HTTPReceiver({ | ||
signingSecret: signingSecret || '', | ||
endpoints, | ||
port, | ||
customRoutes, | ||
processBeforeResponse, | ||
signatureVerification, | ||
clientId, | ||
clientSecret, | ||
stateSecret, | ||
redirectUri, | ||
installationStore, | ||
scopes, | ||
logger, | ||
logLevel: this.logLevel, | ||
installerOptions: this.installerOptions, | ||
}); | ||
} | ||
|
||
private initAuthorizeIfNoTokenIsGiven( | ||
filmaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
token?: string, | ||
authorize?: Authorize, | ||
): void { | ||
let usingOauth = false; | ||
const httpReceiver = (this.receiver as HTTPReceiver); | ||
if ( | ||
httpReceiver.installer !== undefined && | ||
httpReceiver.installer.authorize !== undefined | ||
) { | ||
// This supports using the built in HTTPReceiver, declaring your own HTTPReceiver | ||
// and theoretically, doing a fully custom (non express) receiver that implements OAuth | ||
usingOauth = true; | ||
} | ||
|
||
if (token !== undefined) { | ||
if (usingOauth || authorize !== undefined) { | ||
throw new AppInitializationError( | ||
`You cannot provide a token along with either oauth installer options or authorize. ${tokenUsage}`, | ||
); | ||
} | ||
return; | ||
} | ||
|
||
if (authorize === undefined && !usingOauth) { | ||
throw new AppInitializationError( | ||
`${tokenUsage} \n\nSince you have not provided a token or authorize, you might be missing one or more required oauth installer options. See https://slack.dev/bolt-js/concepts#authenticating-oauth for these required fields.\n`, | ||
); | ||
} else if (authorize !== undefined && usingOauth) { | ||
throw new AppInitializationError(`You cannot provide both authorize and oauth installer options. ${tokenUsage}`); | ||
} else if (authorize === undefined && usingOauth) { | ||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
this.authorize = httpReceiver.installer!.authorize; | ||
} else if (authorize !== undefined && !usingOauth) { | ||
this.authorize = authorize as Authorize<boolean>; | ||
} | ||
} | ||
|
||
private initInConstructor( | ||
token?: string, | ||
authorize?: Authorize, | ||
authorization?: Authorization, | ||
): void { | ||
this.initAuthorizeIfNoTokenIsGiven( | ||
token, | ||
authorize, | ||
); | ||
if (this.authorize !== undefined) { | ||
return; | ||
} | ||
if (token !== undefined && authorization !== undefined) { | ||
this.authorize = singleAuthorization( | ||
this.client, | ||
authorization, | ||
this.tokenVerificationEnabled, | ||
); | ||
} else { | ||
this.logger.error('Something has gone wrong. Please report this issue to the maintainers. https://github.com/slackapi/bolt-js/issues'); | ||
assertNever(); | ||
} | ||
} | ||
} | ||
|
||
function defaultErrorHandler(logger: Logger): ErrorHandler { | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This naming makes the most sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine