Add stateVerification option to documentation #1338
Conversation
srajiang
requested review from
seratch and
stevengill
and removed request for
seratch
docs/_packages/oauth.md
Outdated
|
|
||
| By default, this package handles generating and verifying a `state` parameter during OAuth installation. This added measure helps to mitigate the risk of [Cross-Site Request Forgery](https://datatracker.ietf.org/doc/html/rfc6749#section-10.12) and is strongly recommended. | ||
|
|
||
| In specific installation scenarios, such as when an Org-Wide app is installed from an admin page, state verification cannot be completed because a `state` parameter isn't provided. In this case, you can disable `state` verification via setting the `InstallProvider#stateVerification` option to `false`. In this case, the installer will no longer require that `state` be present to proceed with installation. If `state` is provided in the request url parameters, it will still be verified. |
There was a problem hiding this comment.
A few comments:
- How about being more specific here> In specific installation scenarios -> In specific installation scenarios with Enterprise Grid organizations?
In this case,is repeated in two consecutive sentences. Can we change the transition words to something else?- I think that we don't need to do the verification in this case because it's disabled> If
stateis provided in the request url parameters, it will still be verified.
There was a problem hiding this comment.
@seratch There's also another scenario, the Add to Slack button on the App config page also doesn't supply state.
I'm thinking it's possible for an app to need to be installed from theslack/install page and from admin page / Add to Slack app configuration button. In that case a state param could sometimes be supplied and sometimes not. If an app is providing a state, wouldn't it be more secure to verify it?
There was a problem hiding this comment.
@srajiang For the scenario, our recommendation would be using Direct Install URL. As you know, we've recently added the support for it in bolt-js (it's not yet released though): slackapi/bolt-js#977 With this way, you can use /slack/install for Add to Slack in App Directory.
docs/_packages/oauth.md
Outdated
| const installer = new InstallProvider({ | ||
| clientId: process.env.SLACK_CLIENT_ID, | ||
| clientSecret: process.env.SLACK_CLIENT_SECRET, | ||
| stateSecret: 'my-state-secret', |
There was a problem hiding this comment.
Should developers still set stateSecret here?
In the above comment, you mentioned that even with stateVerification: false, state parameter can be generated. But I don't think that it's easy-to-understand behavior. If the flag is false, we can skip state parameter generation.
There was a problem hiding this comment.
Great point! With the latest merged changes to the oauth package, state store initialization is skipped when state verification is disabled (so implicitly state parameter generation and verification are both skipped as well). This also means that we can support removing the hard requirement for developers to set stateSecret in these specific cases. Hopefully that presents a more clear developer experience.
const installer = new InstallProvider({
clientId: process.env.SLACK_CLIENT_ID,
clientSecret: process.env.SLACK_CLIENT_SECRET,
stateVerification: false,
stateSecret: 'mysecret' // no longer required when stateVerification is false,
});
srajiang
changed the title
Summary
Adds documentation for stateVerification option. Re: node-slack-sdk/pull/1329.
Requirements (place an
xin each[ ])