Security configuration / access control around Bitbucket webhook provisioning

diff --git a/public/app/features/provisioning/Config/ConfigForm.tsx b/public/app/features/provisioning/Config/ConfigForm.tsx index 4125e92e969a8..33224742788ef 100644 --- a/public/app/features/provisioning/Config/ConfigForm.tsx +++ b/public/app/features/provisioning/Config/ConfigForm.tsx @@ -103,6 +103,7 @@ export function ConfigForm({ data }: ConfigFormProps) { const selectedConnection = connections.find((c) => c.metadata?.name === watchedConnectionName); const connectionWebhookDisabled = Boolean(selectedConnection?.spec?.webhook?.disabled); + const emailWebhookDisabled = type === 'bitbucket' && !watch('email')?.trim(); useEffect(() => { if (connectionWebhookDisabled) { @@ -291,6 +292,25 @@ export function ConfigForm({ data }: ConfigFormProps) { /> </Field> )} + {gitFields.emailConfig && ( + <Field + noMargin + label={gitFields.emailConfig.label} + required={gitFields.emailConfig.required} + error={errors?.email?.message} + invalid={!!errors?.email} + description={gitFields.emailConfig.description} + > + <Input + {...register('email', { + required: gitFields.emailConfig.validation?.required, + pattern: gitFields.emailConfig.validation?.pattern, + })} + type="email" + placeholder={gitFields.emailConfig.placeholder} + /> + </Field> + )} {hasTokenInstructions && <TokenPermissionsInfo type={type} url={watch('url')} />} <Field noMargin @@ -436,6 +456,14 @@ export function ConfigForm({ data }: ConfigFormProps) { name="webhook.baseUrl" disabledName="webhook.disabled" connectionWebhookDisabled={connectionWebhookDisabled} + disabledReason={ + emailWebhookDisabled + ? t( + 'provisioning.webhook-section.description-webhook-disabled-email', + 'Webhook integration is disabled because the Atlassian account email is not set. Set it above to enable webhooks.' + ) + : undefined + } disabledError={errors?.webhook?.disabled?.message} /> )} diff --git a/public/app/features/provisioning/Config/WebhookSection.test.tsx b/public/app/features/provisioning/Config/WebhookSection.test.tsx index 7a94a3f1b198b..c614a20362ce8 100644 --- a/public/app/features/provisioning/Config/WebhookSection.test.tsx +++ b/public/app/features/provisioning/Config/WebhookSection.test.tsx @@ -7,7 +7,13 @@ import { type RepositoryFormData } from '../types'; import { WebhookSection } from './WebhookSection'; -function Wrapper({ connectionWebhookDisabled }: { connectionWebhookDisabled?: boolean }) { +function Wrapper({ + connectionWebhookDisabled, + disabledReason, +}: { + connectionWebhookDisabled?: boolean; + disabledReason?: string; +}) { const { register, control } = useForm<RepositoryFormData>(); return ( <WebhookSection @@ -16,6 +22,7 @@ function Wrapper({ connectionWebhookDisabled }: { connectionWebhookDisabled?: bo name="webhook.baseUrl" disabledName="webhook.disabled" connectionWebhookDisabled={connectionWebhookDisabled} + disabledReason={disabledReason} /> ); } @@ -104,6 +111,40 @@ describe('WebhookSection', () => { screen.getByText(/when checked, grafana will not register or receive webhook events/i) ).toBeInTheDocument(); }); + + it('shows the forced description when connectionWebhookDisabled is true', async () => { + const { user } = render(<Wrapper connectionWebhookDisabled={true} />); + + await user.click(screen.getByText('Webhook options')); + + expect(screen.getByText(/disabled because the referenced github app connection/i)).toBeInTheDocument(); + }); + }); + + describe('disabledReason', () => { + it('restores the stored value when the disabled reason goes away', async () => { + const { user, rerender } = render(<Wrapper />); + + await user.click(screen.getByText('Webhook options')); + expect(screen.getByRole('checkbox', { name: /disable webhook integration/i })).not.toBeChecked(); + + rerender(<Wrapper disabledReason="Webhooks need an email." />); + expect(screen.getByRole('checkbox', { name: /disable webhook integration/i })).toBeChecked(); + + rerender(<Wrapper />); + expect(screen.getByRole('checkbox', { name: /disable webhook integration/i })).not.toBeChecked(); + }); + + it('disables the checkbox and URL input and shows the reason', async () => { + const { user } = render(<Wrapper disabledReason="Webhooks need an email." />); + + await user.click(screen.getByText('Webhook options')); + + expect(screen.getByRole('checkbox', { name: /disable webhook integration/i })).toBeDisabled(); + expect(screen.getByRole('checkbox', { name: /disable webhook integration/i })).toBeChecked(); + expect(screen.getByRole('textbox')).toBeDisabled(); + expect(screen.getByText('Webhooks need an email.')).toBeInTheDocument(); + }); }); it('hides the learn more link on public instances', async () => { diff --git a/public/app/features/provisioning/Config/WebhookSection.tsx b/public/app/features/provisioning/Config/WebhookSection.tsx index cba1bdccf4613..029a8833c0ba9 100644 --- a/public/app/features/provisioning/Config/WebhookSection.tsx +++ b/public/app/features/provisioning/Config/WebhookSection.tsx @@ -12,6 +12,7 @@ export interface WebhookSectionProps<T extends FieldValues> { name: Path<T>; disabledName: Path<T>; connectionWebhookDisabled?: boolean; + disabledReason?: string; disabledError?: string; } @@ -21,41 +22,44 @@ export function WebhookSection<T extends FieldValues>({ name, disabledName, connectionWebhookDisabled, + disabledReason, disabledError, }: WebhookSectionProps<T>) { const isPublic = checkPublicAccess(); const webhookDisabled = Boolean(useWatch({ control, name: disabledName })); - const urlDisabled = webhookDisabled || Boolean(connectionWebhookDisabled); + const forcedDisabled = Boolean(connectionWebhookDisabled) || Boolean(disabledReason); + const urlDisabled = webhookDisabled || forcedDisabled; return ( <ControlledCollapse label={t('provisioning.webhook-section.label-webhook', 'Webhook options')} isOpen={false}> <Stack direction="column" gap={2}> - <Field - noMargin - invalid={!!disabledError} - error={disabledError} - description={ - connectionWebhookDisabled - ? t( - 'provisioning.webhook-section.description-webhook-disabled-forced', - 'Webhook integration is disabled because the referenced GitHub App connection has webhook integration disabled.' - ) - : undefined - } - > - <Checkbox - {...register(disabledName)} - disabled={connectionWebhookDisabled} - label={t('provisioning.webhook-section.label-webhook-disabled', 'Disable webhook integration')} - description={ - connectionWebhookDisabled - ? undefined - : t( - 'provisioning.webhook-section.description-webhook-disabled', - 'When checked, Grafana will not register or receive webhook events and will poll the repository on an interval instead. Use this when Grafana is not reachable from the public internet.' - ) - } - /> + <Field noMargin invalid={!!disabledError} error={disabledError}> + {forcedDisabled ? ( + <Checkbox + key="forced" + disabled + checked + label={t('provisioning.webhook-section.label-webhook-disabled', 'Disable webhook integration')} + description={ + connectionWebhookDisabled + ? t( + 'provisioning.webhook-section.description-webhook-disabled-forced', + 'Webhook integration is disabled because the referenced GitHub App connection has webhook integration disabled.' + ) + : disabledReason + } + /> + ) : ( + <Checkbox + key="user" + {...register(disabledName)} + label={t('provisioning.webhook-section.label-webhook-disabled', 'Disable webhook integration')} + description={t( + 'provisioning.webhook-section.description-webhook-disabled', + 'When checked, Grafana will not register or receive webhook events and will poll the repository on an interval instead. Use this when Grafana is not reachable from the public internet.' + )} + /> + )} </Field> <Field noMargin diff --git a/public/app/features/provisioning/Repository/RepositoryOverview.test.tsx b/public/app/features/provisioning/Repository/RepositoryOverview.test.tsx index 92dea7a9a27fc..242ed5f189b26 100644 --- a/public/app/features/provisioning/Repository/RepositoryOverview.test.tsx +++ b/public/app/features/provisioning/Repository/RepositoryOverview.test.tsx @@ -21,7 +21,10 @@ jest.mock('./RepositoryPullStatusCard', () => ({ RepositoryPullStatusCard: () => null, })); -const createMockRepository = (spec: Partial<RepositorySpec>): Repository => ({ +const createMockRepository = ( + spec: Partial<RepositorySpec>, + webhook: NonNullable<Repository['status']>['webhook'] = { id: 42, url: 'https://grafana.example/webhook' } +): Repository => ({ metadata: { name: 'test-repo' }, spec: { title: 'Test Repository', @@ -34,7 +37,7 @@ const createMockRepository = (spec: Partial<RepositorySpec>): Repository => ({ health: { healthy: true, checked: Date.now() }, sync: { state: 'success', message: [] }, observedGeneration: 1, - webhook: { id: 42, url: 'https://grafana.example/webhook' }, + webhook, }, }); @@ -79,6 +82,35 @@ describe('RepositoryOverview', () => { ); }); + it('should link to Bitbucket webhook settings for bitbucket repositories', () => { + const repo = createMockRepository( + { + type: 'bitbucket', + bitbucket: { url: 'https://bitbucket.org/org/repo', branch: 'main' }, + }, + { uuid: '{9a41cbfa-9b26-45f6-8b1a-ce8f7c78b6f0}', url: 'https://grafana.example/webhook' } + ); + render(<RepositoryOverview repo={repo} />); + + expect(screen.getByRole('link', { name: 'View Webhook' })).toHaveAttribute( + 'href', + 'https://bitbucket.org/org/repo/admin/webhooks/%7B9a41cbfa-9b26-45f6-8b1a-ce8f7c78b6f0%7D/edit' + ); + }); + + it('should display the webhook UUID without braces for bitbucket repositories', () => { + const repo = createMockRepository( + { + type: 'bitbucket', + bitbucket: { url: 'https://bitbucket.org/org/repo', branch: 'main' }, + }, + { uuid: '{9a41cbfa-9b26-45f6-8b1a-ce8f7c78b6f0}', url: 'https://grafana.example/webhook' } + ); + render(<RepositoryOverview repo={repo} />); + + expect(screen.getByText('9a41cbfa-9b26-45f6-8b1a-ce8f7c78b6f0')).toBeInTheDocument(); + }); + it('should not render the webhook link for repositories without webhook support', () => { const repo = createMockRepository({ type: 'local', local: { path: '/tmp/repo' } }); render(<RepositoryOverview repo={repo} />); diff --git a/public/app/features/provisioning/Repository/RepositoryOverview.tsx b/public/app/features/provisioning/Repository/RepositoryOverview.tsx index 9098371228af7..9b33feafd61f5 100644 --- a/public/app/features/provisioning/Repository/RepositoryOverview.tsx +++ b/public/app/features/provisioning/Repository/RepositoryOverview.tsx @@ -145,7 +145,9 @@ export function RepositoryOverview({ repo }: { repo: Repository }) { </Text> </div> <div className={styles.valueColumn}> - <Text variant="body">{status?.webhook?.id ?? 'N/A'}</Text> + <Text variant="body"> + {status?.webhook?.id ?? status?.webhook?.uuid?.replace(/[{}]/g, '') ?? 'N/A'} + </Text> </div> <div className={styles.labelColumn}> <Text color="secondary"> @@ -246,5 +248,8 @@ function getWebhookURL(repo: Repository) { if (spec?.type === 'gitlab' && status?.webhook?.url && repoUrl) { return textUtil.sanitizeUrl(`${repoUrl}/-/hooks/${status.webhook?.id}/edit`); } + if (spec?.type === 'bitbucket' && status?.webhook?.uuid && spec.bitbucket?.url) { + return textUtil.sanitizeUrl(`${spec.bitbucket.url}/admin/webhooks/${encodeURIComponent(status.webhook.uuid)}/edit`); + } return undefined; } diff --git a/public/app/features/provisioning/Wizard/FinishStep.test.tsx b/public/app/features/provisioning/Wizard/FinishStep.test.tsx index 1e2d480c712f7..b4e2adf7317c1 100644 --- a/public/app/features/provisioning/Wizard/FinishStep.test.tsx +++ b/public/app/features/provisioning/Wizard/FinishStep.test.tsx @@ -108,10 +108,25 @@ describe('FinishStep', () => { expect(await screen.findByText('Webhook options')).toBeInTheDocument(); }); - it('does not show the webhook section for a git provider without webhooks', async () => { + it('shows the webhook section for a Bitbucket repository', async () => { setup('bitbucket'); - expect(await screen.findByText(PR_LABEL)).toBeInTheDocument(); + expect(await screen.findByText('Webhook options')).toBeInTheDocument(); + }); + + it('forces webhooks off for a Bitbucket repository without an email', async () => { + const { user } = setup('bitbucket'); + + await user.click(await screen.findByText('Webhook options')); + + expect(screen.getByRole('checkbox', { name: /disable webhook integration/i })).toBeDisabled(); + expect(screen.getByText(/atlassian account email is not set/i)).toBeInTheDocument(); + }); + + it('does not show the webhook section for a git provider without webhooks', async () => { + setup('git'); + + expect(await screen.findByText(BRANCH_LABEL)).toBeInTheDocument(); expect(screen.queryByText('Webhook options')).not.toBeInTheDocument(); }); }); diff --git a/public/app/features/provisioning/Wizard/FinishStep.tsx b/public/app/features/provisioning/Wizard/FinishStep.tsx index 93e44b457fec0..5415c68d1ce20 100644 --- a/public/app/features/provisioning/Wizard/FinishStep.tsx +++ b/public/app/features/provisioning/Wi ... [truncated]