Skip to content

Commit

Permalink
updated oauth integration based on feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@stevengill
stevengill committed Apr 30, 2020
1 parent 83ee5b2 commit b39a6a3
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 53 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
},
"dependencies": {
"@slack/logger": "^2.0.0",
"@slack/oauth": "file:../node-slack-sdk/packages/oauth",
"@slack/oauth": "^1.0.0",
"@slack/types": "^1.5.0",
"@slack/web-api": "^5.8.0",
"@types/express": "^4.16.1",
Expand Down
39 changes: 18 additions & 21 deletions src/App.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,10 @@ export interface AppOptions {
processBeforeResponse?: ExpressReceiverOptions['processBeforeResponse'];
clientId?: ExpressReceiverOptions['clientId'];
clientSecret?: ExpressReceiverOptions['clientSecret'];
stateStore?: ExpressReceiverOptions['stateStore']; // default ClearStateStore
stateSecret?: ExpressReceiverOptions['stateSecret']; // required when using default stateStore
installationStore?: ExpressReceiverOptions['installationStore']; // default MemoryInstallationStore
authVersion?: ExpressReceiverOptions['authVersion']; // default 'v2'
scopes?: ExpressReceiverOptions['scopes'];
metadata?: ExpressReceiverOptions['metadata'];
installerOptions?: ExpressReceiverOptions['installerOptions'];
agent?: Agent;
clientTls?: Pick<SecureContextOptions, 'pfx' | 'key' | 'passphrase' | 'cert' | 'ca'>;
convoStore?: ConversationStore | false;
Expand Down Expand Up @@ -196,12 +194,10 @@ export default class App {
processBeforeResponse = false,
clientId = undefined,
clientSecret = undefined,
stateStore = undefined,
stateSecret = undefined,
installationStore = undefined,
authVersion = 'v2',
scopes = undefined,
metadata = undefined,
installerOptions = undefined,
}: AppOptions = {}) {

if (typeof logger === 'undefined') {
Expand Down Expand Up @@ -255,46 +251,47 @@ export default class App {
processBeforeResponse,
clientId,
clientSecret,
stateStore,
stateSecret,
installationStore,
authVersion,
metadata,
installerOptions,
scopes,
logger: this.logger,
});
}
}

let usingBuiltinOauth = undefined;
let usingBuiltinOauth = false;
if (
clientId !== undefined
&& clientSecret !== undefined
&& (stateSecret !== undefined || stateStore !== undefined)
&& (stateSecret !== undefined || (installerOptions !== undefined && installerOptions.stateStore !== undefined))
&& this.receiver instanceof ExpressReceiver
) {
usingBuiltinOauth = true;
}

