diff --git a/database/sql/enterprise.go b/database/sql/enterprise.go index 25e063b5..79afa39b 100644 --- a/database/sql/enterprise.go +++ b/database/sql/enterprise.go @@ -2,7 +2,7 @@ package sql import ( "context" - "fmt" + runnerErrors "garm/errors" "garm/params" "garm/util" @@ -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{}, fmt.Errorf("failed to encrypt string") - } + 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,8 +31,10 @@ func (s *sqlDatabase) CreateEnterprise(ctx context.Context, name, credentialsNam return params.Enterprise{}, errors.Wrap(q.Error, "creating enterprise") } - param := s.sqlToCommonEnterprise(newEnterprise) - param.WebhookSecret = webhookSecret + param, err := s.sqlToCommonEnterprise(newEnterprise) + if err != nil { + return params.Enterprise{}, errors.Wrap(err, "creating enterprise") + } return param, nil } @@ -44,13 +45,10 @@ func (s *sqlDatabase) GetEnterprise(ctx context.Context, name string) (params.En return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } - param := s.sqlToCommonEnterprise(enterprise) - secret, err := util.Aes256DecodeString(enterprise.WebhookSecret, s.cfg.Passphrase) + param, err := s.sqlToCommonEnterprise(enterprise) if err != nil { - return params.Enterprise{}, errors.Wrap(err, "decrypting secret") + return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } - param.WebhookSecret = secret - return param, nil } @@ -60,13 +58,10 @@ func (s *sqlDatabase) GetEnterpriseByID(ctx context.Context, enterpriseID string return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } - param := s.sqlToCommonEnterprise(enterprise) - secret, err := util.Aes256DecodeString(enterprise.WebhookSecret, s.cfg.Passphrase) + param, err := s.sqlToCommonEnterprise(enterprise) if err != nil { - return params.Enterprise{}, errors.Wrap(err, "decrypting secret") + return params.Enterprise{}, errors.Wrap(err, "fetching enterprise") } - param.WebhookSecret = secret - return param, nil } @@ -74,12 +69,16 @@ func (s *sqlDatabase) ListEnterprises(ctx context.Context) ([]params.Enterprise, var enterprises []Enterprise q := s.conn.Find(&enterprises) if q.Error != nil { - return []params.Enterprise{}, errors.Wrap(q.Error, "fetching enterprise from database") + return []params.Enterprise{}, errors.Wrap(q.Error, "fetching enterprises") } ret := make([]params.Enterprise, len(enterprises)) for idx, val := range enterprises { - ret[idx] = s.sqlToCommonEnterprise(val) + var err error + ret[idx], err = s.sqlToCommonEnterprise(val) + if err != nil { + return nil, errors.Wrap(err, "fetching enterprises") + } } return ret, nil @@ -112,7 +111,7 @@ func (s *sqlDatabase) UpdateEnterprise(ctx context.Context, enterpriseID string, if param.WebhookSecret != "" { secret, err := util.Aes256EncodeString(param.WebhookSecret, s.cfg.Passphrase) if err != nil { - return params.Enterprise{}, fmt.Errorf("failed to encrypt string") + return params.Enterprise{}, errors.Wrap(err, "encoding secret") } enterprise.WebhookSecret = secret } @@ -122,12 +121,10 @@ func (s *sqlDatabase) UpdateEnterprise(ctx context.Context, enterpriseID string, return params.Enterprise{}, errors.Wrap(q.Error, "saving enterprise") } - newParams := s.sqlToCommonEnterprise(enterprise) - secret, err := util.Aes256DecodeString(enterprise.WebhookSecret, s.cfg.Passphrase) + newParams, err := s.sqlToCommonEnterprise(enterprise) if err != nil { - return params.Enterprise{}, errors.Wrap(err, "decrypting secret") + return params.Enterprise{}, errors.Wrap(err, "updating enterprise") } - newParams.WebhookSecret = secret return newParams, nil } diff --git a/database/sql/enterprise_test.go b/database/sql/enterprise_test.go index 56865aeb..a01b0399 100644 --- a/database/sql/enterprise_test.go +++ b/database/sql/enterprise_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" @@ -194,7 +194,7 @@ func (s *EnterpriseTestSuite) TestCreateEnterpriseInvalidDBPassphrase() { s.Fixtures.CreateEnterpriseParams.WebhookSecret) s.Require().NotNil(err) - s.Require().Equal("failed to encrypt string", err.Error()) + s.Require().Equal("encoding secret: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *EnterpriseTestSuite) TestCreateEnterpriseDBCreateErr() { @@ -247,7 +247,7 @@ func (s *EnterpriseTestSuite) TestGetEnterpriseDBDecryptingErr() { 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 *EnterpriseTestSuite) TestListEnterprises() { @@ -266,7 +266,7 @@ func (s *EnterpriseTestSuite) TestListEnterprisesDBFetchErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("fetching enterprise from database: fetching user from database mock error", err.Error()) + s.Require().Equal("fetching enterprises: fetching user from database mock error", err.Error()) } func (s *EnterpriseTestSuite) TestDeleteEnterprise() { @@ -331,7 +331,7 @@ func (s *EnterpriseTestSuite) TestUpdateEnterpriseDBEncryptErr() { s.assertSQLMockExpectations() s.Require().NotNil(err) - s.Require().Equal("failed to encrypt string", err.Error()) + s.Require().Equal("encoding secret: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *EnterpriseTestSuite) TestUpdateEnterpriseDBSaveErr() { @@ -354,23 +354,18 @@ func (s *EnterpriseTestSuite) TestUpdateEnterpriseDBSaveErr() { func (s *EnterpriseTestSuite) TestUpdateEnterpriseDBDecryptingErr() { s.StoreSQLMocked.cfg.Passphrase = "wrong-passphrase" - s.Fixtures.UpdateRepoParams.WebhookSecret = "" + s.Fixtures.UpdateRepoParams.WebhookSecret = "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() _, 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.Require().Equal("encoding secret: invalid passphrase length (expected length 32 characters)", err.Error()) } func (s *EnterpriseTestSuite) TestGetEnterpriseByID() { @@ -401,7 +396,7 @@ func (s *EnterpriseTestSuite) TestGetEnterpriseByIDDBDecryptingErr() { 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 *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..35aa4417 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 = "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..65af2181 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 = "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 7f5caa69..93f631d1 100644 --- a/database/sql/util.go +++ b/database/sql/util.go @@ -16,7 +16,9 @@ package sql import ( "fmt" + "garm/params" + "garm/util" "github.com/pkg/errors" uuid "github.com/satori/go.uuid" @@ -74,34 +76,52 @@ 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) params.Enterprise { +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{ ID: enterprise.ID.String(), Name: enterprise.Name, CredentialsName: enterprise.CredentialsName, Pools: make([]params.Pool, len(enterprise.Pools)), + WebhookSecret: secret, } for idx, pool := range enterprise.Pools { ret.Pools[idx] = s.sqlToCommonPool(pool) } - return ret + return ret, nil } func (s *sqlDatabase) sqlToCommonPool(pool Pool) params.Pool { @@ -158,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/params/requests.go b/params/requests.go index 5a1999ea..defa60b0 100644 --- a/params/requests.go +++ b/params/requests.go @@ -16,6 +16,7 @@ package params import ( "fmt" + "garm/config" "garm/errors" "garm/runner/providers/common" @@ -48,6 +49,9 @@ func (c *CreateRepoParams) Validate() error { if c.CredentialsName == "" { return errors.NewBadRequestError("missing credentials name") } + if c.WebhookSecret == "" { + return errors.NewMissingSecretError("missing secret") + } return nil } @@ -65,6 +69,9 @@ func (c *CreateOrgParams) Validate() error { if c.CredentialsName == "" { return errors.NewBadRequestError("missing credentials name") } + if c.WebhookSecret == "" { + return errors.NewMissingSecretError("missing secret") + } return nil } @@ -78,10 +85,12 @@ func (c *CreateEnterpriseParams) Validate() error { if c.Name == "" { return errors.NewBadRequestError("missing enterprise name") } - if c.CredentialsName == "" { return errors.NewBadRequestError("missing credentials name") } + if c.WebhookSecret == "" { + return errors.NewMissingSecretError("missing secret") + } return nil } diff --git a/runner/enterprises_test.go b/runner/enterprises_test.go index 86591e77..688596b3 100644 --- a/runner/enterprises_test.go +++ b/runner/enterprises_test.go @@ -17,6 +17,8 @@ package runner import ( "context" "fmt" + "testing" + "garm/auth" "garm/config" "garm/database" @@ -27,7 +29,6 @@ import ( "garm/runner/common" runnerCommonMocks "garm/runner/common/mocks" runnerMocks "garm/runner/mocks" - "testing" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -106,6 +107,7 @@ func (s *EnterpriseTestSuite) SetupTest() { CreateEnterpriseParams: params.CreateEnterpriseParams{ Name: "test-enterprise-create", CredentialsName: "test-creds", + WebhookSecret: "test-create-enterprise-webhook-secret", }, CreatePoolParams: params.CreatePoolParams{ ProviderName: "test-provider", diff --git a/runner/organizations_test.go b/runner/organizations_test.go index b8a7ef48..683fa609 100644 --- a/runner/organizations_test.go +++ b/runner/organizations_test.go @@ -17,6 +17,8 @@ package runner import ( "context" "fmt" + "testing" + "garm/auth" "garm/config" "garm/database" @@ -27,7 +29,6 @@ import ( "garm/runner/common" runnerCommonMocks "garm/runner/common/mocks" runnerMocks "garm/runner/mocks" - "testing" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -106,6 +107,7 @@ func (s *OrgTestSuite) SetupTest() { CreateOrgParams: params.CreateOrgParams{ Name: "test-org-create", CredentialsName: "test-creds", + WebhookSecret: "test-create-org-webhook-secret", }, CreatePoolParams: params.CreatePoolParams{ ProviderName: "test-provider", diff --git a/runner/repositories_test.go b/runner/repositories_test.go index e372f6dd..1e924832 100644 --- a/runner/repositories_test.go +++ b/runner/repositories_test.go @@ -17,6 +17,8 @@ package runner import ( "context" "fmt" + "testing" + "garm/auth" "garm/config" "garm/database" @@ -27,7 +29,6 @@ import ( "garm/runner/common" runnerCommonMocks "garm/runner/common/mocks" runnerMocks "garm/runner/mocks" - "testing" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" @@ -106,6 +107,7 @@ func (s *RepoTestSuite) SetupTest() { Owner: "test-owner-create", Name: "test-repo-create", CredentialsName: "test-creds", + WebhookSecret: "test-create-repo-webhook-secret", }, CreatePoolParams: params.CreatePoolParams{ ProviderName: "test-provider", @@ -404,6 +406,7 @@ func (s *RepoTestSuite) TestGetRepoPoolByIDErrUnauthorized() { s.Require().Equal(runnerErrors.ErrUnauthorized, err) } + func (s *RepoTestSuite) TestDeleteRepoPool() { pool, err := s.Fixtures.Store.CreateRepositoryPool(s.Fixtures.AdminContext, s.Fixtures.StoreRepos["test-repo-1"].ID, s.Fixtures.CreatePoolParams) if err != nil { 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 == "" {