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

Default token_management_enabled:true for runOnSlack:false apps #90

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

srajiang
Copy link
Member

Summary

Removes tokenManagementEnabled top level property from the Manifest definition, and sets corresponding token_management_enabled property in the Manifest Schema as true for runOnSlack: false apps.

Samples with tokenManagementEnabled field should also be updated.

Requirements (place an x in each [ ])

@srajiang srajiang added the feature request New feature or request label Aug 29, 2022
@srajiang srajiang self-assigned this Aug 29, 2022
@srajiang srajiang requested a review from a team as a code owner August 29, 2022 18:42
@srajiang srajiang force-pushed the sjiang-update-token-management branch from d2cc1aa to 123bdd5 Compare August 29, 2022 18:42
@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Merging #90 (123bdd5) into main (74b731f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #90   +/-   ##
=======================================
  Coverage   96.20%   96.21%           
=======================================
  Files          43       43           
  Lines        1609     1610    +1     
  Branches       87       87           
=======================================
+ Hits         1548     1549    +1     
  Misses         59       59           
  Partials        2        2           
Impacted Files Coverage Δ
src/manifest/mod.ts 89.44% <100.00%> (+0.04%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@stevengill
Copy link
Member

Could remote apps one day decide they want to disable token management? Allowing slack to manage it for them? I would assume this "remote app with token management disabled" would only rely on JIT tokens, no bot token

Copy link
Contributor

@selfcontained selfcontained left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@srajiang
Copy link
Member Author

@stevengill - Not out of the realm of possibility, but it seems sufficiently far off that we can wait to see. The door isn't closed to re-introducing the tokenManagementEnabled flag after this change, but if we roll into beta letting Bolt devs set tokenManagementEnabled to false, I don't see how they could deploy their apps today, since no xoxb will be visible to them.

@srajiang srajiang merged commit 55a193a into main Aug 29, 2022
@srajiang srajiang deleted the sjiang-update-token-management branch August 29, 2022 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants