Skip to content

feat(scopes): Add Distribution permission UI for integration tokens #102298

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: no/add-project-distribution-permission
Choose a base branch
from
+32 −1
Draft

feat(scopes): Add Distribution permission UI for integration tokens #102298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions static/app/constants/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const API_ACCESS_SCOPES = [
'org:read',
'org:write',
'project:admin',
'project:distribution',
'project:read',
'project:releases',
'project:write',
Expand All @@ -68,6 +69,7 @@ export const ALLOWED_SCOPES = [
'org:superuser', // not an assignable API access scope
'org:write',
'project:admin',
'project:distribution',
'project:read',
'project:releases',
'project:write',
Expand Down Expand Up @@ -165,6 +167,14 @@ export const SENTRY_APP_PERMISSIONS: PermissionObj[] = [
admin: {label: 'Admin', scopes: ['project:releases']},
},
},
{
resource: 'Distribution',
help: 'Distribution metadata for releases',
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Maybe:

"Pre-release app distribution for trusted testers."

or something along those lines.

choices: {
'no-access': {label: 'No Access', scopes: []},
read: {label: 'Read', scopes: ['project:distribution']},
},
},
{
resource: 'Event',
label: 'Issue & Event',
Expand Down
1 change: 1 addition & 0 deletions static/app/types/integrations.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type Permissions = {
Release: PermissionValue;
Team: PermissionValue;
Alerts?: PermissionValue;
Distribution?: PermissionValue;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

How come only Alerts and Distribution are optional (with the ?).


export type PermissionResource = keyof Permissions;
Expand Down
11 changes: 11 additions & 0 deletions static/app/utils/consolidatedScopes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const HUMAN_RESOURCE_NAMES = {
project: 'Project',
team: 'Team',
release: 'Release',
distribution: 'Distribution',
event: 'Event',
org: 'Organization',
member: 'Member',
Expand All @@ -25,13 +26,15 @@ const DEFAULT_RESOURCE_PERMISSIONS: Permissions = {
Project: 'no-access',
Team: 'no-access',
Release: 'no-access',
Distribution: 'no-access',
Event: 'no-access',
Organization: 'no-access',
Member: 'no-access',
Alerts: 'no-access',
};

const PROJECT_RELEASES = 'project:releases';
const PROJECT_DISTRIBUTION = 'project:distribution';
const ORG_INTEGRATIONS = 'org:integrations';

type PermissionLevelResources = {
Expand Down Expand Up @@ -98,6 +101,14 @@ function toResourcePermissions(scopes: string[]): Permissions {
filteredScopes = scopes.filter((scope: string) => scope !== PROJECT_RELEASES); // remove project:releases
}

// The scope for distribution is `project:distribution`, but instead of displaying
// it as a permission of Project, we want to separate it out into its own
// row for Distribution.
if (scopes.includes(PROJECT_DISTRIBUTION)) {
permissions.Distribution = 'read';
filteredScopes = scopes.filter((scope: string) => scope !== PROJECT_DISTRIBUTION); // remove project:distribution
}

// We have a special case with the org:integrations scope. This scope is
// added when selecting org:admin for hierarchy, but the reverse is not true.
// It doesn't indicate any specific org permission, so we can remove it
Expand Down
1 change: 1 addition & 0 deletions static/app/views/settings/account/apiNewToken.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export default function ApiNewToken() {
setPermissions(p);
setPreview(getPreview());
}}
hiddenPermissions={['Distribution']}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if this is a good idea or if there is a better way of doing this.
We want to filter the distribution permission from the personal tokens.

Copy link
Contributor

Choose a reason for hiding this comment

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

One alternative would be to change this component to take PermissionObj[] as another argument (maybe 'displayedPermissionsor something). People who want to show all the boxes passSENTRY_APP_PERMISSIONS(which could also be the default argument) and people who want to show only some can pass:SENTRY_APP_PERMISSIONS.filter(o => o.resource !== "Distribution")` or whatever they like.

I think doing it positively (e.g. show acd) rather than negatively (e.g. show everything except b) is a bit more of a clear API.

/>
</PanelBody>
<TextareaField
Expand Down
10 changes: 9 additions & 1 deletion static/app/views/settings/organizationDeveloperSettings/permissionSelection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ type Props = {
appPublished: boolean;
onChange: (permissions: Permissions) => void;
permissions: Permissions;
/**
* Optional list of resources to hide from the permission selection.
* Useful for limiting permissions available to personal tokens vs integration tokens.
*/
hiddenPermissions?: PermissionResource[];
};

type State = {
Expand Down Expand Up @@ -132,10 +137,13 @@ export default class PermissionSelection extends Component<Props, State> {

render() {
const {permissions} = this.state;
const {hiddenPermissions = []} = this.props;

return (
<Fragment>
{SENTRY_APP_PERMISSIONS.map(config => {
{SENTRY_APP_PERMISSIONS.filter(
config => !hiddenPermissions.includes(config.resource)
).map(config => {
const options = Object.entries(config.choices).map(([value, {label}]) => ({
value,
label,
Expand Down
Loading