Add automatic access token rotation for workspace apps#347
Merged
Conversation
Added token refresh method, automatic refresh on invalid_auth Added expires_at to requester object to store token TTL updated rtm_connect to check for token type split HTTP request helper into builder and submitter methods fixed token expiration ts, moved rtm_connect token type check into client Added tests
aoberoi
suggested changes
Sep 6, 2018
aoberoi
left a comment
aoberoi
left a comment
Contributor
There was a problem hiding this comment.
I didn't get a chance to look through the tests just yet. But I think there are enough comments here that the implementation might change significantly, so I didn't think it would be worth reviewing them just yet.
Removed unused token attribute Added enterprise_id to token updater (commented out)
Codecov Report
@@ Coverage Diff @@
## master #347 +/- ##
==========================================
+ Coverage 74.4% 76.38% +1.97%
==========================================
Files 10 10
Lines 379 415 +36
Branches 67 74 +7
==========================================
+ Hits 282 317 +35
+ Misses 88 87 -1
- Partials 9 11 +2
Continue to review full report at Codecov.
|
jeffomatic
reviewed
Sep 6, 2018
aoberoi
suggested changes
Sep 9, 2018
aoberoi
suggested changes
Sep 10, 2018
Contributor
|
my review from the last 2 days is complete and up to date with all the latest code changes. the only unresolved issue from the previous reviews is this one: #347 (comment) |
|
|
aoberoi
suggested changes
Sep 11, 2018
| self.reconnect_count = 0 | ||
|
|
||
| reply = self.api_requester.do(self.token, connect_method, timeout=timeout, post_data=kwargs) | ||
| reply = self.api_requester.do(self.token, connect_method, kwargs) |
Contributor
There was a problem hiding this comment.
please put the timeout argument back in and name the post_data argument.
aoberoi
approved these changes
Sep 12, 2018
lavabyrd
pushed a commit
that referenced
this pull request
Apr 25, 2019
c-goosen
pushed a commit
to c-goosen/python-slackclient
that referenced
this pull request
Jun 18, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
With workspace apps comes the ability to use short-lived, expiring, tokens. This patch adds the ability for the client to automatically refresh access tokens after expiration and call your app's refresh callback to update said tokens in your app's datastore.
More info: https://api.slack.com/changelog/2018-08-workspace-token-rotation
Docs are still in progress.
Requirements (place an
xin each[ ])