From 026fee7bb4e0f5bf1c35394e0f411159658ca3f6 Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Wed, 16 Aug 2023 18:22:50 +0530 Subject: [PATCH 1/5] duplicate role-group-fix --- pkg/user/RoleGroupService.go | 9 ++++++++- pkg/user/repository/RoleGroupRepository.go | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/user/RoleGroupService.go b/pkg/user/RoleGroupService.go index 28c9168688..fffeb267d1 100644 --- a/pkg/user/RoleGroupService.go +++ b/pkg/user/RoleGroupService.go @@ -85,19 +85,26 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean //loading policy for safety casbin2.LoadPolicy() //create new user in our db on d basis of info got from google api or hex. assign a basic role + model := &repository2.RoleGroup{ Name: request.Name, Description: request.Description, } rgName := strings.ToLower(request.Name) object := "group:" + strings.ReplaceAll(rgName, " ", "_") + + _, err := impl.roleGroupRepository.GetRoleGroupByCasbinName(object) + if err != nil && err != pg.ErrNoRows { + impl.logger.Errorw("error in getting role group by casbin name", "err", err, "casbinName", object) + return nil, err + } model.CasbinName = object model.CreatedBy = request.UserId model.UpdatedBy = request.UserId model.CreatedOn = time.Now() model.UpdatedOn = time.Now() model.Active = true - model, err := impl.roleGroupRepository.CreateRoleGroup(model, tx) + model, err = impl.roleGroupRepository.CreateRoleGroup(model, tx) request.Id = model.Id if err != nil { impl.logger.Errorw("error in creating new user group", "error", err) diff --git a/pkg/user/repository/RoleGroupRepository.go b/pkg/user/repository/RoleGroupRepository.go index bd09373d7f..49a1df74c1 100644 --- a/pkg/user/repository/RoleGroupRepository.go +++ b/pkg/user/repository/RoleGroupRepository.go @@ -31,7 +31,7 @@ type RoleGroupRepository interface { GetRoleGroupListByName(name string) ([]*RoleGroup, error) GetAllRoleGroup() ([]*RoleGroup, error) GetRoleGroupListByCasbinNames(name []string) ([]*RoleGroup, error) - + GetRoleGroupByCasbinName(name string) (*RoleGroup, error) CreateRoleGroupRoleMapping(model *RoleGroupRoleMapping, tx *pg.Tx) (*RoleGroupRoleMapping, error) GetRoleGroupRoleMapping(model int32) (*RoleGroupRoleMapping, error) GetRoleGroupRoleMappingByRoleGroupId(roleGroupId int32) ([]*RoleGroupRoleMapping, error) @@ -123,6 +123,11 @@ func (impl RoleGroupRepositoryImpl) GetRoleGroupListByCasbinNames(names []string err := impl.dbConnection.Model(&model).Where("casbin_name in (?)", pg.In(names)).Where("active = ?", true).Select() return model, err } +func (impl RoleGroupRepositoryImpl) GetRoleGroupByCasbinName(name string) (*RoleGroup, error) { + var model RoleGroup + err := impl.dbConnection.Model(&model).Where("casbin_name = ?", name).Where("active = ?", true).Order("updated_on desc").Select() + return &model, err +} func (impl RoleGroupRepositoryImpl) CreateRoleGroupRoleMapping(model *RoleGroupRoleMapping, tx *pg.Tx) (*RoleGroupRoleMapping, error) { err := tx.Insert(model) From f04296a73bbe4a28f3b04dd80ee33966023ca40a Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Wed, 16 Aug 2023 19:01:27 +0530 Subject: [PATCH 2/5] condition addition --- pkg/user/RoleGroupService.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/user/RoleGroupService.go b/pkg/user/RoleGroupService.go index fffeb267d1..a6f94f4c23 100644 --- a/pkg/user/RoleGroupService.go +++ b/pkg/user/RoleGroupService.go @@ -18,6 +18,7 @@ package user import ( + "errors" "fmt" bean2 "github.com/devtron-labs/devtron/pkg/user/bean" repository2 "github.com/devtron-labs/devtron/pkg/user/repository" @@ -93,10 +94,13 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean rgName := strings.ToLower(request.Name) object := "group:" + strings.ReplaceAll(rgName, " ", "_") - _, err := impl.roleGroupRepository.GetRoleGroupByCasbinName(object) + roleGroup, err := impl.roleGroupRepository.GetRoleGroupByCasbinName(object) if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in getting role group by casbin name", "err", err, "casbinName", object) return nil, err + } else if roleGroup != nil { + impl.logger.Errorw("role group already present", "err", err) + return nil, errors.New("role group already exist") } model.CasbinName = object model.CreatedBy = request.UserId From e71452f2a46205ff8b1cdf90b36bfb4aacda72ca Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Wed, 16 Aug 2023 19:11:18 +0530 Subject: [PATCH 3/5] review comments --- pkg/user/RoleGroupService.go | 4 ++-- pkg/user/repository/RoleGroupRepository.go | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/user/RoleGroupService.go b/pkg/user/RoleGroupService.go index a6f94f4c23..f99a5736ec 100644 --- a/pkg/user/RoleGroupService.go +++ b/pkg/user/RoleGroupService.go @@ -94,11 +94,11 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean rgName := strings.ToLower(request.Name) object := "group:" + strings.ReplaceAll(rgName, " ", "_") - roleGroup, err := impl.roleGroupRepository.GetRoleGroupByCasbinName(object) + exists, err := impl.roleGroupRepository.CheckRoleGroupExistByCasbinName(object) if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in getting role group by casbin name", "err", err, "casbinName", object) return nil, err - } else if roleGroup != nil { + } else if exists { impl.logger.Errorw("role group already present", "err", err) return nil, errors.New("role group already exist") } diff --git a/pkg/user/repository/RoleGroupRepository.go b/pkg/user/repository/RoleGroupRepository.go index 49a1df74c1..ce1e320ed7 100644 --- a/pkg/user/repository/RoleGroupRepository.go +++ b/pkg/user/repository/RoleGroupRepository.go @@ -31,7 +31,7 @@ type RoleGroupRepository interface { GetRoleGroupListByName(name string) ([]*RoleGroup, error) GetAllRoleGroup() ([]*RoleGroup, error) GetRoleGroupListByCasbinNames(name []string) ([]*RoleGroup, error) - GetRoleGroupByCasbinName(name string) (*RoleGroup, error) + CheckRoleGroupExistByCasbinName(name string) (bool, error) CreateRoleGroupRoleMapping(model *RoleGroupRoleMapping, tx *pg.Tx) (*RoleGroupRoleMapping, error) GetRoleGroupRoleMapping(model int32) (*RoleGroupRoleMapping, error) GetRoleGroupRoleMappingByRoleGroupId(roleGroupId int32) ([]*RoleGroupRoleMapping, error) @@ -123,10 +123,11 @@ func (impl RoleGroupRepositoryImpl) GetRoleGroupListByCasbinNames(names []string err := impl.dbConnection.Model(&model).Where("casbin_name in (?)", pg.In(names)).Where("active = ?", true).Select() return model, err } -func (impl RoleGroupRepositoryImpl) GetRoleGroupByCasbinName(name string) (*RoleGroup, error) { - var model RoleGroup - err := impl.dbConnection.Model(&model).Where("casbin_name = ?", name).Where("active = ?", true).Order("updated_on desc").Select() - return &model, err + +func (impl RoleGroupRepositoryImpl) CheckRoleGroupExistByCasbinName(name string) (bool, error) { + var model *RoleGroup + return impl.dbConnection.Model(&model).Where("casbin_name = ?", name).Where("active = ?", true).Exists() + } func (impl RoleGroupRepositoryImpl) CreateRoleGroupRoleMapping(model *RoleGroupRoleMapping, tx *pg.Tx) (*RoleGroupRoleMapping, error) { From 31421a0cb68cef5a1ead31a392472760f84b491c Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Wed, 16 Aug 2023 19:13:23 +0530 Subject: [PATCH 4/5] review comments --- pkg/user/RoleGroupService.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/user/RoleGroupService.go b/pkg/user/RoleGroupService.go index f99a5736ec..c4e2aaa5d8 100644 --- a/pkg/user/RoleGroupService.go +++ b/pkg/user/RoleGroupService.go @@ -86,7 +86,6 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean //loading policy for safety casbin2.LoadPolicy() //create new user in our db on d basis of info got from google api or hex. assign a basic role - model := &repository2.RoleGroup{ Name: request.Name, Description: request.Description, @@ -95,7 +94,7 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean object := "group:" + strings.ReplaceAll(rgName, " ", "_") exists, err := impl.roleGroupRepository.CheckRoleGroupExistByCasbinName(object) - if err != nil && err != pg.ErrNoRows { + if err != nil { impl.logger.Errorw("error in getting role group by casbin name", "err", err, "casbinName", object) return nil, err } else if exists { From 3b4f532e9a9a91f4b651407ccc476f5b86297027 Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Wed, 16 Aug 2023 19:22:40 +0530 Subject: [PATCH 5/5] review comments --- pkg/user/RoleGroupService.go | 2 +- pkg/user/repository/RoleGroupRepository.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/user/RoleGroupService.go b/pkg/user/RoleGroupService.go index c4e2aaa5d8..78456b0faa 100644 --- a/pkg/user/RoleGroupService.go +++ b/pkg/user/RoleGroupService.go @@ -98,7 +98,7 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean impl.logger.Errorw("error in getting role group by casbin name", "err", err, "casbinName", object) return nil, err } else if exists { - impl.logger.Errorw("role group already present", "err", err) + impl.logger.Errorw("role group already present", "err", err, "roleGroup", request.Name) return nil, errors.New("role group already exist") } model.CasbinName = object diff --git a/pkg/user/repository/RoleGroupRepository.go b/pkg/user/repository/RoleGroupRepository.go index ce1e320ed7..262d7a7634 100644 --- a/pkg/user/repository/RoleGroupRepository.go +++ b/pkg/user/repository/RoleGroupRepository.go @@ -125,7 +125,7 @@ func (impl RoleGroupRepositoryImpl) GetRoleGroupListByCasbinNames(names []string } func (impl RoleGroupRepositoryImpl) CheckRoleGroupExistByCasbinName(name string) (bool, error) { - var model *RoleGroup + var model RoleGroup return impl.dbConnection.Model(&model).Where("casbin_name = ?", name).Where("active = ?", true).Exists() }