Skip to content

feat(functions/slack): replace deprecated @slack/events-api with manual Slack signature verification #4082

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

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
62de3df
chore(deps): remove deprecated @slack/events-api package
hivanalejandro Apr 29, 2025
74c8f3e
feat(security): add manual Slack request signature verification
hivanalejandro Apr 29, 2025
e2d1b2b
docs(security): add detailed comments to Slack request verification f…
hivanalejandro Apr 29, 2025
fbf48db
test(webhook): remove deprecated events-api mocks and update signatur…
hivanalejandro Apr 29, 2025
b7dd18d
test(webhook): fix HMAC verification setup and remove env destructuri…
hivanalejandro Apr 29, 2025
7a1689c
test(webhook): add Slack signature and timestamp headers in unit test…
hivanalejandro Apr 29, 2025
4a417ad
fix(webhook): avoid timingSafeEqual range error by checking buffer le…
hivanalejandro Apr 30, 2025
fc5e181
test(webhook): fix signature mocks and assert error details explicitl…
hivanalejandro Apr 30, 2025
316233a
fix(webhook): align error messages and status codes in tests and vali…
hivanalejandro Apr 30, 2025
22aaf29
test(unit): mock verifyWebhook and fix expected error codes
hivanalejandro Apr 30, 2025
6d85540
test(unit): globally mock verifyWebhook to bypass signature validatio…
hivanalejandro Apr 30, 2025
c91fc2c
test(unit): properly mock verifyWebhook using proxysquire to bypass s…
hivanalejandro Apr 30, 2025
92b3322
fix(tests): correct typo in proxyquire usage to fix ReferenceError in…
hivanalejandro Apr 30, 2025
d941c86
fix(test): mock verifyWebhook to bypass signature validation in unit …
hivanalejandro Apr 30, 2025
fb8bc95
fix(tests): mock crypto module in unit tests to bypass Slack signatur…
hivanalejandro Apr 30, 2025
7069532
fix(test): mock verifyWebhook for Slack tests without altering docume…
hivanalejandro May 6, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 52 additions & 12 deletions functions/slack/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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,
};
3 changes: 1 addition & 2 deletions functions/slack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 9 additions & 3 deletions functions/slack/test/integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');

Expand Down Expand Up @@ -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);
});
});
55 changes: 16 additions & 39 deletions functions/slack/test/unit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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();

Expand All @@ -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);
Expand Down
Loading