From aeef2ef8bc0618c2a9767fe0b55f7f1d2d982e57 Mon Sep 17 00:00:00 2001 From: Davide Bianchi <10374360+davidebianchi@users.noreply.github.com> Date: Thu, 5 Dec 2024 12:57:11 +0100 Subject: [PATCH 1/4] feat: refactor in order to use custom auth for sources --- internal/config/config.schema.json | 3 +- internal/config/config_test.go | 40 +++- internal/config/testdata/console-config.json | 39 ++++ internal/server/integrations.go | 9 +- internal/sources/jira/config_test.go | 183 ------------------ internal/sources/jira/{config.go => jira.go} | 61 ++++-- internal/sources/jira/jira_test.go | 172 ++++++++++++++++ internal/sources/webhook/config.go | 19 +- internal/sources/webhook/config_test.go | 47 +++++ .../webhook/{validation.go => hmac.go} | 21 +- .../{validation_test.go => hmac_test.go} | 27 +-- internal/sources/webhook/service.go | 2 +- internal/sources/webhook/service_test.go | 30 +-- internal/testutils/router.go | 44 +++++ 14 files changed, 429 insertions(+), 268 deletions(-) create mode 100644 internal/config/testdata/console-config.json delete mode 100644 internal/sources/jira/config_test.go rename internal/sources/jira/{config.go => jira.go} (52%) create mode 100644 internal/sources/jira/jira_test.go rename internal/sources/webhook/{validation.go => hmac.go} (80%) rename internal/sources/webhook/{validation_test.go => hmac_test.go} (80%) create mode 100644 internal/testutils/router.go diff --git a/internal/config/config.schema.json b/internal/config/config.schema.json index eb2eedc..33c4b00 100644 --- a/internal/config/config.schema.json +++ b/internal/config/config.schema.json @@ -13,7 +13,8 @@ "type": { "type": "string", "enum": [ - "jira" + "jira", + "console" ] }, "webhookPath": { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ed29fa3..7b1e621 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -16,6 +16,7 @@ package config import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -72,12 +73,41 @@ func TestLoadServiceConfiguration(t *testing.T) { }, expectedSinkConfig: getExpectedSinkConfig(t), expectedProcessorConfig: getExpectedProcessorConfig(t), - expectedSourceConfig: getExpectedSourceConfig(t), + expectedSourceConfig: getExpectedSourceConfig(t, "jira"), }, "invalid config if integrations is empty": { path: "./testdata/empty-integrations.json", expectedError: "configuration not valid: json schema validation errors:", }, + "console config is parsed correctly": { + path: "./testdata/console-config.json", + expectedContent: &Configuration{ + Integrations: []Integration{ + { + Source: GenericConfig{ + Type: "console", + }, + Pipelines: []Pipeline{ + { + Processors: Processors{ + { + Type: "mapper", + }, + }, + Sinks: Sinks{ + { + Type: "mongo", + }, + }, + }, + }, + }, + }, + }, + expectedSinkConfig: getExpectedSinkConfig(t), + expectedProcessorConfig: getExpectedProcessorConfig(t), + expectedSourceConfig: getExpectedSourceConfig(t, "console"), + }, } for testName, test := range tests { @@ -178,16 +208,16 @@ func getExpectedProcessorConfig(t *testing.T) string { }` } -func getExpectedSourceConfig(t *testing.T) string { +func getExpectedSourceConfig(t *testing.T, sourceType string) string { t.Helper() - return `{ - "type": "jira", + return fmt.Sprintf(`{ + "type": "%s", "webhookPath": "/custom-webhook-path", "authentication": { "secret": { "fromFile": "testdata/secret" } } -}` +}`, sourceType) } diff --git a/internal/config/testdata/console-config.json b/internal/config/testdata/console-config.json new file mode 100644 index 0000000..9815698 --- /dev/null +++ b/internal/config/testdata/console-config.json @@ -0,0 +1,39 @@ +{ + "integrations": [ + { + "source": { + "type": "console", + "webhookPath": "/custom-webhook-path", + "authentication": { + "secret": { + "fromFile": "testdata/secret" + } + } + }, + "pipelines": [ + { + "processors": [ + { + "type": "mapper", + "outputEvent": { + "key": "{{ issue.key }}", + "summary": "{{ issue.fields.summary }}", + "createdAt": "{{ issue.fields.created }}", + "description": "{{ issue.fields.description }}" + } + } + ], + "sinks": [ + { + "type": "mongo", + "url": { + "fromEnv": "TEST_LOAD_SERVICE_MONGO_URL" + }, + "collection": "my-collection" + } + ] + } + ] + } + ] +} diff --git a/internal/server/integrations.go b/internal/server/integrations.go index 3094b53..e2e95d0 100644 --- a/internal/server/integrations.go +++ b/internal/server/integrations.go @@ -28,7 +28,6 @@ import ( "github.com/mia-platform/integration-connector-agent/internal/sinks/mongo" "github.com/mia-platform/integration-connector-agent/internal/sources" "github.com/mia-platform/integration-connector-agent/internal/sources/jira" - "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" swagger "github.com/davidebianchi/gswagger" "github.com/gofiber/fiber/v2" @@ -68,15 +67,9 @@ func setupPipelines(ctx context.Context, log *logrus.Logger, cfg *config.Configu source := cfgIntegration.Source switch source.Type { case sources.Jira: - jiraConfig, err := config.GetConfig[*jira.Config](cfgIntegration.Source) - if err != nil { - return err - } - - if err := webhook.SetupService(ctx, oasRouter, &jiraConfig.Configuration, pg); err != nil { + if err := jira.AddSourceToRouter(ctx, source, pg, oasRouter); err != nil { return fmt.Errorf("%w: %s", errSetupSource, err) } - case "test": // do nothing only for testing return nil diff --git a/internal/sources/jira/config_test.go b/internal/sources/jira/config_test.go deleted file mode 100644 index bc499ea..0000000 --- a/internal/sources/jira/config_test.go +++ /dev/null @@ -1,183 +0,0 @@ -// Copyright Mia srl -// SPDX-License-Identifier: Apache-2.0 -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package jira - -import ( - "encoding/json" - "os" - "testing" - - "github.com/mia-platform/integration-connector-agent/internal/config" - "github.com/mia-platform/integration-connector-agent/internal/entities" - "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" - "github.com/stretchr/testify/require" -) - -func TestConfig(t *testing.T) { - testCases := map[string]struct { - config *Config - - expectedConfig *Config - expectedError error - }{ - "with default": { - config: &Config{}, - expectedConfig: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: defaultWebhookPath, - Authentication: webhook.Authentication{ - HeaderName: defaultAuthHeaderName, - }, - Events: &DefaultSupportedEvents, - }, - }, - }, - "with custom values": { - config: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/custom/webhook", - Authentication: webhook.Authentication{ - HeaderName: "X-Custom-Header", - Secret: config.SecretSource("secret"), - }, - Events: &webhook.Events{ - EventTypeFieldPath: "customEventPath", - Supported: map[string]webhook.Event{ - "event1": { - Operation: entities.Write, - FieldID: "id", - }, - }, - }, - }, - }, - expectedConfig: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/custom/webhook", - Authentication: webhook.Authentication{ - HeaderName: "X-Custom-Header", - Secret: config.SecretSource("secret"), - }, - Events: &webhook.Events{ - EventTypeFieldPath: "customEventPath", - Supported: map[string]webhook.Event{ - "event1": { - Operation: entities.Write, - FieldID: "id", - }, - }, - }, - }, - }, - }, - "with custom event and default event type field path": { - config: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/custom/webhook", - Authentication: webhook.Authentication{ - HeaderName: "X-Custom-Header", - Secret: config.SecretSource("secret"), - }, - Events: &webhook.Events{ - Supported: map[string]webhook.Event{ - "event1": { - Operation: entities.Write, - FieldID: "id", - }, - }, - }, - }, - }, - expectedConfig: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/custom/webhook", - Authentication: webhook.Authentication{ - HeaderName: "X-Custom-Header", - Secret: config.SecretSource("secret"), - }, - Events: &webhook.Events{ - EventTypeFieldPath: webhookEventPath, - Supported: map[string]webhook.Event{ - "event1": { - Operation: entities.Write, - FieldID: "id", - }, - }, - }, - }, - }, - }, - "with custom event and default supported events": { - config: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/custom/webhook", - Authentication: webhook.Authentication{ - HeaderName: "X-Custom-Header", - Secret: config.SecretSource("secret"), - }, - Events: &webhook.Events{ - EventTypeFieldPath: "customEventPath", - }, - }, - }, - expectedConfig: &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/custom/webhook", - Authentication: webhook.Authentication{ - HeaderName: "X-Custom-Header", - Secret: config.SecretSource("secret"), - }, - Events: &webhook.Events{ - EventTypeFieldPath: "customEventPath", - Supported: DefaultSupportedEvents.Supported, - }, - }, - }, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - err := tc.config.Validate() - if tc.expectedError != nil { - require.EqualError(t, err, tc.expectedError.Error()) - return - } - require.NoError(t, err) - require.Equal(t, tc.expectedConfig, tc.config) - }) - } - - t.Run("unmarshal config", func(t *testing.T) { - rawConfig, err := os.ReadFile("testdata/config.json") - require.NoError(t, err) - - actual := &Config{} - require.NoError(t, json.Unmarshal(rawConfig, actual)) - require.NoError(t, actual.Validate()) - - require.Equal(t, &Config{ - Configuration: webhook.Configuration{ - WebhookPath: "/webhook", - Authentication: webhook.Authentication{ - HeaderName: defaultAuthHeaderName, - Secret: config.SecretSource("SECRET_VALUE"), - }, - Events: &DefaultSupportedEvents, - }, - }, actual) - }) -} diff --git a/internal/sources/jira/config.go b/internal/sources/jira/jira.go similarity index 52% rename from internal/sources/jira/config.go rename to internal/sources/jira/jira.go index 0aa6a3b..19f9fd8 100644 --- a/internal/sources/jira/config.go +++ b/internal/sources/jira/jira.go @@ -16,7 +16,14 @@ package jira import ( + "context" + + "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/pipeline" "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + + swagger "github.com/davidebianchi/gswagger" + "github.com/gofiber/fiber/v2" ) const ( @@ -27,28 +34,16 @@ const ( ) type Config struct { - webhook.Configuration + Authentication webhook.HMAC `json:"authentication"` + WebhookPath string `json:"webhookPath"` } func (c *Config) withDefault() *Config { - if c.Authentication.HeaderName == "" { - c.Authentication.HeaderName = defaultAuthHeaderName - } - if c.WebhookPath == "" { c.WebhookPath = defaultWebhookPath } - - if c.Events == nil { - c.Events = &DefaultSupportedEvents - } - - if c.Events.EventTypeFieldPath == "" { - c.Events.EventTypeFieldPath = webhookEventPath - } - - if c.Events.Supported == nil { - c.Events.Supported = DefaultSupportedEvents.Supported + if c.Authentication.HeaderName == "" { + c.Authentication.HeaderName = defaultAuthHeaderName } return c @@ -57,9 +52,39 @@ func (c *Config) withDefault() *Config { func (c *Config) Validate() error { c.withDefault() - if err := c.Configuration.Validate(); err != nil { + return nil +} + +func (c *Config) getWebhookConfig() (*webhook.Configuration, error) { + webhookConfig := &webhook.Configuration{ + WebhookPath: c.WebhookPath, + Authentication: webhook.HMAC{ + HeaderName: c.Authentication.HeaderName, + Secret: c.Authentication.Secret, + }, + Events: &DefaultSupportedEvents, + } + if err := webhookConfig.Validate(); err != nil { + return nil, err + } + return webhookConfig, nil +} + +func AddSourceToRouter( + ctx context.Context, + cfg config.GenericConfig, + pg *pipeline.Group, + router *swagger.Router[fiber.Handler, fiber.Router], +) error { + jiraConfig, err := config.GetConfig[*Config](cfg) + if err != nil { return err } - return nil + webhookConfig, err := jiraConfig.getWebhookConfig() + if err != nil { + return err + } + + return webhook.SetupService(ctx, router, webhookConfig, pg) } diff --git a/internal/sources/jira/jira_test.go b/internal/sources/jira/jira_test.go new file mode 100644 index 0000000..a18de6b --- /dev/null +++ b/internal/sources/jira/jira_test.go @@ -0,0 +1,172 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package jira + +import ( + "context" + "encoding/json" + "os" + "testing" + + "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/pipeline" + "github.com/mia-platform/integration-connector-agent/internal/processors" + fakewriter "github.com/mia-platform/integration-connector-agent/internal/sinks/fake" + "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + "github.com/mia-platform/integration-connector-agent/internal/testutils" + "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/require" +) + +func TestValidateConfig(t *testing.T) { + testCases := map[string]struct { + config *Config + + expectedConfig *Config + expectedError error + }{ + "with default": { + config: &Config{}, + expectedConfig: &Config{ + WebhookPath: defaultWebhookPath, + Authentication: webhook.HMAC{ + HeaderName: defaultAuthHeaderName, + }, + }, + }, + "with custom values": { + config: &Config{ + WebhookPath: "/custom/webhook", + Authentication: webhook.HMAC{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + }, + expectedConfig: &Config{ + WebhookPath: "/custom/webhook", + Authentication: webhook.HMAC{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + err := tc.config.Validate() + if tc.expectedError != nil { + require.EqualError(t, err, tc.expectedError.Error()) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectedConfig, tc.config) + }) + } + + t.Run("unmarshal config", func(t *testing.T) { + rawConfig, err := os.ReadFile("testdata/config.json") + require.NoError(t, err) + + actual := &Config{} + require.NoError(t, json.Unmarshal(rawConfig, actual)) + require.NoError(t, actual.Validate()) + + require.Equal(t, &Config{ + WebhookPath: "/webhook", + Authentication: webhook.HMAC{ + HeaderName: defaultAuthHeaderName, + Secret: config.SecretSource("SECRET_VALUE"), + }, + }, actual) + }) +} + +func TestGetWebhookConfig(t *testing.T) { + testCases := map[string]struct { + config *Config + + expectedConfig *webhook.Configuration + expectedError string + }{ + "valid config without authentication": { + config: &Config{ + WebhookPath: "/webhook", + }, + expectedConfig: &webhook.Configuration{ + WebhookPath: "/webhook", + Authentication: webhook.HMAC{}, + Events: &DefaultSupportedEvents, + }, + }, + "valid config with authentication": { + config: &Config{ + WebhookPath: "/webhook", + Authentication: webhook.HMAC{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + }, + expectedConfig: &webhook.Configuration{ + WebhookPath: "/webhook", + Authentication: webhook.HMAC{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + Events: &DefaultSupportedEvents, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + webhookConfig, err := tc.config.getWebhookConfig() + require.NoError(t, err) + + if tc.expectedError != "" { + require.EqualError(t, err, tc.expectedError) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectedConfig, webhookConfig) + }) + } +} + +func TestAddSourceToRouter(t *testing.T) { + logger, _ := test.NewNullLogger() + + t.Run("setup webhook", func(t *testing.T) { + ctx := context.Background() + + rawConfig, err := os.ReadFile("testdata/config.json") + require.NoError(t, err) + cfg := config.GenericConfig{} + require.NoError(t, json.Unmarshal(rawConfig, &cfg)) + + _, router := testutils.GetTestRouter(t) + + proc := &processors.Processors{} + s := fakewriter.New(nil) + p1, err := pipeline.New(logger, proc, s) + require.NoError(t, err) + + pg := pipeline.NewGroup(logger, p1) + + err = AddSourceToRouter(ctx, cfg, pg, router) + require.NoError(t, err) + }) +} diff --git a/internal/sources/webhook/config.go b/internal/sources/webhook/config.go index 031a96c..50492c4 100644 --- a/internal/sources/webhook/config.go +++ b/internal/sources/webhook/config.go @@ -17,13 +17,15 @@ package webhook import ( "fmt" - - "github.com/mia-platform/integration-connector-agent/internal/config" ) -type Authentication struct { - Secret config.SecretSource `json:"secret"` - HeaderName string `json:"headerName"` +type ValidatingRequest interface { + GetReqHeaders() map[string][]string + Body() []byte +} + +type Authentication interface { + CheckSignature(req ValidatingRequest) error } // Configuration is the representation of the configuration for a Jira Cloud webhook @@ -46,3 +48,10 @@ func (c *Configuration) Validate() error { return nil } + +func (c *Configuration) CheckSignature(req ValidatingRequest) error { + if c == nil || c.Authentication == nil { + return nil + } + return c.Authentication.CheckSignature(req) +} diff --git a/internal/sources/webhook/config_test.go b/internal/sources/webhook/config_test.go index 5d9a623..2c58a7e 100644 --- a/internal/sources/webhook/config_test.go +++ b/internal/sources/webhook/config_test.go @@ -55,3 +55,50 @@ func TestValidateConfiguration(t *testing.T) { }) } } + +type fakeRequest struct { + body []byte + headers map[string][]string +} + +func (f fakeRequest) GetReqHeaders() map[string][]string { + return f.headers +} +func (f fakeRequest) Body() []byte { + return f.body +} + +func TestCheckSignature(t *testing.T) { + testCases := map[string]struct { + config Configuration + req ValidatingRequest + + expectedErr string + }{ + "no authentication": {}, + "request is nil": { + config: Configuration{ + Authentication: &HMAC{}, + }, + expectedErr: "invalid webhook authentication configuration: request is nil", + }, + "ok": { + config: Configuration{}, + req: &fakeRequest{ + body: []byte("body"), + headers: map[string][]string{"header": {"sha256=signature"}}, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + err := tc.config.CheckSignature(tc.req) + if tc.expectedErr != "" { + require.EqualError(t, err, tc.expectedErr) + return + } + require.NoError(t, err) + }) + } +} diff --git a/internal/sources/webhook/validation.go b/internal/sources/webhook/hmac.go similarity index 80% rename from internal/sources/webhook/validation.go rename to internal/sources/webhook/hmac.go index 8066ce9..05b451c 100644 --- a/internal/sources/webhook/validation.go +++ b/internal/sources/webhook/hmac.go @@ -22,6 +22,8 @@ import ( "errors" "fmt" "strings" + + "github.com/mia-platform/integration-connector-agent/internal/config" ) const ( @@ -32,21 +34,24 @@ const ( invalidSignatureError = "invalid signature in request" ) -type ValidatingRequest interface { - GetReqHeaders() map[string][]string - Body() []byte +type HMAC struct { + Secret config.SecretSource `json:"secret"` + HeaderName string `json:"headerName"` } -// ValidateWebhookRequest will read the webhook signature header and the given secret for validating the webhook +// CheckSignature will read the webhook signature header and the given secret for validating the webhook // payload. It will fail if there is a mismatch in the signatures and if a signature or a secret is provided and the // other is not present. -func ValidateWebhookRequest(req ValidatingRequest, authentication Authentication) error { - secret := authentication.Secret.String() - if secret != "" && authentication.HeaderName == "" { +func (h HMAC) CheckSignature(req ValidatingRequest) error { + if req == nil { + return fmt.Errorf("%s: request is nil", invalidWebhookAuthenticationConfig) + } + secret := h.Secret.String() + if secret != "" && h.HeaderName == "" { return fmt.Errorf("%s: secret is set but headerName not present", invalidWebhookAuthenticationConfig) } - headerValues := req.GetReqHeaders()[authentication.HeaderName] + headerValues := req.GetReqHeaders()[h.HeaderName] switch { case len(headerValues) == 0 && len(secret) == 0: return nil diff --git a/internal/sources/webhook/validation_test.go b/internal/sources/webhook/hmac_test.go similarity index 80% rename from internal/sources/webhook/validation_test.go rename to internal/sources/webhook/hmac_test.go index d7c380f..fd13326 100644 --- a/internal/sources/webhook/validation_test.go +++ b/internal/sources/webhook/hmac_test.go @@ -19,11 +19,10 @@ import ( "errors" "testing" - "github.com/mia-platform/integration-connector-agent/internal/config" "github.com/stretchr/testify/assert" ) -func TestValidateWebhookRequest(t *testing.T) { +func TestCheckHMACSignature(t *testing.T) { t.Parallel() webhookSignatureHeader := "X-Hub-Signature" @@ -33,9 +32,11 @@ func TestValidateWebhookRequest(t *testing.T) { authentication Authentication expectedErr error }{ - "no header and no secret return no error": {}, + "no header and no secret return no error": { + authentication: &HMAC{}, + }, "missing secret return error": { - authentication: Authentication{ + authentication: &HMAC{ HeaderName: webhookSignatureHeader, }, request: fakeValidatingRequest{ @@ -46,16 +47,16 @@ func TestValidateWebhookRequest(t *testing.T) { expectedErr: errors.New(signatureHeaderButNoSecretError), }, "missing header return error": { - authentication: Authentication{ + authentication: &HMAC{ HeaderName: webhookSignatureHeader, - Secret: config.SecretSource("secret"), + Secret: "secret", }, expectedErr: errors.New(noSignatureHeaderButSecretError), }, "multiple header return error": { - authentication: Authentication{ + authentication: &HMAC{ HeaderName: webhookSignatureHeader, - Secret: config.SecretSource("secret"), + Secret: "secret", }, request: fakeValidatingRequest{ headers: map[string][]string{ @@ -65,9 +66,9 @@ func TestValidateWebhookRequest(t *testing.T) { expectedErr: errors.New(multipleSignatureHeadersError), }, "valid signature return nil": { - authentication: Authentication{ + authentication: &HMAC{ HeaderName: webhookSignatureHeader, - Secret: config.SecretSource("It's a Secret to Everybody"), + Secret: "It's a Secret to Everybody", }, request: fakeValidatingRequest{ body: []byte("Hello World!"), @@ -77,9 +78,9 @@ func TestValidateWebhookRequest(t *testing.T) { }, }, "invalid signature return error": { - authentication: Authentication{ + authentication: &HMAC{ HeaderName: webhookSignatureHeader, - Secret: config.SecretSource("It's a Secret to Everybody"), + Secret: "It's a Secret to Everybody", }, request: fakeValidatingRequest{ body: []byte("tampered body"), @@ -93,7 +94,7 @@ func TestValidateWebhookRequest(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - err := ValidateWebhookRequest(test.request, test.authentication) + err := test.authentication.CheckSignature(test.request) assert.Equal(t, test.expectedErr, err) }) } diff --git a/internal/sources/webhook/service.go b/internal/sources/webhook/service.go index a2f7e8a..a08e79e 100644 --- a/internal/sources/webhook/service.go +++ b/internal/sources/webhook/service.go @@ -58,7 +58,7 @@ func webhookHandler(config *Configuration, p *pipeline.Group) fiber.Handler { return func(c *fiber.Ctx) error { log := glogrus.FromContext(c.UserContext()) - if err := ValidateWebhookRequest(c, config.Authentication); err != nil { + if err := config.CheckSignature(c); err != nil { log.WithError(err).Error("error validating webhook request") return c.Status(http.StatusBadRequest).JSON(utils.ValidationError(err.Error())) } diff --git a/internal/sources/webhook/service_test.go b/internal/sources/webhook/service_test.go index e972321..9b806b0 100644 --- a/internal/sources/webhook/service_test.go +++ b/internal/sources/webhook/service_test.go @@ -24,17 +24,13 @@ import ( "net/http/httptest" "testing" - "github.com/mia-platform/integration-connector-agent/internal/config" "github.com/mia-platform/integration-connector-agent/internal/entities" "github.com/mia-platform/integration-connector-agent/internal/pipeline" "github.com/mia-platform/integration-connector-agent/internal/processors" fakesink "github.com/mia-platform/integration-connector-agent/internal/sinks/fake" + "github.com/mia-platform/integration-connector-agent/internal/testutils" "github.com/mia-platform/integration-connector-agent/internal/utils" - swagger "github.com/davidebianchi/gswagger" - oasfiber "github.com/davidebianchi/gswagger/support/fiber" - "github.com/getkin/kin-openapi/openapi3" - "github.com/gofiber/fiber/v2" "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" ) @@ -65,8 +61,8 @@ func TestSetupServiceWithConfig(t *testing.T) { "fails validation": { config: &Configuration{ WebhookPath: defaultWebhookEndpoint, - Authentication: Authentication{ - Secret: config.SecretSource("SECRET"), + Authentication: HMAC{ + Secret: "SECRET", HeaderName: "X-Hub-Signature", }, Events: &Events{}, @@ -121,7 +117,7 @@ func TestSetupServiceWithConfig(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { - app, router := getRouter(t) + app, router := testutils.GetTestRouter(t) proc := &processors.Processors{} s := fakesink.New(nil) @@ -145,21 +141,3 @@ func TestSetupServiceWithConfig(t *testing.T) { }) } } - -func getRouter(t *testing.T) (*fiber.App, *swagger.Router[fiber.Handler, fiber.Router]) { - t.Helper() - - app := fiber.New() - router, err := swagger.NewRouter(oasfiber.NewRouter(app), swagger.Options{ - Openapi: &openapi3.T{ - OpenAPI: "3.1.0", - Info: &openapi3.Info{ - Title: "Test", - Version: "test-version", - }, - }, - }) - require.NoError(t, err) - - return app, router -} diff --git a/internal/testutils/router.go b/internal/testutils/router.go new file mode 100644 index 0000000..400728d --- /dev/null +++ b/internal/testutils/router.go @@ -0,0 +1,44 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package testutils + +import ( + "testing" + + swagger "github.com/davidebianchi/gswagger" + oasfiber "github.com/davidebianchi/gswagger/support/fiber" + "github.com/getkin/kin-openapi/openapi3" + "github.com/gofiber/fiber/v2" + "github.com/stretchr/testify/require" +) + +func GetTestRouter(t *testing.T) (*fiber.App, *swagger.Router[fiber.Handler, fiber.Router]) { + t.Helper() + + app := fiber.New() + router, err := swagger.NewRouter(oasfiber.NewRouter(app), swagger.Options{ + Openapi: &openapi3.T{ + OpenAPI: "3.1.0", + Info: &openapi3.Info{ + Title: "Test", + Version: "test-version", + }, + }, + }) + require.NoError(t, err) + + return app, router +} From 093950b98fcebd389fcb8da2f968792c5b7fb68f Mon Sep 17 00:00:00 2001 From: Davide Bianchi <10374360+davidebianchi@users.noreply.github.com> Date: Sun, 8 Dec 2024 17:55:36 +0100 Subject: [PATCH 2/4] feat: add support to mia-platform-console webhook and add tests to jira ones --- internal/pipeline/pipeline.go | 1 + internal/sinks/fake/fake.go | 7 + internal/sinks/fake/fake_test.go | 15 + internal/sinks/mongo/mongo.go | 3 +- internal/sources/jira/jira.go | 15 +- internal/sources/jira/jira_test.go | 158 +++++++++- .../sources/mia-platform-console/console.go | 87 ++++++ .../mia-platform-console/console_test.go | 273 ++++++++++++++++++ .../sources/mia-platform-console/events.go | 125 ++++++++ .../mia-platform-console/testdata/config.json | 9 + .../mia-platform-console/testdata/secret | 1 + .../mia-platform-console/validation.go | 68 +++++ .../mia-platform-console/validation_test.go | 133 +++++++++ internal/sources/webhook/config.go | 7 +- internal/sources/webhook/config_test.go | 2 +- internal/sources/webhook/events.go | 13 +- internal/sources/webhook/events_test.go | 18 ++ internal/sources/webhook/hmac.go | 46 +-- internal/sources/webhook/hmac_test.go | 10 +- internal/sources/webhook/service_test.go | 2 +- 20 files changed, 951 insertions(+), 42 deletions(-) create mode 100644 internal/sources/mia-platform-console/console.go create mode 100644 internal/sources/mia-platform-console/console_test.go create mode 100644 internal/sources/mia-platform-console/events.go create mode 100644 internal/sources/mia-platform-console/testdata/config.json create mode 100644 internal/sources/mia-platform-console/testdata/secret create mode 100644 internal/sources/mia-platform-console/validation.go create mode 100644 internal/sources/mia-platform-console/validation_test.go diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index fd4a071..f273eea 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -113,6 +113,7 @@ func New(logger *logrus.Logger, p *processors.Processors, sinks sinks.Sink[entit return pipeline, nil } +// TODO: set as utils and reuse it in CheckSignature func isNil(i any) bool { return i == nil || (reflect.ValueOf(i).Kind() == reflect.Ptr && reflect.ValueOf(i).IsNil()) } diff --git a/internal/sinks/fake/fake.go b/internal/sinks/fake/fake.go index 29303c3..8933e31 100644 --- a/internal/sinks/fake/fake.go +++ b/internal/sinks/fake/fake.go @@ -88,6 +88,13 @@ func (f *Writer) Calls() Calls { return f.stub } +func (f *Writer) ResetCalls() { + f.mtx.Lock() + defer f.mtx.Unlock() + + f.stub = Calls{} +} + func (f *Writer) AddMock(mock Mock) { f.mtx.Lock() defer f.mtx.Unlock() diff --git a/internal/sinks/fake/fake_test.go b/internal/sinks/fake/fake_test.go index f280f5f..f26dc3a 100644 --- a/internal/sinks/fake/fake_test.go +++ b/internal/sinks/fake/fake_test.go @@ -67,6 +67,21 @@ func TestImplementWriter(t *testing.T) { }, f.Calls().LastCall()) }) + t.Run("ResetCalls clean calls", func(t *testing.T) { + f := New(config) + + event := &entities.Event{ + ID: "id", + OperationType: entities.Write, + } + err := f.WriteData(context.Background(), event) + require.NoError(t, err) + + require.Len(t, f.Calls(), 1) + f.ResetCalls() + require.Len(t, f.Calls(), 0) + }) + t.Run("mock error write", func(t *testing.T) { f := New(config) diff --git a/internal/sinks/mongo/mongo.go b/internal/sinks/mongo/mongo.go index 72368e1..9722a7b 100644 --- a/internal/sinks/mongo/mongo.go +++ b/internal/sinks/mongo/mongo.go @@ -172,10 +172,9 @@ func (w *Writer[T]) Delete(ctx context.Context, data T) error { return err } - opts := options.Delete() result, err := w.client.Database(w.database). Collection(w.collection). - DeleteOne(ctxWithCancel, queryFilter, opts) + DeleteOne(ctxWithCancel, queryFilter) if err != nil { return err } diff --git a/internal/sources/jira/jira.go b/internal/sources/jira/jira.go index 19f9fd8..1155447 100644 --- a/internal/sources/jira/jira.go +++ b/internal/sources/jira/jira.go @@ -27,8 +27,8 @@ import ( ) const ( - defaultWebhookPath = "/jira/webhook" - defaultAuthHeaderName = "X-Hub-Signature" + defaultWebhookPath = "/jira/webhook" + authHeaderName = "X-Hub-Signature" webhookEventPath = "webhookEvent" ) @@ -43,7 +43,7 @@ func (c *Config) withDefault() *Config { c.WebhookPath = defaultWebhookPath } if c.Authentication.HeaderName == "" { - c.Authentication.HeaderName = defaultAuthHeaderName + c.Authentication.HeaderName = authHeaderName } return c @@ -57,12 +57,9 @@ func (c *Config) Validate() error { func (c *Config) getWebhookConfig() (*webhook.Configuration, error) { webhookConfig := &webhook.Configuration{ - WebhookPath: c.WebhookPath, - Authentication: webhook.HMAC{ - HeaderName: c.Authentication.HeaderName, - Secret: c.Authentication.Secret, - }, - Events: &DefaultSupportedEvents, + WebhookPath: c.WebhookPath, + Authentication: c.Authentication, + Events: &DefaultSupportedEvents, } if err := webhookConfig.Validate(); err != nil { return nil, err diff --git a/internal/sources/jira/jira_test.go b/internal/sources/jira/jira_test.go index a18de6b..c467bb0 100644 --- a/internal/sources/jira/jira_test.go +++ b/internal/sources/jira/jira_test.go @@ -16,12 +16,21 @@ package jira import ( + "bytes" "context" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" "encoding/json" + "fmt" + "net/http" + "net/http/httptest" "os" "testing" + "time" "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/entities" "github.com/mia-platform/integration-connector-agent/internal/pipeline" "github.com/mia-platform/integration-connector-agent/internal/processors" fakewriter "github.com/mia-platform/integration-connector-agent/internal/sinks/fake" @@ -43,7 +52,7 @@ func TestValidateConfig(t *testing.T) { expectedConfig: &Config{ WebhookPath: defaultWebhookPath, Authentication: webhook.HMAC{ - HeaderName: defaultAuthHeaderName, + HeaderName: authHeaderName, }, }, }, @@ -88,7 +97,7 @@ func TestValidateConfig(t *testing.T) { require.Equal(t, &Config{ WebhookPath: "/webhook", Authentication: webhook.HMAC{ - HeaderName: defaultAuthHeaderName, + HeaderName: authHeaderName, Secret: config.SecretSource("SECRET_VALUE"), }, }, actual) @@ -157,7 +166,7 @@ func TestAddSourceToRouter(t *testing.T) { cfg := config.GenericConfig{} require.NoError(t, json.Unmarshal(rawConfig, &cfg)) - _, router := testutils.GetTestRouter(t) + app, router := testutils.GetTestRouter(t) proc := &processors.Processors{} s := fakewriter.New(nil) @@ -168,5 +177,148 @@ func TestAddSourceToRouter(t *testing.T) { err = AddSourceToRouter(ctx, cfg, pg, router) require.NoError(t, err) + + testCases := []struct { + eventName string + body string + + expectedID string + expectedOperation entities.Operation + }{ + { + eventName: issueCreated, + body: getIssueBody(issueCreated, "12345"), + expectedID: "12345", + }, + { + eventName: issueUpdated, + body: getIssueBody(issueUpdated, "12345"), + expectedID: "12345", + }, + { + eventName: issueDeleted, + body: getIssueBody(issueDeleted, "12345"), + expectedID: "12345", + expectedOperation: entities.Delete, + }, + { + eventName: issueLinkCreated, + body: getIssueLinkBody(issueLinkCreated, "12345"), + expectedID: "12345", + }, + { + eventName: issueLinkDeleted, + body: getIssueLinkBody(issueLinkDeleted, "12345"), + expectedID: "12345", + expectedOperation: entities.Delete, + }, + { + eventName: projectCreated, + body: getProjectBody(projectCreated, "12345"), + expectedID: "12345", + }, + { + eventName: projectUpdated, + body: getProjectBody(projectUpdated, "12345"), + expectedID: "12345", + }, + { + eventName: projectDeleted, + body: getProjectBody(projectDeleted, "12345"), + expectedID: "12345", + expectedOperation: entities.Delete, + }, + { + eventName: projectSoftDeleted, + body: getProjectBody(projectSoftDeleted, "12345"), + expectedID: "12345", + expectedOperation: entities.Delete, + }, + { + eventName: projectRestoredDeleted, + body: getProjectBody(projectRestoredDeleted, "12345"), + expectedID: "12345", + }, + { + eventName: versionReleased, + body: getVersionBody(versionReleased, "12345"), + expectedID: "12345", + }, + { + eventName: versionUnreleased, + body: getVersionBody(versionUnreleased, "12345"), + expectedID: "12345", + }, + { + eventName: versionCreated, + body: getVersionBody(versionCreated, "12345"), + expectedID: "12345", + }, + { + eventName: versionUpdated, + body: getVersionBody(versionUpdated, "12345"), + expectedID: "12345", + }, + { + eventName: versionDeleted, + body: getVersionBody(versionDeleted, "12345"), + expectedID: "12345", + expectedOperation: entities.Delete, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("invoke webhook with %s event", tc.eventName), func(t *testing.T) { + defer s.ResetCalls() + + req := getWebhookRequest(bytes.NewBufferString(tc.body)) + + resp, err := app.Test(req) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Eventually(t, func() bool { + return len(s.Calls()) == 1 + }, 1*time.Second, 10*time.Millisecond) + require.Equal(t, fakewriter.Call{ + Operation: tc.expectedOperation, + Data: &entities.Event{ + ID: tc.expectedID, + Type: tc.eventName, + OperationType: tc.expectedOperation, + OriginalRaw: []byte(tc.body), + }, + }, s.Calls().LastCall()) + }) + } }) } + +func getWebhookRequest(body *bytes.Buffer) *http.Request { + req := httptest.NewRequest(http.MethodPost, "/webhook", body) + hmac := getHMACValidationHeader("SECRET_VALUE", body.Bytes()) + req.Header.Add(authHeaderName, fmt.Sprintf("sha256=%s", hmac)) + return req +} + +func getHMACValidationHeader(secret string, body []byte) string { + hasher := hmac.New(sha256.New, []byte(secret)) + hasher.Write(body) + return hex.EncodeToString(hasher.Sum(nil)) +} + +func getIssueBody(eventName, id string) string { + return fmt.Sprintf(`{"webhookEvent":"%s","issue": {"id":%s,"key": "TEST-123"}}`, eventName, id) +} + +func getIssueLinkBody(eventName, id string) string { + return fmt.Sprintf(`{"webhookEvent":"%s","issueLink": {"id":%s}}`, eventName, id) +} + +func getProjectBody(eventName, id string) string { + return fmt.Sprintf(`{"webhookEvent":"%s","project": {"id":%s}}`, eventName, id) +} + +func getVersionBody(eventName, id string) string { + return fmt.Sprintf(`{"webhookEvent":"%s","version": {"id":%s}}`, eventName, id) +} diff --git a/internal/sources/mia-platform-console/console.go b/internal/sources/mia-platform-console/console.go new file mode 100644 index 0000000..62fb3ac --- /dev/null +++ b/internal/sources/mia-platform-console/console.go @@ -0,0 +1,87 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package console + +import ( + "context" + + "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/pipeline" + "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + + swagger "github.com/davidebianchi/gswagger" + "github.com/gofiber/fiber/v2" +) + +const ( + defaultWebhookPath = "/console/webhook" + authHeaderName = "X-Mia-Signature" + + webhookEventPath = "eventName" +) + +type Config struct { + Authentication ValidationConfig `json:"authentication"` + WebhookPath string `json:"webhookPath"` +} + +func (c *Config) withDefault() *Config { + if c.WebhookPath == "" { + c.WebhookPath = defaultWebhookPath + } + if c.Authentication.HeaderName == "" { + c.Authentication.HeaderName = authHeaderName + } + + return c +} + +func (c *Config) Validate() error { + c.withDefault() + + return nil +} + +func (c *Config) getWebhookConfig() (*webhook.Configuration, error) { + webhookConfig := &webhook.Configuration{ + WebhookPath: c.WebhookPath, + Authentication: c.Authentication, + Events: &DefaultSupportedEvents, + } + if err := webhookConfig.Validate(); err != nil { + return nil, err + } + return webhookConfig, nil +} + +func AddSourceToRouter( + ctx context.Context, + cfg config.GenericConfig, + pg *pipeline.Group, + router *swagger.Router[fiber.Handler, fiber.Router], +) error { + consoleConfig, err := config.GetConfig[*Config](cfg) + if err != nil { + return err + } + + webhookConfig, err := consoleConfig.getWebhookConfig() + if err != nil { + return err + } + + return webhook.SetupService(ctx, router, webhookConfig, pg) +} diff --git a/internal/sources/mia-platform-console/console_test.go b/internal/sources/mia-platform-console/console_test.go new file mode 100644 index 0000000..4640ad2 --- /dev/null +++ b/internal/sources/mia-platform-console/console_test.go @@ -0,0 +1,273 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package console + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/entities" + "github.com/mia-platform/integration-connector-agent/internal/pipeline" + "github.com/mia-platform/integration-connector-agent/internal/processors" + fakewriter "github.com/mia-platform/integration-connector-agent/internal/sinks/fake" + "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + "github.com/mia-platform/integration-connector-agent/internal/testutils" + "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/require" +) + +func TestValidateConfig(t *testing.T) { + testCases := map[string]struct { + config *Config + + expectedConfig *Config + expectedError error + }{ + "with default": { + config: &Config{}, + expectedConfig: &Config{ + WebhookPath: defaultWebhookPath, + Authentication: ValidationConfig{ + HeaderName: authHeaderName, + }, + }, + }, + "with custom values": { + config: &Config{ + WebhookPath: "/custom/webhook", + Authentication: ValidationConfig{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + }, + expectedConfig: &Config{ + WebhookPath: "/custom/webhook", + Authentication: ValidationConfig{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + err := tc.config.Validate() + if tc.expectedError != nil { + require.EqualError(t, err, tc.expectedError.Error()) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectedConfig, tc.config) + }) + } + + t.Run("unmarshal config", func(t *testing.T) { + rawConfig, err := os.ReadFile("testdata/config.json") + require.NoError(t, err) + + actual := &Config{} + require.NoError(t, json.Unmarshal(rawConfig, actual)) + require.NoError(t, actual.Validate()) + + require.Equal(t, &Config{ + WebhookPath: "/webhook", + Authentication: ValidationConfig{ + HeaderName: authHeaderName, + Secret: config.SecretSource("SECRET_VALUE"), + }, + }, actual) + }) +} + +func TestGetWebhookConfig(t *testing.T) { + testCases := map[string]struct { + config *Config + + expectedConfig *webhook.Configuration + expectedError string + }{ + "valid config without authentication": { + config: &Config{ + WebhookPath: "/webhook", + }, + expectedConfig: &webhook.Configuration{ + WebhookPath: "/webhook", + Authentication: ValidationConfig{}, + Events: &DefaultSupportedEvents, + }, + }, + "valid config with authentication": { + config: &Config{ + WebhookPath: "/webhook", + Authentication: ValidationConfig{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + }, + expectedConfig: &webhook.Configuration{ + WebhookPath: "/webhook", + Authentication: ValidationConfig{ + HeaderName: "X-Custom-Header", + Secret: config.SecretSource("secret"), + }, + Events: &DefaultSupportedEvents, + }, + }, + "error on invalid config": { + config: &Config{}, + expectedError: webhook.ErrWebhookPathRequired.Error(), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + webhookConfig, err := tc.config.getWebhookConfig() + + if tc.expectedError != "" { + require.EqualError(t, err, tc.expectedError) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectedConfig, webhookConfig) + }) + } +} + +func TestAddSourceToRouter(t *testing.T) { + logger, _ := test.NewNullLogger() + + t.Run("setup webhook", func(t *testing.T) { + ctx := context.Background() + + rawConfig, err := os.ReadFile("testdata/config.json") + require.NoError(t, err) + cfg := config.GenericConfig{} + require.NoError(t, json.Unmarshal(rawConfig, &cfg)) + + app, router := testutils.GetTestRouter(t) + + proc := &processors.Processors{} + s := fakewriter.New(nil) + p1, err := pipeline.New(logger, proc, s) + require.NoError(t, err) + + pg := pipeline.NewGroup(logger, p1) + + err = AddSourceToRouter(ctx, cfg, pg, router) + require.NoError(t, err) + + tenantID := "my-tenant-id" + projectID := "my-prj-id" + + testCases := []struct { + eventName string + body string + + expectedID string + expectedOperation entities.Operation + }{ + { + eventName: projectCreatedEvent, + body: fmt.Sprintf(`{"eventName":"%s","payload":{"projectId":"%s","tenantId":"%s"}}`, projectCreatedEvent, projectID, tenantID), + + expectedID: getFieldID([]pkFields{ + {"tenantId", tenantID}, + {"projectId", projectID}, + }), + }, + { + eventName: serviceCreatedEvent, + body: fmt.Sprintf(`{"eventName":"%s","payload":{"projectId":"%s","tenantId":"%s","serviceName":"my-service"}}`, serviceCreatedEvent, projectID, tenantID), + + expectedID: getFieldID([]pkFields{ + {"tenantId", tenantID}, + {"projectId", projectID}, + {"serviceName", "my-service"}, + }), + }, + { + eventName: configurationSavedEvent, + body: fmt.Sprintf(`{"eventName":"%s","payload":{"tenantId":"%s","projectId":"%s","revisionName":"my-revision"}}`, configurationSavedEvent, tenantID, projectID), + + expectedID: getFieldID([]pkFields{ + {"tenantId", tenantID}, + {"projectId", projectID}, + {"revisionName", "my-revision"}, + }), + }, + { + eventName: tagCreatedEvent, + body: fmt.Sprintf(`{"eventName":"%s","payload":{"projectId":"%s","tenantId":"%s","tagName":"my-tag"}}`, tagCreatedEvent, projectID, tenantID), + + expectedID: getFieldID([]pkFields{ + {"tenantId", tenantID}, + {"projectId", projectID}, + {"tagName", "my-tag"}, + }), + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("invoke webhook with %s event", tc.eventName), func(t *testing.T) { + defer s.ResetCalls() + + req := getWebhookRequest(bytes.NewBufferString(tc.body)) + + resp, err := app.Test(req) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Eventually(t, func() bool { + return len(s.Calls()) == 1 + }, 1*time.Second, 10*time.Millisecond) + require.Equal(t, fakewriter.Call{ + Operation: tc.expectedOperation, + Data: &entities.Event{ + ID: tc.expectedID, + Type: tc.eventName, + OperationType: tc.expectedOperation, + OriginalRaw: []byte(tc.body), + }, + }, s.Calls().LastCall()) + }) + } + }) +} + +func getWebhookRequest(body *bytes.Buffer) *http.Request { + req := httptest.NewRequest(http.MethodPost, "/webhook", body) + hmac := getHMACValidationHeader("SECRET_VALUE", body.Bytes()) + req.Header.Add(authHeaderName, fmt.Sprintf("sha256=%s", hmac)) + return req +} + +func getHMACValidationHeader(secret string, body []byte) string { + hasher := sha256.New() + hasher.Write(body) + hasher.Write([]byte(secret)) + return hex.EncodeToString(hasher.Sum(nil)) +} diff --git a/internal/sources/mia-platform-console/events.go b/internal/sources/mia-platform-console/events.go new file mode 100644 index 0000000..3fa2585 --- /dev/null +++ b/internal/sources/mia-platform-console/events.go @@ -0,0 +1,125 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package console + +import ( + "fmt" + "strings" + + "github.com/mia-platform/integration-connector-agent/internal/entities" + "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + + "github.com/tidwall/gjson" +) + +const ( + projectCreatedEvent = "project_created" + serviceCreatedEvent = "service_created" + tagCreatedEvent = "tag_created" + configurationSavedEvent = "configuration_saved" + + tenantIDEventPath = "payload.tenantId" + projectIDEventPath = "payload.projectId" + serviceNameEventPath = "payload.serviceName" + tagNameEventPath = "payload.tagName" + revisionNameEventPath = "payload.revisionName" + + tenantIDKey = "tenantId" + projectIDKey = "projectId" + serviceNameKey = "serviceName" + revisionNameKey = "revisionName" +) + +var DefaultSupportedEvents = webhook.Events{ + Supported: map[string]webhook.Event{ + projectCreatedEvent: { + Operation: entities.Write, + GetFieldID: func(parsedData gjson.Result) string { + projectID := parsedData.Get(projectIDEventPath).String() + tenantID := parsedData.Get(tenantIDEventPath).String() + + return getFieldID([]pkFields{ + {key: tenantIDKey, value: tenantID}, + {key: projectIDKey, value: projectID}, + }) + }, + }, + serviceCreatedEvent: { + Operation: entities.Write, + GetFieldID: func(parsedData gjson.Result) string { + projectID := parsedData.Get(projectIDEventPath).String() + serviceName := parsedData.Get(serviceNameEventPath).String() + tenantID := parsedData.Get(tenantIDEventPath).String() + + return getFieldID([]pkFields{ + {key: tenantIDKey, value: tenantID}, + {key: projectIDKey, value: projectID}, + {key: serviceNameKey, value: serviceName}, + }) + }, + }, + configurationSavedEvent: { + Operation: entities.Write, + GetFieldID: func(parsedData gjson.Result) string { + tenantID := parsedData.Get(tenantIDEventPath).String() + projectID := parsedData.Get(projectIDEventPath).String() + revisionName := parsedData.Get(revisionNameEventPath).String() + + return getFieldID([]pkFields{ + {key: tenantIDKey, value: tenantID}, + {key: projectIDKey, value: projectID}, + {key: revisionNameKey, value: revisionName}, + }) + }, + }, + tagCreatedEvent: { + Operation: entities.Write, + GetFieldID: func(parsedData gjson.Result) string { + tenantID := parsedData.Get(tenantIDEventPath).String() + projectID := parsedData.Get(projectIDEventPath).String() + tagName := parsedData.Get(tagNameEventPath).String() + + return getFieldID([]pkFields{ + {key: tenantIDKey, value: tenantID}, + {key: projectIDKey, value: projectID}, + {key: "tagName", value: tagName}, + }) + }, + }, + }, + EventTypeFieldPath: webhookEventPath, +} + +type pkFields struct { + key string + value string +} + +func getFieldID(keyMap []pkFields) string { + keys := []string{} + for _, kv := range keyMap { + keys = append(keys, fmt.Sprintf("%s:\"%s\"", kv.key, kv.value)) + } + key := strings.Join(keys, ",") + + return key + + // TODO: it's not easy to debug with hash. Should I plain string? + // hasher := sha256.New() + // hasher.Write([]byte(key)) + + // return hex.EncodeToString(hasher.Sum(nil)) +} diff --git a/internal/sources/mia-platform-console/testdata/config.json b/internal/sources/mia-platform-console/testdata/config.json new file mode 100644 index 0000000..e0fda86 --- /dev/null +++ b/internal/sources/mia-platform-console/testdata/config.json @@ -0,0 +1,9 @@ +{ + "type": "console", + "authentication": { + "secret": { + "fromFile": "testdata/secret" + } + }, + "webhookPath": "/webhook" +} diff --git a/internal/sources/mia-platform-console/testdata/secret b/internal/sources/mia-platform-console/testdata/secret new file mode 100644 index 0000000..87ff38f --- /dev/null +++ b/internal/sources/mia-platform-console/testdata/secret @@ -0,0 +1 @@ +SECRET_VALUE diff --git a/internal/sources/mia-platform-console/validation.go b/internal/sources/mia-platform-console/validation.go new file mode 100644 index 0000000..9464ef4 --- /dev/null +++ b/internal/sources/mia-platform-console/validation.go @@ -0,0 +1,68 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package console + +import ( + "crypto/sha256" + "errors" + "fmt" + "reflect" + "strings" + + "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" +) + +type ValidationConfig struct { + Secret config.SecretSource `json:"secret"` + HeaderName string `json:"-"` +} + +// CheckSignature will read the webhook signature header and the given secret for validating the webhook +// payload. +func (h ValidationConfig) CheckSignature(req webhook.ValidatingRequest) error { + if req == nil || reflect.ValueOf(req).IsNil() { + return fmt.Errorf("request is nil") + } + secret := h.Secret.String() + if secret != "" && h.HeaderName == "" { + return fmt.Errorf("%s: secret is set but headerName not present", webhook.InvalidWebhookAuthenticationConfig) + } + + headerValues, err := webhook.GetHeaderValues(req, h.HeaderName, secret) + if err != nil { + return err + } + if headerValues == nil { + return nil + } + + signature, _ := strings.CutPrefix(headerValues[0], "sha256=") + if !validateBody(req.Body(), secret, signature) { + return errors.New(webhook.InvalidSignatureError) + } + + return nil +} + +// validateBody will generate an hmac encoding of bodyData using secret, and than compare it with the expectedSignature +func validateBody(bodyData []byte, secret, expectedSignature string) bool { + hasher := sha256.New() + hasher.Write(bodyData) + hasher.Write([]byte(secret)) + generatedHash := hasher.Sum(nil) + return fmt.Sprintf("%x", generatedHash) == expectedSignature +} diff --git a/internal/sources/mia-platform-console/validation_test.go b/internal/sources/mia-platform-console/validation_test.go new file mode 100644 index 0000000..feee712 --- /dev/null +++ b/internal/sources/mia-platform-console/validation_test.go @@ -0,0 +1,133 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package console + +import ( + "fmt" + "testing" + + "github.com/mia-platform/integration-connector-agent/internal/config" + "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + "github.com/stretchr/testify/require" +) + +func TestCheckConsoleSignature(t *testing.T) { + t.Parallel() + + signatureHeader := "X-Hub-Signature" + + tests := map[string]struct { + request *fakeValidatingRequest + authentication ValidationConfig + expectedErr string + }{ + "no request return error": { + authentication: ValidationConfig{}, + expectedErr: "request is nil", + }, + "no header and no secret return no error": { + authentication: ValidationConfig{}, + request: &fakeValidatingRequest{}, + }, + "missing secret return error": { + authentication: ValidationConfig{ + HeaderName: signatureHeader, + }, + request: &fakeValidatingRequest{ + headers: map[string][]string{ + signatureHeader: {"signature"}, + }, + }, + expectedErr: webhook.SignatureHeaderButNoSecretError, + }, + "missing configured header but secret present returns error": { + authentication: ValidationConfig{ + Secret: config.SecretSource("secret"), + }, + request: &fakeValidatingRequest{}, + expectedErr: fmt.Sprintf("%s: secret is set but headerName not present", webhook.InvalidWebhookAuthenticationConfig), + }, + "missing header return error": { + authentication: ValidationConfig{ + HeaderName: signatureHeader, + Secret: "secret", + }, + request: &fakeValidatingRequest{}, + expectedErr: webhook.NoSignatureHeaderButSecretError, + }, + "multiple header return error": { + authentication: ValidationConfig{ + HeaderName: signatureHeader, + Secret: "secret", + }, + request: &fakeValidatingRequest{ + headers: map[string][]string{ + signatureHeader: {"signature", "other"}, + }, + }, + expectedErr: webhook.MultipleSignatureHeadersError, + }, + "valid signature return nil": { + authentication: ValidationConfig{ + HeaderName: signatureHeader, + Secret: "It's a Secret to Everybody", + }, + request: &fakeValidatingRequest{ + body: []byte("Hello World!"), + headers: map[string][]string{ + signatureHeader: {"sha256=b738052486cd876b13b5404a45479bd8caca05e5267c10d8436f1570547e3056"}, + }, + }, + }, + "invalid signature return error": { + authentication: ValidationConfig{ + HeaderName: signatureHeader, + Secret: "It's a Secret to Everybody", + }, + request: &fakeValidatingRequest{ + body: []byte("tampered body"), + headers: map[string][]string{ + signatureHeader: {"sha256=b738052486cd876b13b5404a45479bd8caca05e5267c10d8436f1570547e3056"}, + }, + }, + expectedErr: webhook.InvalidSignatureError, + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + err := test.authentication.CheckSignature(test.request) + if test.expectedErr != "" { + require.EqualError(t, err, test.expectedErr) + return + } + require.NoError(t, err) + }) + } +} + +type fakeValidatingRequest struct { + headers map[string][]string + body []byte +} + +func (r fakeValidatingRequest) GetReqHeaders() map[string][]string { + return r.headers +} + +func (r fakeValidatingRequest) Body() []byte { + return r.body +} diff --git a/internal/sources/webhook/config.go b/internal/sources/webhook/config.go index 50492c4..a0ceb02 100644 --- a/internal/sources/webhook/config.go +++ b/internal/sources/webhook/config.go @@ -16,9 +16,14 @@ package webhook import ( + "errors" "fmt" ) +var ( + ErrWebhookPathRequired = errors.New("webhook path is required") +) + type ValidatingRequest interface { GetReqHeaders() map[string][]string Body() []byte @@ -39,7 +44,7 @@ type Configuration struct { func (c *Configuration) Validate() error { if c.WebhookPath == "" { - return fmt.Errorf("webhook path is empty") + return ErrWebhookPathRequired } if c.Events == nil { diff --git a/internal/sources/webhook/config_test.go b/internal/sources/webhook/config_test.go index 2c58a7e..ff7c59e 100644 --- a/internal/sources/webhook/config_test.go +++ b/internal/sources/webhook/config_test.go @@ -28,7 +28,7 @@ func TestValidateConfiguration(t *testing.T) { expectedErr string }{ "empty configuration": { - expectedErr: "webhook path is empty", + expectedErr: "webhook path is required", }, "empty events": { config: Configuration{ diff --git a/internal/sources/webhook/events.go b/internal/sources/webhook/events.go index f6a7f67..498db41 100644 --- a/internal/sources/webhook/events.go +++ b/internal/sources/webhook/events.go @@ -35,7 +35,16 @@ type Events struct { type Event struct { Operation entities.Operation - FieldID string + // TODO: improve to use on from FieldID and GetFieldID. Maybe creating a factory function? + FieldID string + GetFieldID func(parsedData gjson.Result) string +} + +func (e *Event) GetID(parsedData gjson.Result) string { + if e.GetFieldID != nil { + return e.GetFieldID(parsedData) + } + return parsedData.Get(e.FieldID).String() } func (e *Events) getPipelineEvent(logger *logrus.Entry, rawData []byte) (entities.PipelineEvent, error) { @@ -51,7 +60,7 @@ func (e *Events) getPipelineEvent(logger *logrus.Entry, rawData []byte) (entitie return nil, fmt.Errorf("%w: %s", ErrUnsupportedWebhookEvent, webhookEvent) } - id := parsed.Get(event.FieldID).String() + id := event.GetID(parsed) if id == "" { logger.WithFields(logrus.Fields{ "webhookEvent": webhookEvent, diff --git a/internal/sources/webhook/events_test.go b/internal/sources/webhook/events_test.go index 12b7b7e..ebf9cf5 100644 --- a/internal/sources/webhook/events_test.go +++ b/internal/sources/webhook/events_test.go @@ -22,6 +22,7 @@ import ( "github.com/mia-platform/integration-connector-agent/internal/entities" "github.com/sirupsen/logrus" "github.com/sirupsen/logrus/hooks/test" + "github.com/tidwall/gjson" "github.com/stretchr/testify/require" ) @@ -90,6 +91,23 @@ func TestEvent(t *testing.T) { expectError: fmt.Sprintf("%s: %s", ErrUnsupportedWebhookEvent, "unsupported"), expectedType: "unsupported", }, + "with custom GetFieldID": { + rawData: `{"issue":{"tag":"my-id","projectId":"prj-1","parentId":"my-parent-id"},"webhookEvent": "my-event"}`, + events: &Events{ + Supported: map[string]Event{ + "my-event": { + GetFieldID: func(parsedData gjson.Result) string { + return fmt.Sprintf("parent:\"%s\"-project:\"%s\"", parsedData.Get("issue.parentId").String(), parsedData.Get("issue.projectId").String()) + }, + }, + }, + EventTypeFieldPath: "webhookEvent", + }, + + expectedID: `parent:"my-parent-id"-project:"prj-1"`, + expectedType: "my-event", + expectedOperationType: entities.Write, + }, } for name, tc := range testCases { diff --git a/internal/sources/webhook/hmac.go b/internal/sources/webhook/hmac.go index 05b451c..8df3ca8 100644 --- a/internal/sources/webhook/hmac.go +++ b/internal/sources/webhook/hmac.go @@ -27,11 +27,11 @@ import ( ) const ( - invalidWebhookAuthenticationConfig = "invalid webhook authentication configuration" - signatureHeaderButNoSecretError = "secret not configured for validating webhook signature" - noSignatureHeaderButSecretError = "missing webhook signature" - multipleSignatureHeadersError = "multiple signature headers found" - invalidSignatureError = "invalid signature in request" + InvalidWebhookAuthenticationConfig = "invalid webhook authentication configuration" + SignatureHeaderButNoSecretError = "secret not configured for validating webhook signature" + NoSignatureHeaderButSecretError = "missing webhook signature" + MultipleSignatureHeadersError = "multiple signature headers found" + InvalidSignatureError = "invalid signature in request" ) type HMAC struct { @@ -44,28 +44,24 @@ type HMAC struct { // other is not present. func (h HMAC) CheckSignature(req ValidatingRequest) error { if req == nil { - return fmt.Errorf("%s: request is nil", invalidWebhookAuthenticationConfig) + return fmt.Errorf("%s: request is nil", InvalidWebhookAuthenticationConfig) } secret := h.Secret.String() if secret != "" && h.HeaderName == "" { - return fmt.Errorf("%s: secret is set but headerName not present", invalidWebhookAuthenticationConfig) + return fmt.Errorf("%s: secret is set but headerName not present", InvalidWebhookAuthenticationConfig) } - headerValues := req.GetReqHeaders()[h.HeaderName] - switch { - case len(headerValues) == 0 && len(secret) == 0: + headerValues, err := GetHeaderValues(req, h.HeaderName, secret) + if err != nil { + return err + } + if headerValues == nil { return nil - case len(headerValues) == 0 && len(secret) != 0: - return errors.New(noSignatureHeaderButSecretError) - case len(headerValues) != 0 && len(secret) == 0: - return errors.New(signatureHeaderButNoSecretError) - case len(headerValues) > 1: - return errors.New(multipleSignatureHeadersError) } signature, _ := strings.CutPrefix(headerValues[0], "sha256=") if !validateBody(req.Body(), secret, signature) { - return errors.New(invalidSignatureError) + return errors.New(InvalidSignatureError) } return nil @@ -84,3 +80,19 @@ func validateBody(bodyData []byte, secret, expectedSignature string) bool { return hmac.Equal(generatedMAC, expectedMac) } + +func GetHeaderValues(req ValidatingRequest, headerName, secret string) ([]string, error) { + headerValues := req.GetReqHeaders()[headerName] + switch { + case len(headerValues) == 0 && len(secret) == 0: + return nil, nil + case len(headerValues) == 0 && len(secret) != 0: + return nil, errors.New(NoSignatureHeaderButSecretError) + case len(headerValues) != 0 && len(secret) == 0: + return nil, errors.New(SignatureHeaderButNoSecretError) + case len(headerValues) > 1: + return nil, errors.New(MultipleSignatureHeadersError) + } + + return headerValues, nil +} diff --git a/internal/sources/webhook/hmac_test.go b/internal/sources/webhook/hmac_test.go index fd13326..5748277 100644 --- a/internal/sources/webhook/hmac_test.go +++ b/internal/sources/webhook/hmac_test.go @@ -44,14 +44,14 @@ func TestCheckHMACSignature(t *testing.T) { webhookSignatureHeader: {"signature"}, }, }, - expectedErr: errors.New(signatureHeaderButNoSecretError), + expectedErr: errors.New(SignatureHeaderButNoSecretError), }, "missing header return error": { authentication: &HMAC{ HeaderName: webhookSignatureHeader, Secret: "secret", }, - expectedErr: errors.New(noSignatureHeaderButSecretError), + expectedErr: errors.New(NoSignatureHeaderButSecretError), }, "multiple header return error": { authentication: &HMAC{ @@ -63,7 +63,7 @@ func TestCheckHMACSignature(t *testing.T) { webhookSignatureHeader: {"signature", "other"}, }, }, - expectedErr: errors.New(multipleSignatureHeadersError), + expectedErr: errors.New(MultipleSignatureHeadersError), }, "valid signature return nil": { authentication: &HMAC{ @@ -88,7 +88,7 @@ func TestCheckHMACSignature(t *testing.T) { webhookSignatureHeader: {"sha256=a4771c39fbe90f317c7824e83ddef3caae9cb3d976c214ace1f2937e133263c9"}, }, }, - expectedErr: errors.New(invalidSignatureError), + expectedErr: errors.New(InvalidSignatureError), }, } @@ -100,8 +100,6 @@ func TestCheckHMACSignature(t *testing.T) { } } -var _ ValidatingRequest = fakeValidatingRequest{} - type fakeValidatingRequest struct { headers map[string][]string body []byte diff --git a/internal/sources/webhook/service_test.go b/internal/sources/webhook/service_test.go index 9b806b0..914296a 100644 --- a/internal/sources/webhook/service_test.go +++ b/internal/sources/webhook/service_test.go @@ -78,7 +78,7 @@ func TestSetupServiceWithConfig(t *testing.T) { require.NoError(t, json.NewDecoder(body).Decode(&expectedBody)) require.Equal(t, utils.HTTPError{ Error: "Validation Error", - Message: noSignatureHeaderButSecretError, + Message: NoSignatureHeaderButSecretError, }, expectedBody) }, }, From 30f5c846676bd26c877f77076bf6f7c0108626e3 Mon Sep 17 00:00:00 2001 From: Davide Bianchi <10374360+davidebianchi@users.noreply.github.com> Date: Mon, 9 Dec 2024 10:26:22 +0100 Subject: [PATCH 3/4] fix: lint --- .golangci.yaml | 3 -- internal/sources/jira/jira_test.go | 61 +++++++++++++++--------------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index f2d7afb..a13c9a6 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -40,7 +40,6 @@ linters: - tenv - thelper - unconvert - - unparam - unused - usestdlibvars - whitespace @@ -59,8 +58,6 @@ linters-settings: yaml: camel tenv: all: true - unparam: - check-exported: false mnd: checks: - case diff --git a/internal/sources/jira/jira_test.go b/internal/sources/jira/jira_test.go index c467bb0..d491d86 100644 --- a/internal/sources/jira/jira_test.go +++ b/internal/sources/jira/jira_test.go @@ -175,6 +175,7 @@ func TestAddSourceToRouter(t *testing.T) { pg := pipeline.NewGroup(logger, p1) + id := "12345" err = AddSourceToRouter(ctx, cfg, pg, router) require.NoError(t, err) @@ -187,82 +188,82 @@ func TestAddSourceToRouter(t *testing.T) { }{ { eventName: issueCreated, - body: getIssueBody(issueCreated, "12345"), - expectedID: "12345", + body: getIssueBody(issueCreated, id), + expectedID: id, }, { eventName: issueUpdated, - body: getIssueBody(issueUpdated, "12345"), - expectedID: "12345", + body: getIssueBody(issueUpdated, id), + expectedID: id, }, { eventName: issueDeleted, - body: getIssueBody(issueDeleted, "12345"), - expectedID: "12345", + body: getIssueBody(issueDeleted, id), + expectedID: id, expectedOperation: entities.Delete, }, { eventName: issueLinkCreated, - body: getIssueLinkBody(issueLinkCreated, "12345"), - expectedID: "12345", + body: getIssueLinkBody(issueLinkCreated, id), + expectedID: id, }, { eventName: issueLinkDeleted, - body: getIssueLinkBody(issueLinkDeleted, "12345"), - expectedID: "12345", + body: getIssueLinkBody(issueLinkDeleted, id), + expectedID: id, expectedOperation: entities.Delete, }, { eventName: projectCreated, - body: getProjectBody(projectCreated, "12345"), - expectedID: "12345", + body: getProjectBody(projectCreated, id), + expectedID: id, }, { eventName: projectUpdated, - body: getProjectBody(projectUpdated, "12345"), - expectedID: "12345", + body: getProjectBody(projectUpdated, id), + expectedID: id, }, { eventName: projectDeleted, - body: getProjectBody(projectDeleted, "12345"), - expectedID: "12345", + body: getProjectBody(projectDeleted, id), + expectedID: id, expectedOperation: entities.Delete, }, { eventName: projectSoftDeleted, - body: getProjectBody(projectSoftDeleted, "12345"), - expectedID: "12345", + body: getProjectBody(projectSoftDeleted, id), + expectedID: id, expectedOperation: entities.Delete, }, { eventName: projectRestoredDeleted, - body: getProjectBody(projectRestoredDeleted, "12345"), - expectedID: "12345", + body: getProjectBody(projectRestoredDeleted, id), + expectedID: id, }, { eventName: versionReleased, - body: getVersionBody(versionReleased, "12345"), - expectedID: "12345", + body: getVersionBody(versionReleased, id), + expectedID: id, }, { eventName: versionUnreleased, - body: getVersionBody(versionUnreleased, "12345"), - expectedID: "12345", + body: getVersionBody(versionUnreleased, id), + expectedID: id, }, { eventName: versionCreated, - body: getVersionBody(versionCreated, "12345"), - expectedID: "12345", + body: getVersionBody(versionCreated, id), + expectedID: id, }, { eventName: versionUpdated, - body: getVersionBody(versionUpdated, "12345"), - expectedID: "12345", + body: getVersionBody(versionUpdated, id), + expectedID: id, }, { eventName: versionDeleted, - body: getVersionBody(versionDeleted, "12345"), - expectedID: "12345", + body: getVersionBody(versionDeleted, id), + expectedID: id, expectedOperation: entities.Delete, }, } From 9321293cc6be0a7b16a3197babce9d9ca4b19389 Mon Sep 17 00:00:00 2001 From: Davide Bianchi <10374360+davidebianchi@users.noreply.github.com> Date: Tue, 7 Jan 2025 12:03:35 +0100 Subject: [PATCH 4/4] feat: add IsNil utils --- internal/pipeline/pipeline.go | 9 ++--- .../mia-platform-console/console_test.go | 1 + .../mia-platform-console/validation.go | 4 +-- internal/utils/utils.go | 23 ++++++++++++ internal/utils/utils_test.go | 36 +++++++++++++++++++ 5 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 internal/utils/utils.go create mode 100644 internal/utils/utils_test.go diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index f273eea..d8050e4 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -18,12 +18,12 @@ package pipeline import ( "context" "errors" - "reflect" "github.com/mia-platform/integration-connector-agent/internal/entities" "github.com/mia-platform/integration-connector-agent/internal/processors" "github.com/mia-platform/integration-connector-agent/internal/processors/filter" "github.com/mia-platform/integration-connector-agent/internal/sinks" + "github.com/mia-platform/integration-connector-agent/internal/utils" "github.com/sirupsen/logrus" ) @@ -45,7 +45,7 @@ func (p Pipeline) AddMessage(data entities.PipelineEvent) { } func (p Pipeline) Start(ctx context.Context) error { - if isNil(p.sinks) { + if utils.IsNil(p.sinks) { return ErrWriterNotDefined } @@ -112,8 +112,3 @@ func New(logger *logrus.Logger, p *processors.Processors, sinks sinks.Sink[entit return pipeline, nil } - -// TODO: set as utils and reuse it in CheckSignature -func isNil(i any) bool { - return i == nil || (reflect.ValueOf(i).Kind() == reflect.Ptr && reflect.ValueOf(i).IsNil()) -} diff --git a/internal/sources/mia-platform-console/console_test.go b/internal/sources/mia-platform-console/console_test.go index 4640ad2..948d466 100644 --- a/internal/sources/mia-platform-console/console_test.go +++ b/internal/sources/mia-platform-console/console_test.go @@ -35,6 +35,7 @@ import ( fakewriter "github.com/mia-platform/integration-connector-agent/internal/sinks/fake" "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" "github.com/mia-platform/integration-connector-agent/internal/testutils" + "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" ) diff --git a/internal/sources/mia-platform-console/validation.go b/internal/sources/mia-platform-console/validation.go index 9464ef4..c1a76e4 100644 --- a/internal/sources/mia-platform-console/validation.go +++ b/internal/sources/mia-platform-console/validation.go @@ -19,11 +19,11 @@ import ( "crypto/sha256" "errors" "fmt" - "reflect" "strings" "github.com/mia-platform/integration-connector-agent/internal/config" "github.com/mia-platform/integration-connector-agent/internal/sources/webhook" + "github.com/mia-platform/integration-connector-agent/internal/utils" ) type ValidationConfig struct { @@ -34,7 +34,7 @@ type ValidationConfig struct { // CheckSignature will read the webhook signature header and the given secret for validating the webhook // payload. func (h ValidationConfig) CheckSignature(req webhook.ValidatingRequest) error { - if req == nil || reflect.ValueOf(req).IsNil() { + if utils.IsNil(req) { return fmt.Errorf("request is nil") } secret := h.Secret.String() diff --git a/internal/utils/utils.go b/internal/utils/utils.go new file mode 100644 index 0000000..b46ecb4 --- /dev/null +++ b/internal/utils/utils.go @@ -0,0 +1,23 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +import "reflect" + +func IsNil(i any) bool { + defer func() { recover() }() //nolint:errcheck + return i == nil || reflect.ValueOf(i).IsNil() +} diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go new file mode 100644 index 0000000..9e8c08a --- /dev/null +++ b/internal/utils/utils_test.go @@ -0,0 +1,36 @@ +// Copyright Mia srl +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIsNil(t *testing.T) { + var i interface{} + require.True(t, IsNil(i)) + + type fooType struct{} + var foo *fooType + require.True(t, IsNil(foo)) + + require.False(t, IsNil(fooType{})) + + var s []string + require.True(t, IsNil(s)) +}