Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type {
} from 'sentry/types/integrations';
import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
import type {Organization} from 'sentry/types/organization';
import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';
import {singleLineRenderer} from 'sentry/utils/marked/marked';
import type {ApiQueryKey} from 'sentry/utils/queryClient';
import {setApiQueryData, useApiQuery, useQueryClient} from 'sentry/utils/queryClient';
Expand Down Expand Up @@ -127,7 +128,11 @@ function ConfigureIntegration({params, router, routes, location}: Props) {
useEffect(() => {
// This page should not be accessible by members (unless its github or gitlab)
const allowMemberConfiguration = ['github', 'gitlab'].includes(providerKey);
if (!allowMemberConfiguration && !organization.access.includes('org:integrations')) {
if (
!allowMemberConfiguration &&
!organization.access.includes('org:integrations') &&
!isActiveSuperuser()
) {
router.push(
normalizeUrl({
pathname: `/settings/${organization.slug}/integrations/${providerKey}/`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {Integration, IntegrationProvider} from 'sentry/types/integrations';
import type {Organization} from 'sentry/types/organization';
import type {IntegrationAnalyticsKey} from 'sentry/utils/analytics/integrations';
import {getIntegrationStatus} from 'sentry/utils/integrationUtil';
import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';

import {AddIntegrationButton} from './addIntegrationButton';
import IntegrationItem from './integrationItem';
Expand Down Expand Up @@ -114,6 +115,9 @@ export default class InstalledIntegration extends Component<Props> {
return (
<Access access={['org:integrations']}>
{({hasAccess}) => {
const superuser = isActiveSuperuser();
const canConfigure =
(hasAccess || superuser) && this.integrationStatus === 'active';
const disableAction = !(hasAccess && this.integrationStatus === 'active');
Copy link

Choose a reason for hiding this comment

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

Bug: Superusers without org:integrations permission cannot upgrade integrations because disableAction incorrectly disables the "Update Now" button.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

When a superuser without org:integrations permission attempts to upgrade an integration, the "Update Now" button is disabled. This occurs because the disableAction variable at line 121 does not account for superuser status, unlike the canConfigure variable used for the "Configure" button. This leads to an inconsistent user experience where a superuser can configure an integration but cannot update it.

💡 Suggested Fix

Update the disableAction variable at line 121 to include superuser status, similar to how canConfigure is defined, to allow superusers to upgrade integrations.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
static/app/views/settings/organizationIntegrations/installedIntegration.tsx#L121

Potential issue: When a superuser without `org:integrations` permission attempts to
upgrade an integration, the "Update Now" button is disabled. This occurs because the
`disableAction` variable at line 121 does not account for superuser status, unlike the
`canConfigure` variable used for the "Configure" button. This leads to an inconsistent
user experience where a superuser can configure an integration but cannot update it.

Did we get this right? 👍 / 👎 to inform future reviews.

return (
<Fragment>
Expand All @@ -122,7 +126,7 @@ export default class InstalledIntegration extends Component<Props> {
</IntegrationItemBox>
<div>
<Tooltip
disabled={allowMemberConfiguration || hasAccess}
disabled={allowMemberConfiguration || hasAccess || superuser}
position="left"
title={t(
'You must be an organization owner, manager or admin to configure'
Expand All @@ -148,7 +152,7 @@ export default class InstalledIntegration extends Component<Props> {
<StyledLinkButton
borderless
icon={<IconSettings />}
disabled={!allowMemberConfiguration && disableAction}
disabled={!allowMemberConfiguration && !canConfigure}
to={`/settings/${organization.slug}/integrations/${provider.key}/${integration.id}/`}
data-test-id="integration-configure-button"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import LoadingError from 'sentry/components/loadingError';
import LoadingIndicator from 'sentry/components/loadingIndicator';
import Panel from 'sentry/components/panels/panel';
import PanelItem from 'sentry/components/panels/panelItem';
import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
import {t} from 'sentry/locale';
import {PluginIcon} from 'sentry/plugins/components/pluginIcon';
import {space} from 'sentry/styles/space';
Expand Down Expand Up @@ -556,47 +557,49 @@ export default function IntegrationDetailedView() {
}

return (
<IntegrationLayout.Body
integrationName={integrationName}
alert={<FirstPartyIntegrationAlert integrations={configurations} hideCTA />}
topSection={
<IntegrationLayout.TopSection
featureData={featureData}
integrationName={integrationName}
installationStatus={installationStatus}
integrationIcon={<PluginIcon pluginId={integrationSlug} size={50} />}
addInstallButton={
<IntegrationLayout.AddInstallButton
featureData={featureData}
hideButtonIfDisabled={false}
requiresAccess
renderTopButton={renderTopButton}
/>
}
additionalCTA={
<FirstPartyIntegrationAdditionalCTA integrations={configurations} />
}
/>
}
tabs={renderTabs()}
content={
activeTab === 'overview' ? (
<IntegrationLayout.InformationCard
integrationSlug={integrationSlug}
description={description}
alerts={alerts}
<SentryDocumentTitle title={integrationName}>
Copy link
Member Author

Choose a reason for hiding this comment

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

all whitespace changes except for this

<IntegrationLayout.Body
integrationName={integrationName}
alert={<FirstPartyIntegrationAlert integrations={configurations} hideCTA />}
topSection={
<IntegrationLayout.TopSection
featureData={featureData}
author={author}
resourceLinks={resourceLinks}
permissions={null}
integrationName={integrationName}
installationStatus={installationStatus}
integrationIcon={<PluginIcon pluginId={integrationSlug} size={50} />}
addInstallButton={
<IntegrationLayout.AddInstallButton
featureData={featureData}
hideButtonIfDisabled={false}
requiresAccess
renderTopButton={renderTopButton}
/>
}
additionalCTA={
<FirstPartyIntegrationAdditionalCTA integrations={configurations} />
}
/>
) : activeTab === 'configurations' ? (
renderConfigurations()
) : (
renderFeatures()
)
}
/>
}
tabs={renderTabs()}
content={
activeTab === 'overview' ? (
<IntegrationLayout.InformationCard
integrationSlug={integrationSlug}
description={description}
alerts={alerts}
featureData={featureData}
author={author}
resourceLinks={resourceLinks}
permissions={null}
/>
) : activeTab === 'configurations' ? (
renderConfigurations()
) : (
renderFeatures()
)
}
/>
</SentryDocumentTitle>
);
}

Expand Down
Loading