Skip to content
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

Option to disable signature verification #648

Closed
4 of 9 tasks
meetmangukiya opened this issue Sep 29, 2020 · 3 comments · Fixed by #1088 or #1424
Closed
4 of 9 tasks

Option to disable signature verification #648

meetmangukiya opened this issue Sep 29, 2020 · 3 comments · Fixed by #1088 or #1424
Assignees
Labels
enhancement M-T: A feature request for new functionality tests M-T: Testing work only
Milestone

Comments

@meetmangukiya
Copy link

Description

Option to disable signature verification. This will be particularly useful for running tests and should be used only for tests. Since signature verification in production is a security feature.

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Example usage:

import chaiHttp from 'chai-http'
import App from './app'
import chai from 'chai'
import sinon from 'sinon';
import sinonChai from 'sinon-chai';

chai.use(chaiHttp);
chai.use(sinonChai);

const dummyData = {};

describe('slack message test', () => {
  it('should say hey', async () => {
    const fake = sinon.fake.resolves({ ok: true, ...otherResponseProps });
    sinon.replace(App.client, 'postMessage', fake);
    await chai.request(App.receiver.app).post('/slack/events').send({...dummyData});
    expect(fake).to.have.been.calledWith(...);
  })
})

This currently doesn't work since the verification middleware wouldn't call the next middleware since the verification would fail. And I wasn't able to replace the verifySignatureAndParseRawBody with a fake. I will give some more tries. But maybe it is just easier to allow disabling this middleware from app options?

@gitwave gitwave bot added the untriaged label Sep 29, 2020
@meetmangukiya
Copy link
Author

meetmangukiya commented Sep 29, 2020

Wasn't able to replace it. Ended up computing the signature and sending that header.

Need to add headers x-slack-request-timestamp(unix timestamp in seconds, should be within last 5 minutes for successful verification) and x-slack-signature. Signature can be computed as

const computeSignature = (
  version: string,
  data: string,
  timestamp: number,
  signingSecret: string
) => {
  const hmac = crypto.createHmac('sha256', signingSecret);
  hmac.update(`${version}:${timestamp}:${data}`);
  return `${version}=${hmac.digest('hex')}`;
};

Current version is v0 it seems. data has to be the request body. JSON data, but stringified.

@seratch seratch added enhancement M-T: A feature request for new functionality tests M-T: Testing work only and removed untriaged labels Sep 30, 2020
@seratch
Copy link
Member

seratch commented Sep 30, 2020

Wasn't able to replace it. Ended up computing the signature and sending that header.

Yes, currently it's not possible to replace/disable the logic from ExpressReceiver. The only way to write valid tests is, as you did, to have some code populating x-slack-signature header in requests.

Also, I agree that this project can improve the module structure to support better and easier ways to write unit tests.

@seratch
Copy link
Member

seratch commented Mar 5, 2021

Recently, we've implemented the feature to turn the verification off in the Java SDK.

We can consider adding something similar in Bolt for JS too.

@seratch seratch added this to the 3.5.0 milestone Mar 23, 2021
@seratch seratch modified the milestones: 3.5.0, 3.6.0 Jul 14, 2021
@srajiang srajiang modified the milestones: 3.6.0, 3.7.0 Aug 19, 2021
seratch added a commit to seratch/bolt-js that referenced this issue Aug 26, 2021
@seratch seratch self-assigned this Aug 26, 2021
seratch added a commit to seratch/bolt-js that referenced this issue Aug 28, 2021
seratch added a commit to seratch/bolt-js that referenced this issue Aug 28, 2021
seratch added a commit that referenced this issue Aug 28, 2021
* Fix #648 Option to disable signature verification
* Rename requestVerification to signatureVerification
* Fix lint errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality tests M-T: Testing work only
Projects
None yet
3 participants