oauth: Add timeout for state verification

stevengill commented 4 years ago

Description

Let's add a timeout check in verifyStateParam to not allow stale states to be verified. Probably a 30 second timeout.

Question: Is this something that the developer should be able to override? Other than just providing their own stateStore implementation?

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

[x] enhancement (feature request)

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

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

Packages:

Select all that apply:

[x] `@slack/oauth

seratch commented 4 years ago

Question: Is this something that the developer should be able to override? Other than just providing their own stateStore implementation?

I think we should provide some ways to override it. Bolt for Java does it. https://github.com/slackapi/java-slack-sdk/blob/v1.0.6/bolt/src/main/java/com/slack/api/bolt/service/OAuthStateService.java#L66-L68

Also, I think 30 seconds for the default may be a bit short for some cases. Imagine the situation where an installer of a Slack app carefully reviews the permissions the app is going to acquire on the OAuth confirmation page. So, in Java SDK, I decided to go with 10 minutes. I think a bit shorter time should also work fine.

seratch commented 2 years ago

I just moved this issue to 2.x milestone.

@stevengill Do you think that we should still prioritize this? Or, do you have a different view on it now?

slackapi / node-slack-sdk