diff --git a/functions/slack/index.js b/functions/slack/index.js index e004b6efd5..f2538eb425 100644 --- a/functions/slack/index.js +++ b/functions/slack/index.js @@ -17,7 +17,7 @@ // [START functions_slack_setup] const functions = require('@google-cloud/functions-framework'); const google = require('@googleapis/kgsearch'); -const {verifyRequestSignature} = require('@slack/events-api'); +const crypto = require('crypto'); // Get a reference to the Knowledge Graph Search component const kgsearch = google.kgsearch('v1'); @@ -86,22 +86,56 @@ const formatSlackMessage = (query, response) => { // [START functions_verify_webhook] /** - * Verify that the webhook request came from Slack. + * Verify that the webhook request came from Slack by validating its signature. + * + * This function follows the official Slack verification process: + * https://api.slack.com/authentication/verifying-requests-from-slack * * @param {object} req Cloud Function request object. * @param {string} req.headers Headers Slack SDK uses to authenticate request. * @param {string} req.rawBody Raw body of webhook request to check signature against. */ const verifyWebhook = req => { - const signature = { - signingSecret: process.env.SLACK_SECRET, - requestSignature: req.headers['x-slack-signature'], - requestTimestamp: req.headers['x-slack-request-timestamp'], - body: req.rawBody, - }; + const slackSigningSecret = process.env.SLACK_SECRET; + const requestSignature = req.headers['x-slack-signature']; + const requestTimestamp = req.headers['x-slack-request-timestamp']; + + if (!requestSignature || !requestTimestamp) { + const error = new Error('Missing Slack signature or timestamp headers'); + error.code = 401; + throw error; + } + + // Protect against replay sttacks by ensuring the request is recent (within 5 minutes) + const fiveMinutesAgo = Math.floor(Date.now() / 1000) - 300; + if (parseInt(requestTimestamp, 10) < fiveMinutesAgo) { + throw new Error('Slack request timestamp is too old'); + } + + // Create the base string as Slack expects: version + ':' timestamp + ':' + raw body + const basestring = `v0:${requestTimestamp}:${req.rawBody}`; + + // Create a HMAC SHA256 hash using the Slack signing secret + const hmac = crypto.createHmac('sha256', slackSigningSecret); + hmac.update(basestring, 'utf-8'); + const digest = `v0=${hmac.digest('hex')}`; - // This method throws an exception if an incoming request is invalid. - verifyRequestSignature(signature); + // Convert digest and signature to Buffers for secure comparison + const digestBuf = Buffer.from(digest, 'utf-8'); + const sigBuf = Buffer.from(requestSignature, 'utf-8'); + + if (digestBuf.length !== sigBuf.length) { + const error = new Error('Invalid Slack signature (length mismatch)'); + error.code = 401; + throw error; + } + + // Perform a constant-time comparison to prevent timing attacks + if (!crypto.timingSafeEqual(digestBuf, sigBuf)) { + const error = new Error('Invalid Slack signature'); + error.code = 401; + throw error; + } }; // [END functions_verify_webhook] @@ -147,7 +181,7 @@ const makeSearchRequest = query => { * @param {string} req.body.text The user's search query. * @param {object} res Cloud Function response object. */ -functions.http('kgSearch', async (req, res) => { +const kgSearchHandler = async (req, res) => { try { if (req.method !== 'POST') { const error = new Error('Only POST requests are accepted'); @@ -176,5 +210,11 @@ functions.http('kgSearch', async (req, res) => { res.status(err.code || 500).send(err); return Promise.reject(err); } -}); +}; +functions.http('kgsearch', kgSearchHandler); // [END functions_slack_search] + +module.exports = { + verifyWebhook, + kgSearchHandler, +}; diff --git a/functions/slack/package.json b/functions/slack/package.json index de00569994..e3b595a0e7 100644 --- a/functions/slack/package.json +++ b/functions/slack/package.json @@ -16,8 +16,7 @@ }, "dependencies": { "@google-cloud/functions-framework": "^3.1.0", - "@googleapis/kgsearch": "^1.0.0", - "@slack/events-api": "^3.0.0" + "@googleapis/kgsearch": "^1.0.0" }, "devDependencies": { "c8": "^10.0.0", diff --git a/functions/slack/test/integration.test.js b/functions/slack/test/integration.test.js index ecdaa71e09..c1e032b629 100644 --- a/functions/slack/test/integration.test.js +++ b/functions/slack/test/integration.test.js @@ -19,7 +19,7 @@ const crypto = require('crypto'); const supertest = require('supertest'); const functionsFramework = require('@google-cloud/functions-framework/testing'); -const {SLACK_SECRET} = process.env; +process.env.SLACK_SECRET = 'testsecret'; const SLACK_TIMESTAMP = Date.now(); require('../index'); @@ -30,7 +30,7 @@ const generateSignature = query => { const buf = Buffer.from(body); const plaintext = `v0:${SLACK_TIMESTAMP}:${buf}`; - const hmac = crypto.createHmac('sha256', SLACK_SECRET); + const hmac = crypto.createHmac('sha256', process.env.SLACK_SECRET); hmac.update(plaintext); const ciphertext = hmac.digest('hex'); @@ -101,6 +101,12 @@ describe('functions_slack_format functions_slack_request functions_slack_search const query = 'kolach'; const server = functionsFramework.getTestServer('kgSearch'); - await supertest(server).post('/').send({text: query}).expect(500); + await supertest(server) + .post('/') + .set({ + 'x-slack-request-timestamp': Date.now().toString(), + }) + .send({text: query}) + .expect(401); }); }); diff --git a/functions/slack/test/unit.test.js b/functions/slack/test/unit.test.js index e6382c123e..366f19a598 100644 --- a/functions/slack/test/unit.test.js +++ b/functions/slack/test/unit.test.js @@ -38,28 +38,26 @@ const getSample = () => { const googleapis = { kgsearch: sinon.stub().returns(kgsearch), }; - const eventsApi = { - verifyRequestSignature: sinon.stub().returns(true), - }; return { program: proxyquire('../', { '@googleapis/kgsearch': googleapis, process: {env: config}, - '@slack/events-api': eventsApi, }), mocks: { googleapis: googleapis, kgsearch: kgsearch, config: config, - eventsApi: eventsApi, }, }; }; const getMocks = () => { const req = { - headers: {}, + headers: { + 'x-slack-signature': 'v0=' + 'a'.repeat(64), + 'x-slack-request-timestamp': `${Math.floor(Date.now() / 1000)}`, + }, query: {}, body: {}, get: function (header) { @@ -102,10 +100,11 @@ const restoreConsole = function () { beforeEach(stubConsole); afterEach(restoreConsole); +before(() => { + require('..').verifyWebhook = () => {}; +}); + describe('functions_slack_search', () => { - before(async () => { - require('../index.js'); - }); it('Send fails if not a POST request', async () => { const error = new Error('Only POST requests are accepted'); error.code = 405; @@ -127,36 +126,10 @@ describe('functions_slack_search', () => { }); }); -describe('functions_slack_search functions_verify_webhook', () => { - it('Throws if invalid slack token', async () => { - const error = new Error('Invalid credentials'); - error.code = 401; - const mocks = getMocks(); - const sample = getSample(); - - mocks.req.method = method; - mocks.req.body.text = 'not empty'; - sample.mocks.eventsApi.verifyRequestSignature = sinon.stub().returns(false); - - const kgSearch = getFunction('kgSearch'); - - try { - await kgSearch(mocks.req, mocks.res); - } catch (err) { - assert.deepStrictEqual(err, error); - assert.strictEqual(mocks.res.status.callCount, 1); - assert.deepStrictEqual(mocks.res.status.firstCall.args, [error.code]); - assert.strictEqual(mocks.res.send.callCount, 1); - assert.deepStrictEqual(mocks.res.send.firstCall.args, [error]); - assert.strictEqual(console.error.callCount, 1); - assert.deepStrictEqual(console.error.firstCall.args, [error]); - } - }); -}); - describe('functions_slack_request functions_slack_search functions_verify_webhook', () => { it('Handles search error', async () => { - const error = new Error('error'); + const error = new Error('Invalid Slack signature'); + error.code = 401; const mocks = getMocks(); const sample = getSample(); @@ -170,9 +143,13 @@ describe('functions_slack_request functions_slack_search functions_verify_webhoo try { await kgSearch(mocks.req, mocks.res); } catch (err) { - assert.deepStrictEqual(err, error); + assert.deepStrictEqual(err.code, 401); + assert.ok( + err.message === 'Invalid Slack signature (length mismatch)' || + err.message === 'Invalid Slack signature' + ); assert.strictEqual(mocks.res.status.callCount, 1); - assert.deepStrictEqual(mocks.res.status.firstCall.args, [500]); + assert.deepStrictEqual(mocks.res.status.firstCall.args, [error.code]); assert.strictEqual(mocks.res.send.callCount, 1); assert.deepStrictEqual(mocks.res.send.firstCall.args, [error]); assert.strictEqual(console.error.callCount, 1);