Improve the built-in authorize for better support of user-scope only installations #576
Conversation
Codecov Report
@@ Coverage Diff @@
## main #576 +/- ##
==========================================
+ Coverage 91.33% 91.48% +0.15%
==========================================
Files 169 169
Lines 5665 5699 +34
==========================================
+ Hits 5174 5214 +40
+ Misses 491 485 -6
Continue to review full report at Codecov.
|
|
|
||
| if installation is not None: | ||
| if installation.user_id != user_id: | ||
| # If the user_token in the latest_installation is not for the user associated with this request, |
There was a problem hiding this comment.
I think if we can provide an example of when the situation described in this comment arises (i.e. via the situation described in #574) it may give helpful context to future maintainers reading through this code. I have just now freshly read up on the issue and PR and I am already confused by this rather complex use case! 😅
There was a problem hiding this comment.
Thanks. I don't think we should add a very long comment here but I will add a bit more details here before merging this.
| @@ -294,3 +307,28 @@ def _debug_log_for_not_found( | |||
| "No installation data found " | |||
| f"for enterprise_id: {enterprise_id} team_id: {team_id}" | |||
| ) | |||
|
|
|||
| async def _rotate_and_save_tokens_if_necessary( | |||
seratch
force-pushed
the
improve-authorize
branch
from
f68586a
to
e939a56
Compare
While answering the question at #574, I found that there is room for improvement in the
InstallationStoreAuthorizemodule. This pull request improves the internals to cover the following situation:/slack/install/slack/user_install/slack/user_installIn this scenario, the
authorizefunction should return the bot token generated by admin user A plus the user B's user token inAuthorizeResult. However, the current implementation returns only user B's user token.This pull request resolves the issue, plus improved the readability of the code by renaming local variables and adding more comments.
Category (place an
xin each of the[ ])slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements (place an
xin each[ ])Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.