Skip to content

ref(integration-platform): limit app management to managers/owners#25458

Merged
MeredithAnya merged 3 commits into
masterfrom
integrationplatform/managers-and-owners-only
Apr 21, 2021
Merged

ref(integration-platform): limit app management to managers/owners#25458
MeredithAnya merged 3 commits into
masterfrom
integrationplatform/managers-and-owners-only

Conversation

@MeredithAnya

@MeredithAnya MeredithAnya commented Apr 20, 2021

Copy link
Copy Markdown
Member

Reduces scopes for creating and updating sentry apps (either public or internal) to Managers or Owners. Because sentry apps either have access to all projects or none, Members and Admins can create integrations that have project:read or event:read scopes and can access issue data that belongs to projects they are not apart of. This is fine for organizations with open membership (as any member can choose to join any team and therefore get access), however this is a security concern for closed membership organizations.

UI Tooltips:
Same tooltip for both the Public and Internal integration buttons, just with slightly different text:

Screen Shot 2021-04-20 at 11 25 48 AM

@MeredithAnya MeredithAnya requested review from a team and manuzope April 20, 2021 18:49

@scefali scefali left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm what about admins? And why are we making this change?

Comment thread static/app/views/settings/organizationDeveloperSettings/index.tsx
>
{t('New Internal Integration')}
</Button>
<Access organization={organization} access={['org:integrations']}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@MeredithAnya If we are excluding admins, I don't think this will work since admins have the org:integrations scope.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh yeah this needs to be org:write

@MeredithAnya

Copy link
Copy Markdown
Member Author

Hmmm what about admins? And why are we making this change?

@scefali updated the PR description with more context

@scefali scefali left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants