diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a11215..b47faf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Added ### Changed ### Fixed + - Fix to validate bare minimum role permissions for custom roles from [niravparikh05](https://github.com/niravparikh05) ## [0.1.0] - 2022-06-22 ### Added diff --git a/pkg/service/const.go b/pkg/service/const.go index f14fa1d..75ca0b2 100644 --- a/pkg/service/const.go +++ b/pkg/service/const.go @@ -10,4 +10,6 @@ const ( projectScope = "project" namespaceR = "kubectl.namespace.read" namespaceW = "kubectl.namespace.write" + partnerR = "partner.read" + organizationR = "organization.read" ) diff --git a/pkg/service/role.go b/pkg/service/role.go index 7cb6ad4..4257489 100644 --- a/pkg/service/role.go +++ b/pkg/service/role.go @@ -145,6 +145,13 @@ func (s *roleService) Create(ctx context.Context, role *rolev3.Role) (*rolev3.Ro } } + //validate basic mandatory permissions that should be part of all custom roles + if len(role.Spec.Rolepermissions) > 0 && + !(utils.Contains(role.Spec.Rolepermissions, partnerR) && + utils.Contains(role.Spec.Rolepermissions, organizationR)) { + return nil, fmt.Errorf("invalid role permissions, '%v', '%v' should be present ", partnerR, organizationR) + } + // Only allow internal call (eg: initialize) to set builtin flag builtin := role.GetSpec().GetBuiltin() if builtin { @@ -264,6 +271,12 @@ func (s *roleService) Update(ctx context.Context, role *rolev3.Role) (*rolev3.Ro } } + //validate basic mandatory permissions that should be part of all custom roles + if !(utils.Contains(role.Spec.Rolepermissions, partnerR) && + utils.Contains(role.Spec.Rolepermissions, organizationR)) { + return nil, fmt.Errorf("invalid role permissions, '%v', '%v' should be present ", partnerR, organizationR) + } + if rle, ok := entity.(*models.Role); ok { if rle.Builtin { return role, fmt.Errorf("builtin role '%v' cannot be updated", name) diff --git a/pkg/service/role_test.go b/pkg/service/role_test.go index eea402a..b5342f1 100644 --- a/pkg/service/role_test.go +++ b/pkg/service/role_test.go @@ -155,7 +155,9 @@ func TestCreateRoleWithPermissions(t *testing.T) { mock.ExpectBegin() mock.ExpectQuery(`INSERT INTO "authsrv_resourcerole"`). WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(ruuid)) - mock.ExpectQuery(`SELECT "resourcepermission"."id" FROM "authsrv_resourcepermission" AS "resourcepermission" WHERE .name = 'ops_star.all'.`). + mock.ExpectQuery(`SELECT "resourcepermission"."id" FROM "authsrv_resourcepermission" AS "resourcepermission" WHERE .name = 'partner.read'.`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(uuid.New().String())) + mock.ExpectQuery(`SELECT "resourcepermission"."id" FROM "authsrv_resourcepermission" AS "resourcepermission" WHERE .name = 'organization.read'.`). WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(uuid.New().String())) mock.ExpectQuery(`INSERT INTO "authsrv_resourcerolepermission"`). WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(uuid.New().String())) @@ -163,7 +165,7 @@ func TestCreateRoleWithPermissions(t *testing.T) { role := &rolev3.Role{ Metadata: &v3.Metadata{Partner: "partner-" + puuid, Organization: "org-" + ouuid, Name: "role-" + ruuid}, - Spec: &rolev3.RoleSpec{IsGlobal: true, Scope: "system", Rolepermissions: []string{"ops_star.all"}}, + Spec: &rolev3.RoleSpec{IsGlobal: true, Scope: "system", Rolepermissions: []string{"partner.read", "organization.read"}}, } role, err := rs.Create(context.Background(), role) if err != nil { @@ -221,15 +223,17 @@ func TestUpdateRole(t *testing.T) { mock.ExpectExec(`UPDATE "authsrv_resourcerolepermission" AS "resourcerolepermission" SET trash = TRUE WHERE ."resource_role_id" = '` + ruuid + `'.`). WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectQuery(`SELECT "resourcepermission"."id" FROM "authsrv_resourcepermission" AS "resourcepermission" WHERE .name = 'ops_star.all'.`). - WithArgs().WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("ops_star.all")) + mock.ExpectQuery(`SELECT "resourcepermission"."id" FROM "authsrv_resourcepermission" AS "resourcepermission" WHERE .name = 'partner.read'.`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("partner.read")) + mock.ExpectQuery(`SELECT "resourcepermission"."id" FROM "authsrv_resourcepermission" AS "resourcepermission" WHERE .name = 'organization.read'.`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("organization.read")) mock.ExpectQuery(`INSERT INTO "authsrv_resourcerolepermission"`). WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(ruuid)) mock.ExpectCommit() role := &rolev3.Role{ Metadata: &v3.Metadata{Partner: "partner-" + puuid, Organization: "org-" + ouuid, Name: "role-" + ruuid}, - Spec: &rolev3.RoleSpec{IsGlobal: true, Scope: "system", Rolepermissions: []string{"ops_star.all"}}, + Spec: &rolev3.RoleSpec{IsGlobal: true, Scope: "system", Rolepermissions: []string{"partner.read", "organization.read"}}, } role, err := rs.Update(context.Background(), role) if err != nil {