Skip to content

Commit 3ff7fdb

Browse files
Shivam-nagar23Shivam-nagar23
authored
auth group fix (#5966)
1 parent 5b2f3ad commit 3ff7fdb

File tree

5 files changed

+57
-32
lines changed

5 files changed

+57
-32
lines changed

‎api/auth/user/UserRestHandler.go

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,7 @@ func (handler UserRestHandlerImpl) checkRBACForUserCreate(token string, requestS
11901190
}
11911191

11921192
func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo *bean.UserInfo, isUserAlreadySuperAdmin bool, eliminatedRoleFilters,
1193-
eliminatedGroupRoles []*repository.RoleModel) (isAuthorised bool, err error) {
1193+
eliminatedGroupRoles []*repository.RoleModel, mapOfExistingUserRoleGroup map[string]bool) (isAuthorised bool, err error) {
11941194
isActionUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
11951195
requestSuperAdmin := userInfo.SuperAdmin
11961196
if (requestSuperAdmin || isUserAlreadySuperAdmin) && !isActionUserSuperAdmin {
@@ -1241,33 +1241,37 @@ func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo
12411241
}
12421242
}
12431243
if len(roleGroups) > 0 { // auth check inside groups
1244-
groupRoles, err := handler.roleGroupService.FetchRolesForUserRoleGroups(roleGroups)
1245-
if err != nil && err != pg.ErrNoRows {
1246-
handler.logger.Errorw("service err, UpdateUser", "err", err, "payload", roleGroups)
1247-
return false, err
1248-
}
1249-
if len(groupRoles) > 0 {
1250-
for _, groupRole := range groupRoles {
1251-
switch {
1252-
case groupRole.Action == bean.ACTION_SUPERADMIN:
1253-
isAuthorised = isActionUserSuperAdmin
1254-
case groupRole.AccessType == bean.APP_ACCESS_TYPE_HELM || groupRole.Entity == bean2.EntityJobs:
1255-
isAuthorised = isActionUserSuperAdmin
1256-
case len(groupRole.Team) > 0:
1257-
isAuthorised = handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionCreate, groupRole.Team)
1258-
case groupRole.Entity == bean.CLUSTER_ENTITIY:
1259-
isAuthorised = handler.userCommonService.CheckRbacForClusterEntity(groupRole.Cluster, groupRole.Namespace, groupRole.Group, groupRole.Kind, groupRole.Resource, token, handler.CheckManagerAuth)
1260-
case groupRole.Entity == bean.CHART_GROUP_ENTITY:
1261-
isAuthorised = true
1262-
default:
1263-
isAuthorised = false
1264-
}
1265-
if !isAuthorised {
1266-
return false, nil
1244+
//filter out roleGroups (existing has to be ignore while checking rbac)
1245+
filteredRoleGroups := util2.FilterRoleGroupIfAlreadyPresent(roleGroups, mapOfExistingUserRoleGroup)
1246+
if len(filteredRoleGroups) > 0 {
1247+
groupRoles, err := handler.roleGroupService.FetchRolesForUserRoleGroups(roleGroups)
1248+
if err != nil && err != pg.ErrNoRows {
1249+
handler.logger.Errorw("service err, UpdateUser", "err", err, "filteredRoleGroups", filteredRoleGroups)
1250+
return false, err
1251+
}
1252+
if len(groupRoles) > 0 {
1253+
for _, groupRole := range groupRoles {
1254+
switch {
1255+
case groupRole.Action == bean.ACTION_SUPERADMIN:
1256+
isAuthorised = isActionUserSuperAdmin
1257+
case groupRole.AccessType == bean.APP_ACCESS_TYPE_HELM || groupRole.Entity == bean2.EntityJobs:
1258+
isAuthorised = isActionUserSuperAdmin
1259+
case len(groupRole.Team) > 0:
1260+
isAuthorised = handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionCreate, groupRole.Team)
1261+
case groupRole.Entity == bean.CLUSTER_ENTITIY:
1262+
isAuthorised = handler.userCommonService.CheckRbacForClusterEntity(groupRole.Cluster, groupRole.Namespace, groupRole.Group, groupRole.Kind, groupRole.Resource, token, handler.CheckManagerAuth)
1263+
case groupRole.Entity == bean.CHART_GROUP_ENTITY:
1264+
isAuthorised = true
1265+
default:
1266+
isAuthorised = false
1267+
}
1268+
if !isAuthorised {
1269+
return false, nil
1270+
}
12671271
}
1272+
} else {
1273+
isAuthorised = false
12681274
}
1269-
} else {
1270-
isAuthorised = false
12711275
}
12721276
}
12731277
}

‎api/auth/user/util/util.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,25 @@
1616

1717
package util
1818

19+
import (
20+
"github.com/devtron-labs/devtron/api/bean"
21+
"github.com/devtron-labs/devtron/pkg/auth/user/helper"
22+
)
23+
1924
func IsGroupsPresent(groups []string) bool {
2025
if len(groups) > 0 {
2126
return true
2227
}
2328
return false
2429
}
30+
31+
func FilterRoleGroupIfAlreadyPresent(roleGroups []bean.UserRoleGroup, mapOfExistingUserRoleGroup map[string]bool) []bean.UserRoleGroup {
32+
finalRoleGroups := make([]bean.UserRoleGroup, 0, len(roleGroups))
33+
for _, roleGrp := range roleGroups {
34+
if _, ok := mapOfExistingUserRoleGroup[helper.GetCasbinNameFromRoleGroupName(roleGrp.RoleGroup.Name)]; !ok {
35+
finalRoleGroups = append(finalRoleGroups, roleGrp)
36+
}
37+
}
38+
return finalRoleGroups
39+
40+
}

