Skip to content

Commit ff08608

Browse files
kartik-579kartik-579
committed
role group update prod bug fix
1 parent 5c114c4 commit ff08608

File tree

3 files changed

+101
-69
lines changed

3 files changed

+101
-69
lines changed

‎api/auth/user/UserRestHandler.go‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ func (handler UserRestHandlerImpl) UpdateRoleGroup(w http.ResponseWriter, r *htt
698698
return
699699
}
700700

701-
res, err := handler.roleGroupService.UpdateRoleGroup(&request, token, handler.CheckManagerAuth)
701+
res, err := handler.roleGroupService.UpdateRoleGroup(&request, token, handler.checkRBACForRoleGroupUpdate)
702702
if err != nil {
703703
handler.logger.Errorw("service err, UpdateRoleGroup", "err", err, "payload", request)
704704
common.WriteJsonResp(w, err, "", http.StatusInternalServerError)
@@ -1263,6 +1263,60 @@ func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo
12631263
return isAuthorised, nil
12641264
}
12651265

1266+
func (handler UserRestHandlerImpl) checkRBACForRoleGroupUpdate(token string, groupInfo *bean.RoleGroup,
1267+
eliminatedRoleFilters []*repository.RoleModel) (isAuthorised bool, err error) {
1268+
isActionUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
1269+
requestSuperAdmin := groupInfo.SuperAdmin
1270+
if requestSuperAdmin && !isActionUserSuperAdmin {
1271+
//if user is going to be provided with super-admin access or already a super-admin then the action user should be a super-admin
1272+
return false, nil
1273+
}
1274+
isAuthorised = isActionUserSuperAdmin
1275+
if !isAuthorised {
1276+
if groupInfo.RoleFilters != nil && len(groupInfo.RoleFilters) > 0 { //auth check inside roleFilters
1277+
for _, filter := range groupInfo.RoleFilters {
1278+
switch {
1279+
case filter.Action == bean.ACTION_SUPERADMIN:
1280+
isAuthorised = isActionUserSuperAdmin
1281+
case filter.AccessType == bean.APP_ACCESS_TYPE_HELM || filter.Entity == bean2.EntityJobs:
1282+
isAuthorised = isActionUserSuperAdmin
1283+
case len(filter.Team) > 0:
1284+
isAuthorised = handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionCreate, filter.Team)
1285+
case filter.Entity == bean.CLUSTER_ENTITIY:
1286+
isAuthorised = handler.userCommonService.CheckRbacForClusterEntity(filter.Cluster, filter.Namespace, filter.Group, filter.Kind, filter.Resource, token, handler.CheckManagerAuth)
1287+
case filter.Entity == bean.CHART_GROUP_ENTITY:
1288+
isAuthorised = true
1289+
default:
1290+
isAuthorised = false
1291+
}
1292+
if !isAuthorised {
1293+
break
1294+
}
1295+
}
1296+
}
1297+
if len(eliminatedRoleFilters) > 0 {
1298+
for _, filter := range eliminatedRoleFilters {
1299+
switch {
1300+
case filter.AccessType == bean.APP_ACCESS_TYPE_HELM || filter.Entity == bean2.EntityJobs:
1301+
isAuthorised = isActionUserSuperAdmin
1302+
case len(filter.Team) > 0:
1303+
isAuthorised = handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionCreate, filter.Team)
1304+
case filter.Entity == bean.CLUSTER_ENTITIY:
1305+
isAuthorised = handler.userCommonService.CheckRbacForClusterEntity(filter.Cluster, filter.Namespace, filter.Group, filter.Kind, filter.Resource, token, handler.CheckManagerAuth)
1306+
case filter.Entity == bean.CHART_GROUP_ENTITY:
1307+
isAuthorised = true
1308+
default:
1309+
isAuthorised = false
1310+
}
1311+
if !isAuthorised {
1312+
break
1313+
}
1314+
}
1315+
}
1316+
}
1317+
return isAuthorised, nil
1318+
}
1319+
12661320
func (handler UserRestHandlerImpl) checkRBACForRoleGroupDelete(token string, groupRoles []bean.RoleFilter) (isAuthorised bool, err error) {
12671321
isActionUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
12681322
isAuthorised = isActionUserSuperAdmin

‎pkg/auth/user/RoleGroupService.go‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"errors"
2121
"fmt"
2222
"github.com/devtron-labs/devtron/pkg/auth/user/repository/helper"
23+
"net/http"
2324
"strings"
2425
"time"
2526

@@ -37,7 +38,8 @@ import (
3738

3839
type RoleGroupService interface {
3940
CreateRoleGroup(request *bean.RoleGroup) (*bean.RoleGroup, error)
40-
UpdateRoleGroup(request *bean.RoleGroup, token string, managerAuth func(resource, token string, object string) bool) (*bean.RoleGroup, error)
41+
UpdateRoleGroup(request *bean.RoleGroup, token string, checkRBACForGroupUpdate func(token string, groupInfo *bean.RoleGroup,
42+
eliminatedRoleFilters []*repository.RoleModel) (isAuthorised bool, err error)) (*bean.RoleGroup, error)
4143
FetchDetailedRoleGroups(req *bean.ListingRequest) ([]*bean.RoleGroup, error)
4244
FetchRoleGroupsById(id int32) (*bean.RoleGroup, error)
4345
FetchRoleGroups() ([]*bean.RoleGroup, error)
@@ -136,21 +138,21 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean
136138
for index, roleFilter := range request.RoleFilters {
137139
entity := roleFilter.Entity
138140
if entity == bean.CLUSTER_ENTITIY {
139-
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForClusterEntity(roleFilter, request.UserId, model, nil, "", nil, tx, mapping[index])
141+
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForClusterEntity(roleFilter, request.UserId, model, nil, tx, mapping[index])
140142
policies = append(policies, policiesToBeAdded...)
141143
if err != nil {
142144
// making it non-blocking as it is being done for multiple Role filters and does not want this to be blocking.
143145
impl.logger.Errorw("error in creating updating role group for cluster entity", "err", err, "roleFilter", roleFilter)
144146
}
145147
} else if entity == bean2.EntityJobs {
146-
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForJobsEntity(roleFilter, request.UserId, model, nil, "", nil, tx, mapping[index])
148+
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForJobsEntity(roleFilter, request.UserId, model, nil, tx, mapping[index])
147149
policies = append(policies, policiesToBeAdded...)
148150
if err != nil {
149151
// making it non-blocking as it is being done for multiple Role filters and does not want this to be blocking.
150152
impl.logger.Errorw("error in creating updating role group for jobs entity", "err", err, "roleFilter", roleFilter)
151153
}
152154
} else {
153-
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForOtherEntity(roleFilter, request, model, nil, "", nil, tx, mapping[index])
155+
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForOtherEntity(roleFilter, request, model, nil, tx, mapping[index])
154156
policies = append(policies, policiesToBeAdded...)
155157
if err != nil {
156158
// making it non-blocking as it is being done for multiple Role filters and does not want this to be blocking.
@@ -199,7 +201,7 @@ func (impl RoleGroupServiceImpl) CreateRoleGroup(request *bean.RoleGroup) (*bean
199201
return request, nil
200202
}
201203

202-
func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForClusterEntity(roleFilter bean.RoleFilter, userId int32, model *repository.RoleGroup, existingRoles map[int]*repository.RoleGroupRoleMapping, token string, managerAuth func(resource string, token string, object string) bool, tx *pg.Tx, capacity int) ([]casbin2.Policy, error) {
204+
func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForClusterEntity(roleFilter bean.RoleFilter, userId int32, model *repository.RoleGroup, existingRoles map[int]*repository.RoleGroupRoleMapping, tx *pg.Tx, capacity int) ([]casbin2.Policy, error) {
203205
//var policiesToBeAdded []casbin2.Policy
204206
namespaces := strings.Split(roleFilter.Namespace, ",")
205207
groups := strings.Split(roleFilter.Group, ",")
@@ -213,12 +215,6 @@ func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForClusterEntity(roleFil
213215
for _, group := range groups {
214216
for _, kind := range kinds {
215217
for _, resource := range resources {
216-
if managerAuth != nil {
217-
isValidAuth := impl.userCommonService.CheckRbacForClusterEntity(roleFilter.Cluster, namespace, group, kind, resource, token, managerAuth)
218-
if !isValidAuth {
219-
continue
220-
}
221-
}
222218
roleModel, err := impl.userAuthRepository.GetRoleByFilterForAllTypes(entity, "", "", "", "", accessType, roleFilter.Cluster, namespace, group, kind, resource, actionType, false, "")
223219
if err != nil {
224220
impl.logger.Errorw("error in getting new role model by filter")
@@ -263,7 +259,7 @@ func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForClusterEntity(roleFil
263259
return policiesToBeAdded, nil
264260
}
265261

266-
func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForOtherEntity(roleFilter bean.RoleFilter, request *bean.RoleGroup, model *repository.RoleGroup, existingRoles map[int]*repository.RoleGroupRoleMapping, token string, managerAuth func(resource string, token string, object string) bool, tx *pg.Tx, capacity int) ([]casbin2.Policy, error) {
262+
func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForOtherEntity(roleFilter bean.RoleFilter, request *bean.RoleGroup, model *repository.RoleGroup, existingRoles map[int]*repository.RoleGroupRoleMapping, tx *pg.Tx, capacity int) ([]casbin2.Policy, error) {
267263
actionType := roleFilter.Action
268264
accessType := roleFilter.AccessType
269265
entity := roleFilter.Entity
@@ -319,7 +315,7 @@ func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForOtherEntity(roleFilte
319315
return policiesToBeAdded, nil
320316
}
321317

322-
func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForJobsEntity(roleFilter bean.RoleFilter, userId int32, model *repository.RoleGroup, existingRoles map[int]*repository.RoleGroupRoleMapping, token string, managerAuth func(resource string, token string, object string) bool, tx *pg.Tx, capacity int) ([]casbin2.Policy, error) {
318+
func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForJobsEntity(roleFilter bean.RoleFilter, userId int32, model *repository.RoleGroup, existingRoles map[int]*repository.RoleGroupRoleMapping, tx *pg.Tx, capacity int) ([]casbin2.Policy, error) {
323319
actionType := roleFilter.Action
324320
accessType := roleFilter.AccessType
325321
entity := roleFilter.Entity
@@ -372,7 +368,8 @@ func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForJobsEntity(roleFilter
372368
return policiesToBeAdded, nil
373369
}
374370

375-
func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token string, managerAuth func(resource, token string, object string) bool) (*bean.RoleGroup, error) {
371+
func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token string, checkRBACForGroupUpdate func(token string, groupInfo *bean.RoleGroup,
372+
eliminatedRoleFilters []*repository.RoleModel) (isAuthorised bool, err error)) (*bean.RoleGroup, error) {
376373
dbConnection := impl.roleGroupRepository.GetConnection()
377374
tx, err := dbConnection.Begin()
378375
if err != nil {
@@ -404,6 +401,8 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token
404401
var eliminatedPolicies []casbin2.Policy
405402
capacity, mapping := impl.userCommonService.GetCapacityForRoleFilter(request.RoleFilters)
406403
var policies = make([]casbin2.Policy, 0, capacity)
404+
var eliminatedRoleModels []*repository.RoleModel
405+
var items []casbin2.Policy
407406
if request.SuperAdmin == false {
408407
roleGroupMappingModels, err := impl.roleGroupRepository.GetRoleGroupRoleMappingByRoleGroupId(roleGroup.Id)
409408
if err != nil {
@@ -417,7 +416,7 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token
417416

418417
// DELETE PROCESS STARTS
419418

420-
items, err := impl.userCommonService.RemoveRolesAndReturnEliminatedPoliciesForGroups(request, existingRoles, eliminatedRoles, tx, token, managerAuth)
419+
items, eliminatedRoleModels, err = impl.userCommonService.RemoveRolesAndReturnEliminatedPoliciesForGroups(request, existingRoles, eliminatedRoles, tx)
421420
if err != nil {
422421
return nil, err
423422
}
@@ -427,32 +426,24 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token
427426
//Adding New Policies
428427
for index, roleFilter := range request.RoleFilters {
429428
if roleFilter.Entity == bean.CLUSTER_ENTITIY {
430-
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForClusterEntity(roleFilter, request.UserId, roleGroup, existingRoles, token, managerAuth, tx, mapping[index])
429+
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForClusterEntity(roleFilter, request.UserId, roleGroup, existingRoles, tx, mapping[index])
431430
policies = append(policies, policiesToBeAdded...)
432431
if err != nil {
433432
impl.logger.Errorw("error in creating updating role group for cluster entity", "err", err, "roleFilter", roleFilter)
434433
}
435434
} else {
436-
if len(roleFilter.Team) > 0 {
437-
// check auth only for apps permission, skip for chart group
438-
rbacObject := fmt.Sprintf("%s", roleFilter.Team)
439-
isValidAuth := managerAuth(casbin2.ResourceUser, token, rbacObject)
440-
if !isValidAuth {
441-
continue
442-
}
443-
}
444435
switch roleFilter.Entity {
445436
case bean2.EntityJobs:
446437
{
447-
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForJobsEntity(roleFilter, request.UserId, roleGroup, existingRoles, token, managerAuth, tx, mapping[index])
438+
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForJobsEntity(roleFilter, request.UserId, roleGroup, existingRoles, tx, mapping[index])
448439
policies = append(policies, policiesToBeAdded...)
449440
if err != nil {
450441
impl.logger.Errorw("error in creating updating role group for jobs entity", "err", err, "roleFilter", roleFilter)
451442
}
452443
}
453444
default:
454445
{
455-
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForOtherEntity(roleFilter, request, roleGroup, existingRoles, token, managerAuth, tx, mapping[index])
446+
policiesToBeAdded, err := impl.CreateOrUpdateRoleGroupForOtherEntity(roleFilter, request, roleGroup, existingRoles, tx, mapping[index])
456447
policies = append(policies, policiesToBeAdded...)
457448
if err != nil {
458449
impl.logger.Errorw("error in creating updating role group for other entity", "err", err, "roleFilter", roleFilter)
@@ -486,6 +477,22 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token
486477
policies = append(policies, casbin2.Policy{Type: "g", Sub: casbin2.Subject(roleGroup.CasbinName), Obj: casbin2.Object(roleModel.Role)})
487478
}
488479
}
480+
481+
if checkRBACForGroupUpdate != nil {
482+
isAuthorised, err := checkRBACForGroupUpdate(token, request, eliminatedRoleModels)
483+
if err != nil {
484+
impl.logger.Errorw("error in checking RBAC for role group update", "err", err, "request", request)
485+
return nil, err
486+
} else if !isAuthorised {
487+
impl.logger.Errorw("rbac check failed for role group update", "request", request)
488+
return nil, &util.ApiError{
489+
Code: "403",
490+
HttpStatusCode: http.StatusForbidden,
491+
UserMessage: "unauthorized",
492+
}
493+
}
494+
}
495+
489496
//deleting policies from casbin
490497
impl.logger.Debugw("eliminated policies", "eliminatedPolicies", eliminatedPolicies)
491498
if len(eliminatedPolicies) > 0 {

0 commit comments

Comments
 (0)