diff --git a/functions/slack/index.js b/functions/slack/index.js index e004b6efd5..673d68a547 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'); @@ -93,15 +93,49 @@ const formatSlackMessage = (query, response) => { * @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 signingSecret = process.env.SLACK_SECRET; + const requestSignature = req.headers['x-slack-signature']; + const requestTimestamp = req.headers['x-slack-request-timestamp']; + const requestBody = req.rawBody; + + if (!requestSignature || !requestTimestamp) { + const err = new Error('Missing Slack validation headers.'); + err.code = 400; + throw err; + } + + if (!signingSecret) { + const err = new Error( + 'Server configuration error: SLACK_SECRET is missing.' + ); + err.code = 500; + throw err; + } + + // Prevent replay attacks by verifying the timestamp is recent + const now = Math.floor(Date.now() / 1000); + if (Math.abs(now - Number(requestTimestamp)) > 60 * 5) { + const err = new Error('Slack request timestamp is too old.'); + err.code = 401; + throw err; + } + + const hmac = crypto.createHmac('sha256', signingSecret); + hmac.update('v0:' + requestTimestamp + ':', 'utf8'); + hmac.update(requestBody || ''); + const expectedSignature = 'v0=' + hmac.digest('hex'); + + const sigBuffer = Buffer.from(requestSignature, 'utf8'); + const expBuffer = Buffer.from(expectedSignature, 'utf8'); - // This method throws an exception if an incoming request is invalid. - verifyRequestSignature(signature); + if ( + sigBuffer.length !== expBuffer.length || + !crypto.timingSafeEqual(sigBuffer, expBuffer) + ) { + const err = new Error('Invalid Slack signature.'); + err.code = 401; + throw err; + } }; // [END functions_verify_webhook] diff --git a/functions/slack/package.json b/functions/slack/package.json index de00569994..d38714e8d9 100644 --- a/functions/slack/package.json +++ b/functions/slack/package.json @@ -16,12 +16,12 @@ }, "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", "mocha": "^10.0.0", + "nock": "^13.5.6", "proxyquire": "^2.1.0", "sinon": "^18.0.0", "supertest": "^7.0.0" diff --git a/functions/slack/test/integration.test.js b/functions/slack/test/integration.test.js index ecdaa71e09..4e289c9c15 100644 --- a/functions/slack/test/integration.test.js +++ b/functions/slack/test/integration.test.js @@ -18,9 +18,11 @@ const assert = require('assert'); const crypto = require('crypto'); const supertest = require('supertest'); const functionsFramework = require('@google-cloud/functions-framework/testing'); +const nock = require('nock'); -const {SLACK_SECRET} = process.env; -const SLACK_TIMESTAMP = Date.now(); +process.env.SLACK_SECRET = process.env.SLACK_SECRET || 'test-slack-secret'; +const SLACK_SECRET = process.env.SLACK_SECRET; +const SLACK_TIMESTAMP = Math.floor(Date.now() / 1000).toString(); require('../index'); @@ -38,8 +40,33 @@ const generateSignature = query => { }; describe('functions_slack_format functions_slack_request functions_slack_search functions_verify_webhook', () => { + afterEach(() => { + nock.cleanAll(); + }); + it('returns search results', async () => { const query = 'kolach'; + + // Mock: Intercept the Google API request and return the expected data + nock('https://kgsearch.googleapis.com') + .get('/v1/entities:search') + .query(true) + .reply(200, { + itemListElement: [ + { + result: { + name: 'Kolach', + description: 'Pastry', + detailedDescription: { + articleBody: + 'A kolach is a pastry that holds a portion of fruit surrounded by puffy dough.', + url: 'http://domain.com/kolach', + }, + }, + }, + ], + }); + const server = functionsFramework.getTestServer('kgSearch'); const response = await supertest(server) .post('/') @@ -64,6 +91,13 @@ describe('functions_slack_format functions_slack_request functions_slack_search it('handles non-existent query', async () => { const query = 'g1bb3r1shhhhhhh'; + nock('https://kgsearch.googleapis.com') + .get('/v1/entities:search') + .query(true) + .reply(200, { + itemListElement: [], + }); + const server = functionsFramework.getTestServer('kgSearch'); const response = await supertest(server) .post('/') @@ -101,6 +135,6 @@ 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('/').send({text: query}).expect(400); }); }); diff --git a/functions/slack/test/unit.test.js b/functions/slack/test/unit.test.js index e6382c123e..90f7a6d402 100644 --- a/functions/slack/test/unit.test.js +++ b/functions/slack/test/unit.test.js @@ -17,17 +17,43 @@ const sinon = require('sinon'); const proxyquire = require('proxyquire').noCallThru(); const assert = require('assert'); +const crypto = require('crypto'); const {getFunction} = require('@google-cloud/functions-framework/testing'); const method = 'POST'; const query = 'giraffe'; -const SLACK_TOKEN = 'slack-token'; -const KG_API_KEY = 'kg-api-key'; +process.env.SLACK_SECRET = process.env.SLACK_SECRET || 'slack-token'; +process.env.KG_API_KEY = process.env.KG_API_KEY || 'test-kg-api-key'; + +const SLACK_SECRET = process.env.SLACK_SECRET; +const KG_API_KEY = process.env.KG_API_KEY; + +const signMockRequest = (req, bodyText, isValid = true) => { + req.body = {text: bodyText}; + req.rawBody = JSON.stringify(req.body); + const timestamp = Math.floor(Date.now() / 1000).toString(); + + let signature; + if (!isValid) { + signature = 'v0=invalid_signature_hash_for_testing'; + } else { + const baseString = `v0:${timestamp}:${req.rawBody}`; + signature = + 'v0=' + + crypto + .createHmac('sha256', SLACK_SECRET) + .update(baseString, 'utf8') + .digest('hex'); + } + + req.headers['x-slack-request-timestamp'] = timestamp; + req.headers['x-slack-signature'] = signature; +}; const getSample = () => { const config = { - SLACK_TOKEN: SLACK_TOKEN, + SLACK_SECRET: SLACK_SECRET, KG_API_KEY: KG_API_KEY, }; const kgsearch = { @@ -38,21 +64,16 @@ 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, }, }; }; @@ -129,14 +150,13 @@ describe('functions_slack_search', () => { describe('functions_slack_search functions_verify_webhook', () => { it('Throws if invalid slack token', async () => { - const error = new Error('Invalid credentials'); + const error = new Error('Invalid Slack signature.'); error.code = 401; const mocks = getMocks(); - const sample = getSample(); + getSample(); mocks.req.method = method; - mocks.req.body.text = 'not empty'; - sample.mocks.eventsApi.verifyRequestSignature = sinon.stub().returns(false); + signMockRequest(mocks.req, 'not empty', false); const kgSearch = getFunction('kgSearch'); @@ -161,8 +181,7 @@ describe('functions_slack_request functions_slack_search functions_verify_webhoo const sample = getSample(); mocks.req.method = method; - mocks.req.body.token = SLACK_TOKEN; - mocks.req.body.text = query; + signMockRequest(mocks.req, query, true); sample.mocks.kgsearch.entities.search.yields(error); const kgSearch = getFunction('kgSearch'); @@ -187,8 +206,7 @@ describe('functions_slack_format functions_slack_request functions_slack_search const sample = getSample(); mocks.req.method = method; - mocks.req.body.token = SLACK_TOKEN; - mocks.req.body.text = query; + signMockRequest(mocks.req, query, true); sample.mocks.kgsearch.entities.search.yields(null, { data: {itemListElement: []}, }); @@ -215,8 +233,7 @@ describe('functions_slack_format functions_slack_request functions_slack_search const sample = getSample(); mocks.req.method = method; - mocks.req.body.token = SLACK_TOKEN; - mocks.req.body.text = query; + signMockRequest(mocks.req, query, true); sample.mocks.kgsearch.entities.search.yields(null, { data: { itemListElement: [ @@ -263,8 +280,7 @@ describe('functions_slack_format functions_slack_request functions_slack_search const sample = getSample(); mocks.req.method = method; - mocks.req.body.token = SLACK_TOKEN; - mocks.req.body.text = query; + signMockRequest(mocks.req, query, true); sample.mocks.kgsearch.entities.search.yields(null, { data: { itemListElement: [