webhooks: Add support for secrets for Bitbucket Cloud (#63198)

We didn't support secrets for bitbucket cloud so far, but the code host
does support them. They use the same mechanism as Bitbucket and GitHub
so that was easy to add.

Closes SRC-393

Test plan:

Set up a webhook locally and tested that with a wrong secret it fails,
and with the correct secret it passes. Also wrote a test.
This commit is contained in:
Erik Seliger 2024-06-18 11:54:01 +02:00 committed by GitHub
parent 9b88b93f2a
commit 3e3ef6e8fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 113 additions and 21 deletions

View File

@ -213,7 +213,7 @@ export const WebhookCreateUpdatePage: FC<WebhookCreateUpdatePageProps> = ({ exis
<Input
className={classNames(styles.first, 'flex-1 mb-0')}
message={
webhook.codeHostKind && !codeHostSupportsSecretes(webhook.codeHostKind) ? (
webhook.codeHostKind && !codeHostSupportsSecrets(webhook.codeHostKind) ? (
<small>Code Host doesn't support secrets.</small>
) : (
<small>Randomly generated. Alter as required.</small>
@ -221,14 +221,14 @@ export const WebhookCreateUpdatePage: FC<WebhookCreateUpdatePageProps> = ({ exis
}
label={<span className="small">Secret</span>}
disabled={
webhook.codeHostKind !== null && !codeHostSupportsSecretes(webhook.codeHostKind)
webhook.codeHostKind !== null && !codeHostSupportsSecrets(webhook.codeHostKind)
}
pattern="^[a-zA-Z0-9]+$"
onChange={event => {
onSecretChange(event.target.value)
}}
value={
webhook.codeHostKind && !codeHostSupportsSecretes(webhook.codeHostKind)
webhook.codeHostKind && !codeHostSupportsSecrets(webhook.codeHostKind)
? ''
: webhook.secret || ''
}
@ -310,7 +310,7 @@ function supportedExternalServiceKind(kind: ExternalServiceKind): boolean {
function buildUpdateWebhookVariables(webhook: Webhook, id?: string): UpdateWebhookVariables {
const secret =
webhook.codeHostKind !== null && !codeHostSupportsSecretes(webhook.codeHostKind) ? null : webhook.secret
webhook.codeHostKind !== null && !codeHostSupportsSecrets(webhook.codeHostKind) ? null : webhook.secret
return {
// should not happen when update is called
@ -324,7 +324,7 @@ function buildUpdateWebhookVariables(webhook: Webhook, id?: string): UpdateWebho
function convertWebhookToCreateWebhookVariables(webhook: Webhook): CreateWebhookVariables {
const secret =
webhook.codeHostKind !== null && !codeHostSupportsSecretes(webhook.codeHostKind) ? null : webhook.secret
webhook.codeHostKind !== null && !codeHostSupportsSecrets(webhook.codeHostKind) ? null : webhook.secret
return {
name: webhook.name,
codeHostKind: webhook.codeHostKind || ExternalServiceKind.OTHER,
@ -333,8 +333,8 @@ function convertWebhookToCreateWebhookVariables(webhook: Webhook): CreateWebhook
}
}
function codeHostSupportsSecretes(codeHostKind: ExternalServiceKind): boolean {
if (codeHostKind === ExternalServiceKind.BITBUCKETCLOUD || codeHostKind === ExternalServiceKind.AZUREDEVOPS) {
function codeHostSupportsSecrets(codeHostKind: ExternalServiceKind): boolean {
if (codeHostKind === ExternalServiceKind.AZUREDEVOPS) {
return false
}
return true

View File

@ -96,9 +96,9 @@ func (ws *webhookService) UpdateWebhook(ctx context.Context, id int32, name, cod
func validateCodeHostKindAndSecret(codeHostKind string, secret *string) error {
switch codeHostKind {
case extsvc.KindGitHub, extsvc.KindGitLab, extsvc.KindBitbucketServer:
case extsvc.KindGitHub, extsvc.KindGitLab, extsvc.KindBitbucketServer, extsvc.KindBitbucketCloud:
return nil
case extsvc.KindBitbucketCloud, extsvc.KindAzureDevOps:
case extsvc.KindAzureDevOps:
if secret != nil {
return errors.Newf("webhooks do not support secrets for code host kind %s", codeHostKind)
}

View File

@ -90,9 +90,9 @@ func TestCreateWebhook(t *testing.T) {
},
{
label: "secrets are not supported for code host",
codeHostKind: extsvc.KindBitbucketCloud,
codeHostKind: extsvc.KindAzureDevOps,
secret: &testSecret,
expectedErr: errors.New("webhooks do not support secrets for code host kind BITBUCKETCLOUD"),
expectedErr: errors.New("webhooks do not support secrets for code host kind AZUREDEVOPS"),
},
}

View File

@ -540,13 +540,13 @@ func TestCreateWebhook(t *testing.T) {
ExpectedResult: "null",
ExpectedErrors: []*errors.QueryError{
{
Message: "webhooks do not support secrets for code host kind BITBUCKETCLOUD",
Message: "webhooks do not support secrets for code host kind AZUREDEVOPS",
Path: []any{"createWebhook"},
},
},
Variables: map[string]any{
"name": "webhookName",
"codeHostKind": "BITBUCKETCLOUD",
"codeHostKind": "AZUREDEVOPS",
"codeHostURN": "https://bitbucket.com",
"secret": "mysupersecret",
},

View File

@ -46,6 +46,7 @@ go_test(
"middleware_test.go",
"webhooks_test.go",
],
data = glob(["testdata/**"]),
embed = [":webhooks"],
tags = [
TAG_PLATFORM_SOURCE,

View File

@ -4,6 +4,7 @@ import (
"io"
"net/http"
gh "github.com/google/go-github/v55/github"
"github.com/sourcegraph/log"
"github.com/sourcegraph/sourcegraph/internal/actor"
@ -13,7 +14,7 @@ import (
"github.com/sourcegraph/sourcegraph/lib/errors"
)
func (wr *Router) HandleBitbucketCloudWebhook(logger log.Logger, w http.ResponseWriter, r *http.Request, codeHostURN extsvc.CodeHostBaseURL) {
func (wr *Router) HandleBitbucketCloudWebhook(logger log.Logger, w http.ResponseWriter, r *http.Request, codeHostURN extsvc.CodeHostBaseURL, secret string) {
payload, err := io.ReadAll(r.Body)
if err != nil {
http.Error(w, "Error while reading request body.", http.StatusInternalServerError)
@ -22,6 +23,14 @@ func (wr *Router) HandleBitbucketCloudWebhook(logger log.Logger, w http.Response
defer r.Body.Close()
ctx := actor.WithInternalActor(r.Context())
if secret != "" {
sig := r.Header.Get("X-Hub-Signature")
if err := gh.ValidateSignature(sig, payload, []byte(secret)); err != nil {
http.Error(w, "Could not validate payload with secret.", http.StatusBadRequest)
return
}
}
eventType := r.Header.Get("X-Event-Key")
e, err := bitbucketcloud.ParseWebhookEvent(eventType, payload)
if err != nil {

File diff suppressed because one or more lines are too long

View File

@ -136,8 +136,7 @@ func handler(logger log.Logger, db database.DB, wh *Router) http.HandlerFunc {
wh.handleBitbucketServerWebhook(logger, w, r, webhook.CodeHostURN, secret)
return
case extsvc.KindBitbucketCloud:
// Bitbucket Cloud does not support secrets for webhooks
wh.HandleBitbucketCloudWebhook(logger, w, r, webhook.CodeHostURN)
wh.HandleBitbucketCloudWebhook(logger, w, r, webhook.CodeHostURN, secret)
return
case extsvc.KindAzureDevOps:
wh.HandleAzureDevOpsWebhook(logger, w, r, webhook.CodeHostURN)

View File

@ -10,6 +10,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"testing"
"github.com/sourcegraph/sourcegraph/internal/extsvc/azuredevops"
@ -86,7 +87,27 @@ func TestWebhooksHandler(t *testing.T) {
extsvc.KindBitbucketCloud,
"http://bitbucket.com",
u.ID,
types.NewUnencryptedSecret("bbcloudsecret"),
types.NewUnencryptedSecret("supersecretstring"),
)
require.NoError(t, err)
bbCloudWHOtherSecret, err := dbWebhooks.Create(
context.Background(),
"bb webhook",
extsvc.KindBitbucketCloud,
"http://bitbucket.com",
u.ID,
types.NewUnencryptedSecret("othersecret"),
)
require.NoError(t, err)
bbCloudWHNoSecret, err := dbWebhooks.Create(
context.Background(),
"bb webhook",
extsvc.KindBitbucketCloud,
"http://bitbucket.com",
u.ID,
nil,
)
require.NoError(t, err)
@ -356,8 +377,8 @@ func TestWebhooksHandler(t *testing.T) {
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
})
t.Run("Bitbucket Cloud returns 200", func(t *testing.T) {
requestURL := fmt.Sprintf("%s/.api/webhooks/%v", srv.URL, bbCloudWH.UUID)
t.Run("Bitbucket Cloud returns 200 without secret", func(t *testing.T) {
requestURL := fmt.Sprintf("%s/.api/webhooks/%v", srv.URL, bbCloudWHNoSecret.UUID)
event := bitbucketcloud.PullRequestCommentCreatedEvent{}
payload, err := json.Marshal(event)
@ -379,6 +400,42 @@ func TestWebhooksHandler(t *testing.T) {
assert.Equal(t, http.StatusOK, resp.StatusCode)
logs, _, err := db.WebhookLogs(keyring.Default().WebhookLogKey).List(context.Background(), database.WebhookLogListOpts{
WebhookID: &bbCloudWHNoSecret.ID,
})
assert.NoError(t, err)
assert.Len(t, logs, 1)
for _, log := range logs {
assert.Equal(t, bbCloudWHNoSecret.ID, *log.WebhookID)
}
assert.Equal(t, bbCloudWHNoSecret.CodeHostURN, wh.codeHostURNReceived)
assert.Equal(t, &event, wh.eventReceived)
})
t.Run("Bitbucket Cloud returns 200 with correct secret", func(t *testing.T) {
requestURL := fmt.Sprintf("%s/.api/webhooks/%v", srv.URL, bbCloudWH.UUID)
payload, err := os.ReadFile("testdata/bitbucketcloud_body.json")
require.NoError(t, err)
wh := &fakeWebhookHandler{}
wr.handlers = map[string]eventHandlers{
extsvc.KindBitbucketCloud: {
"repo:push": []Handler{wh.handleEvent},
},
}
req, err := http.NewRequest("POST", requestURL, bytes.NewBuffer(payload))
require.NoError(t, err)
req.Header.Set("X-Event-Key", "repo:push")
req.Header.Set("Content-Type", "application/json")
req.Header.Set("X-Event-Time", "Tue, 11 Jun 2024 10:19:00 GMT")
req.Header.Set("X-Hub-Signature", "sha256=4d0940b0224f927c796e6568837f4af66bd86845fcafab60a357feb2abc5561a")
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
logs, _, err := db.WebhookLogs(keyring.Default().WebhookLogKey).List(context.Background(), database.WebhookLogListOpts{
WebhookID: &bbCloudWH.ID,
})
@ -388,11 +445,36 @@ func TestWebhooksHandler(t *testing.T) {
assert.Equal(t, bbCloudWH.ID, *log.WebhookID)
}
assert.Equal(t, bbCloudWH.CodeHostURN, wh.codeHostURNReceived)
assert.Equal(t, &event, wh.eventReceived)
assert.Equal(t, "{dfeaae25-8168-466f-ade4-d07e6837dedf}", wh.eventReceived.(*bitbucketcloud.PushEvent).Repository.UUID)
})
t.Run("Bitbucket Cloud returns 400 with wrong secret", func(t *testing.T) {
requestURL := fmt.Sprintf("%s/.api/webhooks/%v", srv.URL, bbCloudWHOtherSecret.UUID)
payload, err := os.ReadFile("testdata/bitbucketcloud_body.json")
require.NoError(t, err)
wh := &fakeWebhookHandler{}
wr.handlers = map[string]eventHandlers{
extsvc.KindBitbucketCloud: {
"repo:push": []Handler{wh.handleEvent},
},
}
req, err := http.NewRequest("POST", requestURL, bytes.NewBuffer(payload))
require.NoError(t, err)
req.Header.Set("X-Event-Key", "repo:push")
req.Header.Set("Content-Type", "application/json")
req.Header.Set("X-Event-Time", "Tue, 11 Jun 2024 10:19:00 GMT")
req.Header.Set("X-Hub-Signature", "sha256=4d0940b0224f927c796e6568837f4af66bd86845fcafab60a357feb2abc5561a")
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
})
t.Run("Bitbucket Cloud returns 404 not found if webhook event type unknown", func(t *testing.T) {
requestURL := fmt.Sprintf("%s/.api/webhooks/%v", srv.URL, bbCloudWH.UUID)
requestURL := fmt.Sprintf("%s/.api/webhooks/%v", srv.URL, bbCloudWHNoSecret.UUID)
payload := []byte(`{"body": "text"}`)
require.NoError(t, err)