From 94d95febcd9fc84b1cf2460fe89e41ff6ea2d03c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thor=20Anker=20Kvisg=C3=A5rd=20Lange?= Date: Tue, 3 Oct 2023 09:14:13 +0200 Subject: [PATCH] feat: Using repo hash as HMAC key for Bitbucket webhook --- go.mod | 2 +- go.sum | 4 +- .../forge/bitbucketserver/bitbucketserver.go | 45 ++++++++++++------- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/go.mod b/go.mod index d804f8cbf5..25d53b3133 100644 --- a/go.mod +++ b/go.mod @@ -36,6 +36,7 @@ require ( github.com/moby/moby v20.10.25+incompatible github.com/moby/term v0.5.0 github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450 + github.com/neticdk/go-bitbucket v1.0.0 github.com/oklog/ulid/v2 v2.1.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.16.0 @@ -123,7 +124,6 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/morikuni/aec v1.0.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/neticdk/go-bitbucket v0.1.2 github.com/onsi/ginkgo v1.16.4 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.0.2 // indirect diff --git a/go.sum b/go.sum index 6076452cdd..8ba6cfa527 100644 --- a/go.sum +++ b/go.sum @@ -331,8 +331,8 @@ github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450 h1:j2kD3MT1z4PXCiUll github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450/go.mod h1:skjdDftzkFALcuGzYSklqYd8gvat6F1gZJ4YPVbkZpM= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= -github.com/neticdk/go-bitbucket v0.1.2 h1:V7xC3hd3fOGFjGyhfVaf87f6wjo97z9lv3T6gLxtXUo= -github.com/neticdk/go-bitbucket v0.1.2/go.mod h1:IrHeWO1CrNi0DlOvfhAA9bGRSeNSUB6/SAfzmwbA5aU= +github.com/neticdk/go-bitbucket v1.0.0 h1:FPvHEgPHoDwD2VHbpyu2R2gnoWQ867RxZd2FivS4wSw= +github.com/neticdk/go-bitbucket v1.0.0/go.mod h1:IrHeWO1CrNi0DlOvfhAA9bGRSeNSUB6/SAfzmwbA5aU= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= diff --git a/server/forge/bitbucketserver/bitbucketserver.go b/server/forge/bitbucketserver/bitbucketserver.go index 169dffb4df..f2c6e2c38a 100644 --- a/server/forge/bitbucketserver/bitbucketserver.go +++ b/server/forge/bitbucketserver/bitbucketserver.go @@ -43,8 +43,6 @@ const ( requestTokenURL = "%s/plugins/servlet/oauth/request-token" authorizeTokenURL = "%s/plugins/servlet/oauth/authorize" accessTokenURL = "%s/plugins/servlet/oauth/access-token" - - secret = "045dfb11b042c3c44d68274fd22338e0" // TODO: Temporary 32 bytes? ) // Opts defines configuration options. @@ -420,7 +418,7 @@ func (c *client) Activate(ctx context.Context, u *model.User, r *model.Repo, lin Events: []bb.EventKey{bb.EventKeyRepoRefsChanged, bb.EventKeyPullRequestFrom}, Active: true, Config: &bb.WebhookConfiguration{ - Secret: secret, + Secret: r.Hash, }, } _, _, err = bc.Projects.CreateWebhook(ctx, r.Owner, r.Name, webhook) @@ -468,7 +466,7 @@ func (c *client) Deactivate(ctx context.Context, u *model.User, r *model.Repo, l } func (c *client) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model.Pipeline, error) { - ev, err := bb.ParsePayload(r, []byte(secret)) + ev, payload, err := bb.ParsePayloadWithoutSignature(r) if err != nil { return nil, nil, fmt.Errorf("unable to parse payload from webhook invocation: %w", err) } @@ -486,7 +484,17 @@ func (c *client) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model return nil, nil, nil } - pipe, err = c.updatePipelineFromCommit(ctx, repo, pipe) + user, repo, err := c.getUserAndRepo(ctx, repo) + if err != nil { + return nil, nil, err + } + + err = bb.ValidateSignature(r, payload, []byte(repo.Hash)) + if err != nil { + return nil, nil, fmt.Errorf("unable to validate signature on incoming webhook payload: %w", err) + } + + pipe, err = c.updatePipelineFromCommit(ctx, user, repo, pipe) if err != nil { return nil, nil, err } @@ -498,26 +506,31 @@ func (c *client) Hook(ctx context.Context, r *http.Request) (*model.Repo, *model return repo, pipe, nil } -func (c *client) updatePipelineFromCommit(ctx context.Context, r *model.Repo, p *model.Pipeline) (*model.Pipeline, error) { - if p == nil { - return nil, nil - } - +func (c *client) getUserAndRepo(ctx context.Context, r *model.Repo) (*model.User, *model.Repo, error) { _store, ok := store.TryFromContext(ctx) if !ok { log.Error().Msg("could not get store from context") - return nil, nil + return nil, nil, fmt.Errorf("unable to get store from context") } repo, err := _store.GetRepoForgeID(r.ForgeRemoteID) if err != nil { - return nil, err + return nil, nil, fmt.Errorf("unable to get repo: %w", err) } log.Trace().Any("repo", repo).Msg("got repo") - u, err := _store.GetUser(repo.UserID) + user, err := _store.GetUser(repo.UserID) if err != nil { - return nil, err + return nil, nil, fmt.Errorf("unable to get user: %w", err) + } + log.Trace().Any("user", user).Msg("got user") + + return user, repo, nil +} + +func (c *client) updatePipelineFromCommit(ctx context.Context, u *model.User, r *model.Repo, p *model.Pipeline) (*model.Pipeline, error) { + if p == nil { + return nil, nil } bc, err := c.newClient(u) @@ -525,7 +538,7 @@ func (c *client) updatePipelineFromCommit(ctx context.Context, r *model.Repo, p return nil, fmt.Errorf("unable to create bitbucket client: %w", err) } - commit, _, err := bc.Projects.GetCommit(ctx, repo.Owner, repo.Name, p.Commit) + commit, _, err := bc.Projects.GetCommit(ctx, r.Owner, r.Name, p.Commit) if err != nil { return nil, fmt.Errorf("unable to read commit: %w", err) } @@ -533,7 +546,7 @@ func (c *client) updatePipelineFromCommit(ctx context.Context, r *model.Repo, p opts := &bb.ListOptions{} for { - changes, resp, err := bc.Projects.ListChanges(ctx, repo.Owner, repo.Name, p.Commit, opts) + changes, resp, err := bc.Projects.ListChanges(ctx, r.Owner, r.Name, p.Commit, opts) if err != nil { return nil, fmt.Errorf("unable to list commit changes: %w", err) }