‎pkg/auth/user/RoleGroupService.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package user
1919
import (
2020
"errors"
2121
"fmt"
22+
helper2 "github.com/devtron-labs/devtron/pkg/auth/user/helper"
2223
"github.com/devtron-labs/devtron/pkg/auth/user/repository/helper"
2324
"net/http"
2425
"strings"
@@ -101,9 +102,7 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean
101102
Name: request.Name,
102103
Description: request.Description,
103104
}
104-
rgName := strings.ToLower(request.Name)
105-
object := "group:" + strings.ReplaceAll(rgName, " ", "_")
106-
105+
object := helper2.GetCasbinNameFromRoleGroupName(request.Name)
107106
exists, err := impl.roleGroupRepository.CheckRoleGroupExistByCasbinName(object)
108107
if err != nil {
109108
impl.logger.Errorw("error in getting role group by casbin name", "err", err, "casbinName", object)

‎pkg/auth/user/UserService.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ type UserService interface {
5454
CreateUser(userInfo *bean.UserInfo, token string, managerAuth func(resource, token string, object string) bool) ([]*bean.UserInfo, error)
5555
SelfRegisterUserIfNotExists(userInfo *bean.UserInfo) ([]*bean.UserInfo, error)
5656
UpdateUser(userInfo *bean.UserInfo, token string, checkRBACForUserUpdate func(token string, userInfo *bean.UserInfo, isUserAlreadySuperAdmin bool,
57-
eliminatedRoleFilters, eliminatedGroupRoles []*repository.RoleModel) (isAuthorised bool, err error), managerAuth func(resource, token string, object string) bool) (*bean.UserInfo, error)
57+
eliminatedRoleFilters, eliminatedGroupRoles []*repository.RoleModel, mapOfExistingUserRoleGroup map[string]bool) (isAuthorised bool, err error), managerAuth func(resource, token string, object string) bool) (*bean.UserInfo, error)
5858
GetById(id int32) (*bean.UserInfo, error)
5959
GetAll() ([]bean.UserInfo, error)
6060
GetAllWithFilters(request *bean.ListingRequest) (*bean.UserListingResponse, error)
@@ -635,7 +635,7 @@ func (impl *UserServiceImpl) mergeUserRoleGroup(oldUserRoleGroups []bean.UserRol
635635
}
636636

637637
func (impl *UserServiceImpl) UpdateUser(userInfo *bean.UserInfo, token string, checkRBACForUserUpdate func(token string, userInfo *bean.UserInfo,
638-
isUserAlreadySuperAdmin bool, eliminatedRoleFilters, eliminatedGroupRoles []*repository.RoleModel) (isAuthorised bool, err error), managerAuth func(resource, token string, object string) bool) (*bean.UserInfo, error) {
638+
isUserAlreadySuperAdmin bool, eliminatedRoleFilters, eliminatedGroupRoles []*repository.RoleModel, mapOfExistingUserRoleGroup map[string]bool) (isAuthorised bool, err error), managerAuth func(resource, token string, object string) bool) (*bean.UserInfo, error) {
639639
//checking if request for same user is being processed
640640
isLocked := impl.getUserReqLockStateById(userInfo.Id)
641641
if isLocked {
@@ -684,6 +684,7 @@ func (impl *UserServiceImpl) UpdateUser(userInfo *bean.UserInfo, token string, c
684684
//loading policy for safety
685685
casbin2.LoadPolicy()
686686
var eliminatedRoles, eliminatedGroupRoles []*repository.RoleModel
687+
mapOfExistingUserRoleGroup := make(map[string]bool)
687688
if userInfo.SuperAdmin == false {
688689
//Starts Role and Mapping
689690
userRoleModels, err := impl.userAuthRepository.GetUserRoleMappingByUserId(model.Id)
@@ -732,6 +733,7 @@ func (impl *UserServiceImpl) UpdateUser(userInfo *bean.UserInfo, token string, c
732733
}
733734
for _, oldItem := range userCasbinRoles {
734735
oldGroupMap[oldItem] = oldItem
736+
mapOfExistingUserRoleGroup[oldItem] = true
735737
}
736738
// START GROUP POLICY
737739
for _, item := range userInfo.UserRoleGroup {
@@ -802,7 +804,7 @@ func (impl *UserServiceImpl) UpdateUser(userInfo *bean.UserInfo, token string, c
802804
}
803805

804806
if checkRBACForUserUpdate != nil {
805-
isAuthorised, err := checkRBACForUserUpdate(token, userInfo, isUserSuperAdmin, eliminatedRoles, eliminatedGroupRoles)
807+
isAuthorised, err := checkRBACForUserUpdate(token, userInfo, isUserSuperAdmin, eliminatedRoles, eliminatedGroupRoles, mapOfExistingUserRoleGroup)
806808
if err != nil {
807809
impl.logger.Errorw("error in checking RBAC for user update", "err", err, "userInfo", userInfo)
808810
return nil, err

‎pkg/auth/user/helper/helper.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,7 @@ func CreateErrorMessageForUserRoleGroups(restrictedGroups []bean2.RestrictedGrou
9393
}
9494
return errorMessageForGroupsWithoutSuperAdmin, errorMessageForGroupsWithSuperAdmin
9595
}
96+
97+
func GetCasbinNameFromRoleGroupName(name string) string {
98+
return "group:" + strings.ReplaceAll(strings.ToLower(name), " ", "_")
99+
}

0 commit comments

Comments
 (0)