if (token !== undefined) {
if (authorize !== undefined || usingBuiltinOauth !== undefined) {
if (authorize !== undefined || usingBuiltinOauth) {
throw new AppInitializationError(
`token as well as authorize options or installer options were provided. ${tokenUsage}`,
`token as well as authorize options or oauth installer options were provided. ${tokenUsage}`,
);
}
this.authorize = singleTeamAuthorization(this.client, { botId, botUserId, botToken: token });
} else if (authorize === undefined && usingBuiltinOauth === undefined) {
} else if (authorize === undefined && !usingBuiltinOauth) {
throw new AppInitializationError(
`No token, no authorize options, and no installer options provided. ${tokenUsage}`,
`No token, no authorize options, and no oauth installer options provided. ${tokenUsage}`,
);
} else if (authorize !== undefined && usingBuiltinOauth !== undefined) {
} else if (authorize !== undefined && usingBuiltinOauth) {
throw new AppInitializationError(
`Both authorize options and installer options provided. ${tokenUsage}`,
`Both authorize options and oauth installer options provided. ${tokenUsage}`,
);
} else if (authorize === undefined && usingBuiltinOauth !== undefined) {
this.authorize = (this.receiver as ExpressReceiver).oauthAuthorize as Authorize;
} else if (authorize !== undefined && usingBuiltinOauth === undefined) {
} else if (authorize === undefined && usingBuiltinOauth) {
this.authorize = (this.receiver as ExpressReceiver).installer?.authorize as Authorize;
} else if (authorize !== undefined && !usingBuiltinOauth) {
this.authorize = authorize;
} else {
this.logger.error('Never should have reached this point, please report to the team');
assertNever();
}

// Conditionally use a global middleware that ignores events (including messages) that are sent from this app
Expand Down Expand Up @@ -692,7 +689,7 @@ export default class App {
}

const tokenUsage = 'Apps used in one workspace should be initialized with a token. Apps used in many workspaces ' +
'should be initialized with a authorize.';
'should be initialized with oauth installer or authorize.';

const validViewTypes = ['view_closed', 'view_submission'];

Expand Down
70 changes: 40 additions & 30 deletions src/ExpressReceiver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import tsscmp from 'tsscmp';
import App from './App';
import { ReceiverAuthenticityError, ReceiverMultipleAckError } from './errors';
import { Logger, ConsoleLogger } from '@slack/logger';
import { InstallProvider, StateStore, InstallationStore, OAuthInterface, CallbackOptions } from '@slack/oauth';
import { InstallProvider, StateStore, InstallationStore, CallbackOptions } from '@slack/oauth';

// TODO: we throw away the key names for endpoints, so maybe we should use this interface. is it better for migrations?
// if that's the reason, let's document that with a comment.
Expand All @@ -21,13 +21,20 @@ export interface ExpressReceiverOptions {
processBeforeResponse?: boolean;
clientId?: string;
clientSecret?: string;
stateStore?: StateStore; // default ClearStateStore
stateSecret?: string; // ClearStateStoreOptions['secret']; // required when using default stateStore
installationStore?: InstallationStore; // default MemoryInstallationStore
authVersion?: 'v1' | 'v2'; // default 'v2'
scopes?: string | string[];
installerOptions?: InstallerOptions;
}

// Additional Installer Options
interface InstallerOptions {
stateStore?: StateStore; // default ClearStateStore
authVersion?: 'v1' | 'v2'; // default 'v2'
metadata?: string;
oAuthCallbackOptions?: CallbackOptions;
installPath?: string;
redirectUriPath?: string;
callbackOptions?: CallbackOptions;
}

/**
Expand All @@ -43,7 +50,7 @@ export default class ExpressReceiver implements Receiver {
private logger: Logger;
private processBeforeResponse: boolean;
public router: Router;
public oauthAuthorize: OAuthInterface['authorize'] | undefined;
public installer: InstallProvider | undefined = undefined;

constructor({
signingSecret = '',
Expand All @@ -52,18 +59,16 @@ export default class ExpressReceiver implements Receiver {
processBeforeResponse = false,
clientId = undefined,
clientSecret = undefined,
stateStore = undefined,
stateSecret = undefined,
installationStore = undefined,
authVersion = 'v2',
metadata = undefined,
scopes = undefined,
installerOptions = {},
}: ExpressReceiverOptions) {
this.app = express();
// TODO: what about starting an https server instead of http? what about other options to create the server?
this.server = createServer(this.app);

const expressMiddleware = [
const expressMiddleware: RequestHandler[] = [
verifySignatureAndParseRawBody(logger, signingSecret),
respondToSslCheck,
respondToUrlVerification,
Expand All @@ -77,45 +82,50 @@ export default class ExpressReceiver implements Receiver {
for (const endpoint of endpointList) {
this.router.post(endpoint, ...expressMiddleware);
}
this.app.use(this.router);
this.oauthAuthorize = undefined;

let oauthInstaller: OAuthInterface | undefined = undefined;
if (
clientId !== undefined
&& clientSecret !== undefined
&& (stateSecret !== undefined || stateStore !== undefined)
&& (stateSecret !== undefined || installerOptions.stateStore !== undefined)
) {

oauthInstaller = new InstallProvider({
this.installer = new InstallProvider({
clientId,
clientSecret,
stateSecret,
stateStore,
installationStore,
authVersion,
stateStore: installerOptions.stateStore,
authVersion: installerOptions.authVersion!,
});
}

// Add OAuth routes to receiver
if (oauthInstaller !== undefined) {
this.app.use('/slack/install', (req, res) => {
return oauthInstaller!.handleCallback(req, res, {});
if (this.installer !== undefined) {
const redirectUriPath = installerOptions.redirectUriPath === undefined ?
'/slack/oauth_redirect' : installerOptions.redirectUriPath;
this.router.use(redirectUriPath, async (req, res) => {
await this.installer!.handleCallback(req, res, installerOptions.callbackOptions);
});
this.app.get('/slack/begin_auth', (_req, res, next) => {
// TODO, take in arguments or function for
oauthInstaller!.generateInstallUrl({
metadata,
scopes: scopes!,
}).then((url: string) => {

const installPath = installerOptions.installPath === undefined ?
'/slack/install' : installerOptions.installPath;
this.router.get(installPath, async (_req, res, next) => {
try {
const url = await this.installer!.generateInstallUrl({
metadata: installerOptions.metadata,
scopes: scopes!,
});
res.send(`<a href=${url}><img alt=""Add to Slack"" height="40" width="139"
src="https://platform.slack-edge.com/img/add_to_slack.png"
srcset="https://platform.slack-edge.com/img/add_to_slack.png 1x,
https://platform.slack-edge.com/img/add_to_slack@2x.png 2x" /></a>`);
}).catch(next);
src="https://platform.slack-edge.com/img/add_to_slack.png"
srcset="https://platform.slack-edge.com/img/add_to_slack.png 1x,
https://platform.slack-edge.com/img/add_to_slack@2x.png 2x" /></a>`);
} catch (error) {
next(error);
}
});
this.oauthAuthorize = oauthInstaller.authorize;
}

this.app.use(this.router);
}

private async requestHandler(req: Request, res: Response): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion src/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ export function getTypeAndConversation(body: any): { type?: IncomingEventType, c
/* istanbul ignore next */

/** Helper that should never be called, but is useful for exhaustiveness checking in conditional branches */
export function assertNever(x: never): never {
export function assertNever(x?: never): never {
throw new Error(`Unexpected object: ${x}`);
}

0 comments on commit b39a6a3

Please sign in to comment.