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

Add timeout for state #1007

Open
5 tasks done
stevengill opened this issue May 4, 2020 · 2 comments
Open
5 tasks done

Add timeout for state #1007

stevengill opened this issue May 4, 2020 · 2 comments
Labels
enhancement M-T: A feature request for new functionality pkg:oauth applies to `@slack/oauth-helper`
Milestone

Comments

@stevengill
Copy link
Member

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 [ ])

  • enhancement (feature request)

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.

Packages:

Select all that apply:

  • `@slack/oauth
@stevengill stevengill added enhancement M-T: A feature request for new functionality pkg:oauth applies to `@slack/oauth-helper` labels May 4, 2020
@seratch
Copy link
Member

seratch commented May 5, 2020

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 seratch added this to the oauth@2.1 milestone Mar 25, 2021
@seratch seratch modified the milestones: oauth@2.1, oauth@2.2 Jun 3, 2021
@seratch seratch modified the milestones: oauth@2.2, oauth@2.3 Jul 13, 2021
@seratch seratch modified the milestones: oauth@2.3, oauth@2.4 Sep 12, 2021
@seratch seratch modified the milestones: oauth@2.4, oauth@2.x Jan 12, 2022
@seratch
Copy link
Member

seratch commented Jan 12, 2022

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?

@seratch seratch modified the milestones: oauth@2.5.0, oauth@2.x Feb 15, 2022
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 pkg:oauth applies to `@slack/oauth-helper`
Projects
None yet
Development

No branches or pull requests

2 participants