-
Notifications
You must be signed in to change notification settings - Fork 479
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
More fine-grained permissions for Fact Tables and SDK Connections #2503
Conversation
@@ -40,7 +43,8 @@ export const GLOBAL_PERMISSIONS = [ | |||
"manageTags", | |||
"manageApiKeys", | |||
"manageIntegrations", | |||
"manageWebhooks", | |||
"manageEventWebhooks", | |||
"manageSDKWebhooks", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this more, I think we should make a breaking change and grant manageSDKWebhooks
permission to engineers and experimenters. I think it was a bug to not do this originally and it will simplify the policies since we can just include this permission in the SDKConnectionsFullAccess
policy instead of making a separate SDKWebhook one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
So, just to clarify, we'll keep the change that removes manageWebhooks
in lieu of the two different manageEventWebhooks
and manageSDKWebhooks
.
And we'll update engineer and experimenter roles to include manageSDKWebhooks
, but we'll keep manageEventWebhooks
as an admin-only permission.
In these changes, I think it also makes sense to change the scoping of manageSDKWebhooks
so it's env scoped like manageSDKConnections
.
@@ -31,7 +34,8 @@ export const GLOBAL_PERMISSIONS = [ | |||
"manageTags", | |||
"manageApiKeys", | |||
"manageIntegrations", | |||
"manageWebhooks", | |||
"manageEventWebhooks", | |||
"manageSDKWebhooks", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to ENV_SCOPED_PERMISSIONS
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file in back-end is not being used. I deleted it in the other PR.
Your preview environment pr-2503-bttf has been deployed. Preview environment endpoints are available at: |
Features and Changes
This PR, currently based on 2499 introduces some finer grain permissions that will eventually be used to define the policies in 2495.
This includes net-new permissions
manageFactMetrics
,manageFactFilters
,manageSDKConnections
, and also replacesmanageWebhooks
withmanageEventWebhooks
andmanageSDKWebhooks
.This was done in a way where it shouldn't impact existing organizations at all.
Dependencies
Testing