From f9f917ba05e104d380d424cf080490f99686fac9 Mon Sep 17 00:00:00 2001 From: mgoeppe Date: Mon, 23 Jan 2023 17:43:32 +0100 Subject: [PATCH] aligned code enterprises,organizations and repositories and fixed sql tests --- database/sql/enterprise.go | 23 +++++------ database/sql/enterprise_test.go | 65 ++++++++++++++---------------- database/sql/organizations.go | 46 ++++++++++----------- database/sql/organizations_test.go | 17 +++----- database/sql/repositories.go | 52 ++++++++++-------------- database/sql/repositories_test.go | 26 +++++------- database/sql/util.go | 42 +++++++++++++------ errors/errors.go | 14 +++++++ runner/runner.go | 6 +-- 9 files changed, 147 insertions(+), 144 deletions(-) diff --git a/database/sql/enterprise.go b/database/sql/enterprise.go index 3df7f2f1..79afa39b 100644 --- a/database/sql/enterprise.go +++ b/database/sql/enterprise.go @@ -13,13 +13,12 @@ import ( ) func (s *sqlDatabase) CreateEnterprise(ctx context.Context, name, credentialsName, webhookSecret string) (params.Enterprise, error) { - secret := []byte{} - var err error - if webhookSecret != "" { - secret, err = util.Aes256EncodeString(webhookSecret, s.cfg.Passphrase) - if err != nil { - return params.Enterprise{}, errors.Wrap(err, "encoding secret") - } + if webhookSecret == "" { + return params.Enterprise{}, errors.New("creating enterprise: missing secret") + } + secret, err := util.Aes256EncodeString(webhookSecret, s.cfg.Passphrase) + if err != nil { + return params.Enterprise{}, errors.Wrap(err, "encoding secret") } newEnterprise := Enterprise{ Name: name, @@ -32,7 +31,7 @@ func (s *sqlDatabase) CreateEnterprise(ctx context.Context, name, credentialsNam return params.Enterprise{}, errors.Wrap(q.Error, "creating enterprise") } - param, err := s.sqlToCommonEnterprise(newEnterprise, s.cfg) + param, err := s.sqlToCommonEnterprise(newEnterprise) if err != nil { return params.Enterprise{}, errors.Wrap(err, "creating enterprise") } @@ -46,7 +45,7 @@ func (s *sqlDatabase) GetEnterprise(ctx context.Context, name string) (params.En return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } - param, err := s.sqlToCommonEnterprise(enterprise, s.cfg) + param, err := s.sqlToCommonEnterprise(enterprise) if err != nil { return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } @@ -59,7 +58,7 @@ func (s *sqlDatabase) GetEnterpriseByID(ctx context.Context, enterpriseID string return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } - param, err := s.sqlToCommonEnterprise(enterprise, s.cfg) + param, err := s.sqlToCommonEnterprise(enterprise) if err != nil { return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } @@ -76,7 +75,7 @@ func (s *sqlDatabase) ListEnterprises(ctx context.Context) ([]params.Enterprise, ret := make([]params.Enterprise, len(enterprises)) for idx, val := range enterprises { var err error - ret[idx], err = s.sqlToCommonEnterprise(val, s.cfg) + ret[idx], err = s.sqlToCommonEnterprise(val) if err != nil { return nil, errors.Wrap(err, "fetching enterprises") } @@ -122,7 +121,7 @@ func (s *sqlDatabase) UpdateEnterprise(ctx context.Context, enterpriseID string, return params.Enterprise{}, errors.Wrap(q.Error, "saving enterprise") } - newParams, err := s.sqlToCommonEnterprise(enterprise, s.cfg) + newParams, err := s.sqlToCommonEnterprise(enterprise) if err != nil { return params.Enterprise{}, errors.Wrap(err, "updating enterprise") } diff --git a/database/sql/enterprise_test.go b/database/sql/enterprise_test.go index 2b36b8dc..bbedcf29 100644 --- a/database/sql/enterprise_test.go +++ b/database/sql/enterprise_test.go @@ -238,16 +238,16 @@ func (s *EnterpriseTestSuite) TestGetEnterpriseNotFound() { } func (s *EnterpriseTestSuite) TestGetEnterpriseDBDecryptingErr() { - // s.Fixtures.SQLMock. - // ExpectQuery(regexp.QuoteMeta("SELECT * FROM `enterprises` WHERE name = ? COLLATE NOCASE AND `enterprises`.`deleted_at` IS NULL ORDER BY `enterprises`.`id` LIMIT 1")). - // WithArgs(s.Fixtures.Enterprises[0].Name). - // WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow(s.Fixtures.Enterprises[0].Name)) + s.Fixtures.SQLMock. + ExpectQuery(regexp.QuoteMeta("SELECT * FROM `enterprises` WHERE name = ? COLLATE NOCASE AND `enterprises`.`deleted_at` IS NULL ORDER BY `enterprises`.`id` LIMIT 1")). + WithArgs(s.Fixtures.Enterprises[0].Name). + WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow(s.Fixtures.Enterprises[0].Name)) - //_, err := s.StoreSQLMocked.GetEnterprise(context.Background(), s.Fixtures.Enterprises[0].Name) + _, err := s.StoreSQLMocked.GetEnterprise(context.Background(), s.Fixtures.Enterprises[0].Name) - // s.assertSQLMockExpectations() - // s.Require().NotNil(err) - // s.Require().Equal("decrypting secret: failed to decrypt text", err.Error()) + s.assertSQLMockExpectations() + s.Require().NotNil(err) + s.Require().Equal("fetching enterprise: missing secret", err.Error()) } func (s *EnterpriseTestSuite) TestListEnterprises() { @@ -353,24 +353,19 @@ func (s *EnterpriseTestSuite) TestUpdateEnterpriseDBSaveErr() { } func (s *EnterpriseTestSuite) TestUpdateEnterpriseDBDecryptingErr() { - // s.StoreSQLMocked.cfg.Passphrase = "wrong-passphrase" - // s.Fixtures.UpdateRepoParams.WebhookSecret = "" + s.StoreSQLMocked.cfg.Passphrase = "wrong-passphrase" + s.Fixtures.UpdateRepoParams.WebhookSecret = "some-webhook-secret" - // s.Fixtures.SQLMock. - // ExpectQuery(regexp.QuoteMeta("SELECT * FROM `enterprises` WHERE id = ? AND `enterprises`.`deleted_at` IS NULL ORDER BY `enterprises`.`id` LIMIT 1")). - // WithArgs(s.Fixtures.Enterprises[0].ID). - // WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Enterprises[0].ID)) - // s.Fixtures.SQLMock.ExpectBegin() - // s.Fixtures.SQLMock. - // ExpectExec(("UPDATE `enterprises` SET")). - // WillReturnResult(sqlmock.NewResult(1, 1)) - // s.Fixtures.SQLMock.ExpectCommit() + s.Fixtures.SQLMock. + ExpectQuery(regexp.QuoteMeta("SELECT * FROM `enterprises` WHERE id = ? AND `enterprises`.`deleted_at` IS NULL ORDER BY `enterprises`.`id` LIMIT 1")). + WithArgs(s.Fixtures.Enterprises[0].ID). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Enterprises[0].ID)) - // _, err := s.StoreSQLMocked.UpdateEnterprise(context.Background(), s.Fixtures.Enterprises[0].ID, s.Fixtures.UpdateRepoParams) + _, err := s.StoreSQLMocked.UpdateEnterprise(context.Background(), s.Fixtures.Enterprises[0].ID, s.Fixtures.UpdateRepoParams) - // s.assertSQLMockExpectations() - // s.Require().NotNil(err) - // s.Require().Equal("decrypting secret: invalid passphrase length (expected length 32 characters)", err.Error()) + s.assertSQLMockExpectations() + s.Require().NotNil(err) + s.Require().Equal("encoding secret: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *EnterpriseTestSuite) TestGetEnterpriseByID() { @@ -388,20 +383,20 @@ func (s *EnterpriseTestSuite) TestGetEnterpriseByIDInvalidEnterpriseID() { } func (s *EnterpriseTestSuite) TestGetEnterpriseByIDDBDecryptingErr() { - // s.Fixtures.SQLMock. - // ExpectQuery(regexp.QuoteMeta("SELECT * FROM `enterprises` WHERE id = ? AND `enterprises`.`deleted_at` IS NULL ORDER BY `enterprises`.`id` LIMIT 1")). - // WithArgs(s.Fixtures.Enterprises[0].ID). - // WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Enterprises[0].ID)) - // s.Fixtures.SQLMock. - // ExpectQuery(regexp.QuoteMeta("SELECT * FROM `pools` WHERE `pools`.`enterprise_id` = ? AND `pools`.`deleted_at` IS NULL")). - // WithArgs(s.Fixtures.Enterprises[0].ID). - // WillReturnRows(sqlmock.NewRows([]string{"enterprise_id"}).AddRow(s.Fixtures.Enterprises[0].ID)) + s.Fixtures.SQLMock. + ExpectQuery(regexp.QuoteMeta("SELECT * FROM `enterprises` WHERE id = ? AND `enterprises`.`deleted_at` IS NULL ORDER BY `enterprises`.`id` LIMIT 1")). + WithArgs(s.Fixtures.Enterprises[0].ID). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Enterprises[0].ID)) + s.Fixtures.SQLMock. + ExpectQuery(regexp.QuoteMeta("SELECT * FROM `pools` WHERE `pools`.`enterprise_id` = ? AND `pools`.`deleted_at` IS NULL")). + WithArgs(s.Fixtures.Enterprises[0].ID). + WillReturnRows(sqlmock.NewRows([]string{"enterprise_id"}).AddRow(s.Fixtures.Enterprises[0].ID)) - // _, err := s.StoreSQLMocked.GetEnterpriseByID(context.Background(), s.Fixtures.Enterprises[0].ID) + _, err := s.StoreSQLMocked.GetEnterpriseByID(context.Background(), s.Fixtures.Enterprises[0].ID) - // s.assertSQLMockExpectations() - // s.Require().NotNil(err) - // s.Require().Equal("decrypting secret: failed to decrypt text", err.Error()) + s.assertSQLMockExpectations() + s.Require().NotNil(err) + s.Require().Equal("fetching enterprise: missing secret", err.Error()) } func (s *EnterpriseTestSuite) TestCreateEnterprisePool() { diff --git a/database/sql/organizations.go b/database/sql/organizations.go index 37e35ef6..0777a0ff 100644 --- a/database/sql/organizations.go +++ b/database/sql/organizations.go @@ -17,6 +17,7 @@ package sql import ( "context" "fmt" + runnerErrors "garm/errors" "garm/params" "garm/util" @@ -27,13 +28,12 @@ import ( ) func (s *sqlDatabase) CreateOrganization(ctx context.Context, name, credentialsName, webhookSecret string) (params.Organization, error) { - secret := []byte{} - var err error - if webhookSecret != "" { - secret, err = util.Aes256EncodeString(webhookSecret, s.cfg.Passphrase) - if err != nil { - return params.Organization{}, fmt.Errorf("failed to encrypt string") - } + if webhookSecret == "" { + return params.Organization{}, errors.New("creating org: missing secret") + } + secret, err := util.Aes256EncodeString(webhookSecret, s.cfg.Passphrase) + if err != nil { + return params.Organization{}, fmt.Errorf("failed to encrypt string") } newOrg := Organization{ Name: name, @@ -46,7 +46,10 @@ func (s *sqlDatabase) CreateOrganization(ctx context.Context, name, credentialsN return params.Organization{}, errors.Wrap(q.Error, "creating org") } - param := s.sqlToCommonOrganization(newOrg) + param, err := s.sqlToCommonOrganization(newOrg) + if err != nil { + return params.Organization{}, errors.Wrap(err, "creating org") + } param.WebhookSecret = webhookSecret return param, nil @@ -58,12 +61,10 @@ func (s *sqlDatabase) GetOrganization(ctx context.Context, name string) (params. return params.Organization{}, errors.Wrap(err, "fetching org") } - param := s.sqlToCommonOrganization(org) - secret, err := util.Aes256DecodeString(org.WebhookSecret, s.cfg.Passphrase) + param, err := s.sqlToCommonOrganization(org) if err != nil { - return params.Organization{}, errors.Wrap(err, "decrypting secret") + return params.Organization{}, errors.Wrap(err, "fetching org") } - param.WebhookSecret = secret return param, nil } @@ -77,7 +78,11 @@ func (s *sqlDatabase) ListOrganizations(ctx context.Context) ([]params.Organizat ret := make([]params.Organization, len(orgs)) for idx, val := range orgs { - ret[idx] = s.sqlToCommonOrganization(val) + var err error + ret[idx], err = s.sqlToCommonOrganization(val) + if err != nil { + return nil, errors.Wrap(err, "fetching org") + } } return ret, nil @@ -110,7 +115,7 @@ func (s *sqlDatabase) UpdateOrganization(ctx context.Context, orgID string, para if param.WebhookSecret != "" { secret, err := util.Aes256EncodeString(param.WebhookSecret, s.cfg.Passphrase) if err != nil { - return params.Organization{}, fmt.Errorf("failed to encrypt string") + return params.Organization{}, fmt.Errorf("saving org: failed to encrypt string: %w", err) } org.WebhookSecret = secret } @@ -120,12 +125,10 @@ func (s *sqlDatabase) UpdateOrganization(ctx context.Context, orgID string, para return params.Organization{}, errors.Wrap(q.Error, "saving org") } - newParams := s.sqlToCommonOrganization(org) - secret, err := util.Aes256DecodeString(org.WebhookSecret, s.cfg.Passphrase) + newParams, err := s.sqlToCommonOrganization(org) if err != nil { - return params.Organization{}, errors.Wrap(err, "decrypting secret") + return params.Organization{}, errors.Wrap(err, "saving org") } - newParams.WebhookSecret = secret return newParams, nil } @@ -135,13 +138,10 @@ func (s *sqlDatabase) GetOrganizationByID(ctx context.Context, orgID string) (pa return params.Organization{}, errors.Wrap(err, "fetching org") } - param := s.sqlToCommonOrganization(org) - secret, err := util.Aes256DecodeString(org.WebhookSecret, s.cfg.Passphrase) + param, err := s.sqlToCommonOrganization(org) if err != nil { - return params.Organization{}, errors.Wrap(err, "decrypting secret") + return params.Organization{}, errors.Wrap(err, "fetching enterprise") } - param.WebhookSecret = secret - return param, nil } diff --git a/database/sql/organizations_test.go b/database/sql/organizations_test.go index 8b2af3b5..8f3154ea 100644 --- a/database/sql/organizations_test.go +++ b/database/sql/organizations_test.go @@ -20,12 +20,12 @@ import ( "fmt" "regexp" "sort" + "testing" dbCommon "garm/database/common" runnerErrors "garm/errors" garmTesting "garm/internal/testing" "garm/params" - "testing" "github.com/stretchr/testify/suite" "gopkg.in/DATA-DOG/go-sqlmock.v1" @@ -247,7 +247,7 @@ func (s *OrgTestSuite) TestGetOrganizationDBDecryptingErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: failed to decrypt text", err.Error()) + s.Require().Equal("fetching org: missing secret", err.Error()) } func (s *OrgTestSuite) TestListOrganizations() { @@ -331,7 +331,7 @@ func (s *OrgTestSuite) TestUpdateOrganizationDBEncryptErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("failed to encrypt string", err.Error()) + s.Require().Equal("saving org: failed to encrypt string: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *OrgTestSuite) TestUpdateOrganizationDBSaveErr() { @@ -354,23 +354,18 @@ func (s *OrgTestSuite) TestUpdateOrganizationDBSaveErr() { func (s *OrgTestSuite) TestUpdateOrganizationDBDecryptingErr() { s.StoreSQLMocked.cfg.Passphrase = "wrong-passphrase" - s.Fixtures.UpdateRepoParams.WebhookSecret = "" + s.Fixtures.UpdateRepoParams.WebhookSecret = "some-webhook-secret" s.Fixtures.SQLMock. ExpectQuery(regexp.QuoteMeta("SELECT * FROM `organizations` WHERE id = ? AND `organizations`.`deleted_at` IS NULL ORDER BY `organizations`.`id` LIMIT 1")). WithArgs(s.Fixtures.Orgs[0].ID). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Orgs[0].ID)) - s.Fixtures.SQLMock.ExpectBegin() - s.Fixtures.SQLMock. - ExpectExec(("UPDATE `organizations` SET")). - WillReturnResult(sqlmock.NewResult(1, 1)) - s.Fixtures.SQLMock.ExpectCommit() _, err := s.StoreSQLMocked.UpdateOrganization(context.Background(), s.Fixtures.Orgs[0].ID, s.Fixtures.UpdateRepoParams) s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: invalid passphrase length (expected length 32 characters)", err.Error()) + s.Require().Equal("saving org: failed to encrypt string: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *OrgTestSuite) TestGetOrganizationByID() { @@ -401,7 +396,7 @@ func (s *OrgTestSuite) TestGetOrganizationByIDDBDecryptingErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: failed to decrypt text", err.Error()) + s.Require().Equal("fetching enterprise: missing secret", err.Error()) } func (s *OrgTestSuite) TestCreateOrganizationPool() { diff --git a/database/sql/repositories.go b/database/sql/repositories.go index b40147e0..5cb0dc5f 100644 --- a/database/sql/repositories.go +++ b/database/sql/repositories.go @@ -17,6 +17,7 @@ package sql import ( "context" "fmt" + runnerErrors "garm/errors" "garm/params" "garm/util" @@ -27,13 +28,12 @@ import ( ) func (s *sqlDatabase) CreateRepository(ctx context.Context, owner, name, credentialsName, webhookSecret string) (params.Repository, error) { - secret := []byte{} - var err error - if webhookSecret != "" { - secret, err = util.Aes256EncodeString(webhookSecret, s.cfg.Passphrase) - if err != nil { - return params.Repository{}, fmt.Errorf("failed to encrypt string") - } + if webhookSecret == "" { + return params.Repository{}, errors.New("creating repo: missing secret") + } + secret, err := util.Aes256EncodeString(webhookSecret, s.cfg.Passphrase) + if err != nil { + return params.Repository{}, fmt.Errorf("failed to encrypt string") } newRepo := Repository{ Name: name, @@ -47,8 +47,10 @@ func (s *sqlDatabase) CreateRepository(ctx context.Context, owner, name, credent return params.Repository{}, errors.Wrap(q.Error, "creating repository") } - param := s.sqlToCommonRepository(newRepo) - param.WebhookSecret = webhookSecret + param, err := s.sqlToCommonRepository(newRepo) + if err != nil { + return params.Repository{}, errors.Wrap(err, "creating repository") + } return param, nil } @@ -59,12 +61,10 @@ func (s *sqlDatabase) GetRepository(ctx context.Context, owner, name string) (pa return params.Repository{}, errors.Wrap(err, "fetching repo") } - param := s.sqlToCommonRepository(repo) - secret, err := util.Aes256DecodeString(repo.WebhookSecret, s.cfg.Passphrase) + param, err := s.sqlToCommonRepository(repo) if err != nil { - return params.Repository{}, errors.Wrap(err, "decrypting secret") + return params.Repository{}, errors.Wrap(err, "fetching repo") } - param.WebhookSecret = secret return param, nil } @@ -78,13 +78,10 @@ func (s *sqlDatabase) ListRepositories(ctx context.Context) ([]params.Repository ret := make([]params.Repository, len(repos)) for idx, val := range repos { - ret[idx] = s.sqlToCommonRepository(val) - if len(val.WebhookSecret) > 0 { - secret, err := util.Aes256DecodeString(val.WebhookSecret, s.cfg.Passphrase) - if err != nil { - return nil, errors.Wrap(err, "decrypting secret") - } - ret[idx].WebhookSecret = secret + var err error + ret[idx], err = s.sqlToCommonRepository(val) + if err != nil { + return nil, errors.Wrap(err, "fetching repositories") } } @@ -118,7 +115,7 @@ func (s *sqlDatabase) UpdateRepository(ctx context.Context, repoID string, param if param.WebhookSecret != "" { secret, err := util.Aes256EncodeString(param.WebhookSecret, s.cfg.Passphrase) if err != nil { - return params.Repository{}, fmt.Errorf("failed to encrypt string") + return params.Repository{}, fmt.Errorf("saving repo: failed to encrypt string: %w", err) } repo.WebhookSecret = secret } @@ -128,12 +125,10 @@ func (s *sqlDatabase) UpdateRepository(ctx context.Context, repoID string, param return params.Repository{}, errors.Wrap(q.Error, "saving repo") } - newParams := s.sqlToCommonRepository(repo) - secret, err := util.Aes256DecodeString(repo.WebhookSecret, s.cfg.Passphrase) + newParams, err := s.sqlToCommonRepository(repo) if err != nil { - return params.Repository{}, errors.Wrap(err, "decrypting secret") + return params.Repository{}, errors.Wrap(err, "saving repo") } - newParams.WebhookSecret = secret return newParams, nil } @@ -143,13 +138,10 @@ func (s *sqlDatabase) GetRepositoryByID(ctx context.Context, repoID string) (par return params.Repository{}, errors.Wrap(err, "fetching repo") } - param := s.sqlToCommonRepository(repo) - secret, err := util.Aes256DecodeString(repo.WebhookSecret, s.cfg.Passphrase) + param, err := s.sqlToCommonRepository(repo) if err != nil { - return params.Repository{}, errors.Wrap(err, "decrypting secret") + return params.Repository{}, errors.Wrap(err, "fetching repo") } - param.WebhookSecret = secret - return param, nil } diff --git a/database/sql/repositories_test.go b/database/sql/repositories_test.go index 95ca90a4..f02497d3 100644 --- a/database/sql/repositories_test.go +++ b/database/sql/repositories_test.go @@ -18,13 +18,14 @@ import ( "context" "flag" "fmt" - dbCommon "garm/database/common" - garmTesting "garm/internal/testing" - "garm/params" "regexp" "sort" "testing" + dbCommon "garm/database/common" + garmTesting "garm/internal/testing" + "garm/params" + "github.com/stretchr/testify/suite" "gopkg.in/DATA-DOG/go-sqlmock.v1" "gorm.io/driver/mysql" @@ -97,7 +98,7 @@ func (s *RepoTestSuite) SetupTest() { fmt.Sprintf("test-webhook-secret-%d", i), ) if err != nil { - s.FailNow(fmt.Sprintf("failed to create database object (test-repo-%d)", i)) + s.FailNow(fmt.Sprintf("failed to create database object (test-repo-%d): %v", i, err)) } repos = append(repos, repo) @@ -271,7 +272,7 @@ func (s *RepoTestSuite) TestGetRepositoryDBDecryptingErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: failed to decrypt text", err.Error()) + s.Require().Equal("fetching repo: missing secret", err.Error()) } func (s *RepoTestSuite) TestListRepositories() { @@ -304,7 +305,7 @@ func (s *RepoTestSuite) TestListRepositoriesDBDecryptingErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: invalid passphrase length (expected length 32 characters)", err.Error()) + s.Require().Equal("fetching repositories: decrypting secret: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *RepoTestSuite) TestDeleteRepository() { @@ -368,7 +369,7 @@ func (s *RepoTestSuite) TestUpdateRepositoryDBEncryptErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("failed to encrypt string", err.Error()) + s.Require().Equal("saving repo: failed to encrypt string: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *RepoTestSuite) TestUpdateRepositoryDBSaveErr() { @@ -391,23 +392,18 @@ func (s *RepoTestSuite) TestUpdateRepositoryDBSaveErr() { func (s *RepoTestSuite) TestUpdateRepositoryDBDecryptingErr() { s.StoreSQLMocked.cfg.Passphrase = "wrong-passphrase" - s.Fixtures.UpdateRepoParams.WebhookSecret = "" + s.Fixtures.UpdateRepoParams.WebhookSecret = "some-webhook-secret" s.Fixtures.SQLMock. ExpectQuery(regexp.QuoteMeta("SELECT * FROM `repositories` WHERE id = ? AND `repositories`.`deleted_at` IS NULL ORDER BY `repositories`.`id` LIMIT 1")). WithArgs(s.Fixtures.Repos[0].ID). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Repos[0].ID)) - s.Fixtures.SQLMock.ExpectBegin() - s.Fixtures.SQLMock. - ExpectExec(("UPDATE `repositories` SET")). - WillReturnResult(sqlmock.NewResult(1, 1)) - s.Fixtures.SQLMock.ExpectCommit() _, err := s.StoreSQLMocked.UpdateRepository(context.Background(), s.Fixtures.Repos[0].ID, s.Fixtures.UpdateRepoParams) s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: invalid passphrase length (expected length 32 characters)", err.Error()) + s.Require().Equal("saving repo: failed to encrypt string: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *RepoTestSuite) TestGetRepositoryByID() { @@ -438,7 +434,7 @@ func (s *RepoTestSuite) TestGetRepositoryByIDDBDecryptingErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("decrypting secret: failed to decrypt text", err.Error()) + s.Require().Equal("fetching repo: missing secret", err.Error()) } func (s *RepoTestSuite) TestCreateRepositoryPool() { diff --git a/database/sql/util.go b/database/sql/util.go index 828fd356..93f631d1 100644 --- a/database/sql/util.go +++ b/database/sql/util.go @@ -17,7 +17,6 @@ package sql import ( "fmt" - "garm/config" "garm/params" "garm/util" @@ -77,29 +76,37 @@ func (s *sqlDatabase) sqlAddressToParamsAddress(addr Address) params.Address { } } -func (s *sqlDatabase) sqlToCommonOrganization(org Organization) params.Organization { +func (s *sqlDatabase) sqlToCommonOrganization(org Organization) (params.Organization, error) { + if len(org.WebhookSecret) == 0 { + return params.Organization{}, errors.New("missing secret") + } + secret, err := util.Aes256DecodeString(org.WebhookSecret, s.cfg.Passphrase) + if err != nil { + return params.Organization{}, errors.Wrap(err, "decrypting secret") + } + ret := params.Organization{ ID: org.ID.String(), Name: org.Name, CredentialsName: org.CredentialsName, Pools: make([]params.Pool, len(org.Pools)), + WebhookSecret: secret, } for idx, pool := range org.Pools { ret.Pools[idx] = s.sqlToCommonPool(pool) } - return ret + return ret, nil } -func (s *sqlDatabase) sqlToCommonEnterprise(enterprise Enterprise, cfg config.Database) (params.Enterprise, error) { - secret := "" - if len(enterprise.WebhookSecret) > 0 { - var err error - secret, err = util.Aes256DecodeString(enterprise.WebhookSecret, s.cfg.Passphrase) - if err != nil { - return params.Enterprise{}, errors.Wrap(err, "decrypting secret") - } +func (s *sqlDatabase) sqlToCommonEnterprise(enterprise Enterprise) (params.Enterprise, error) { + if len(enterprise.WebhookSecret) == 0 { + return params.Enterprise{}, errors.New("missing secret") + } + secret, err := util.Aes256DecodeString(enterprise.WebhookSecret, s.cfg.Passphrase) + if err != nil { + return params.Enterprise{}, errors.Wrap(err, "decrypting secret") } ret := params.Enterprise{ @@ -171,20 +178,29 @@ func (s *sqlDatabase) sqlToCommonTags(tag Tag) params.Tag { } } -func (s *sqlDatabase) sqlToCommonRepository(repo Repository) params.Repository { +func (s *sqlDatabase) sqlToCommonRepository(repo Repository) (params.Repository, error) { + if len(repo.WebhookSecret) == 0 { + return params.Repository{}, errors.New("missing secret") + } + secret, err := util.Aes256DecodeString(repo.WebhookSecret, s.cfg.Passphrase) + if err != nil { + return params.Repository{}, errors.Wrap(err, "decrypting secret") + } + ret := params.Repository{ ID: repo.ID.String(), Name: repo.Name, Owner: repo.Owner, CredentialsName: repo.CredentialsName, Pools: make([]params.Pool, len(repo.Pools)), + WebhookSecret: secret, } for idx, pool := range repo.Pools { ret.Pools[idx] = s.sqlToCommonPool(pool) } - return ret + return ret, nil } func (s *sqlDatabase) sqlToParamsUser(user User) params.User { diff --git a/errors/errors.go b/errors/errors.go index c3a6400b..11ebce92 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -55,6 +55,20 @@ type ProviderError struct { baseError } +// NewMissingSecretError returns a new MissingSecretError +func NewMissingSecretError(msg string, a ...interface{}) error { + return &MissingSecretError{ + baseError{ + msg: fmt.Sprintf(msg, a...), + }, + } +} + +// MissingSecretError is returned the secret to validate a webhook is missing +type MissingSecretError struct { + baseError +} + // NewUnauthorizedError returns a new UnauthorizedError func NewUnauthorizedError(msg string) error { return &UnauthorizedError{ diff --git a/runner/runner.go b/runner/runner.go index 098bdec9..601c1662 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -381,7 +381,6 @@ func (r *Runner) Start() error { go func(repo common.PoolManager) { err := repo.Start() errChan <- err - }(repo) } @@ -390,7 +389,6 @@ func (r *Runner) Start() error { err := org.Start() errChan <- err }(org) - } for _, enterprise := range enterprises { @@ -398,7 +396,6 @@ func (r *Runner) Start() error { err := org.Start() errChan <- err }(enterprise) - } for i := 0; i < expectedReplies; i++ { @@ -542,8 +539,7 @@ func (r *Runner) Wait() error { func (r *Runner) validateHookBody(signature, secret string, body []byte) error { if secret == "" { - // A secret was not set. Skip validation of body. - return nil + return runnerErrors.NewMissingSecretError("missing secret to validate webhook signature") } if signature == "" {