Skip to content

fix: cherrypicked update User Rbac Fixes (#5929) #5940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 1, 2024
Merged

fix: cherrypicked update User Rbac Fixes (#5929) #5940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 53 additions & 40 deletions api/auth/user/UserRestHandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (handler UserRestHandlerImpl) CreateUser(w http.ResponseWriter, r *http.Req
//RBAC enforcer Ends
//In create req, we also check if any email exists already. If yes, then in that case we go on and merge existing roles and groups with the ones in request
//but rbac is only checked on create request roles and groups as existing roles and groups are assumed to be checked when created/updated before
res, err := handler.userService.CreateUser(&userInfo)
res, err := handler.userService.CreateUser(&userInfo, token, handler.CheckManagerAuth)
if err != nil {
handler.logger.Errorw("service err, CreateUser", "err", err, "payload", userInfo)
if _, ok := err.(*util.ApiError); ok {
Expand Down Expand Up @@ -192,7 +192,7 @@ func (handler UserRestHandlerImpl) UpdateUser(w http.ResponseWriter, r *http.Req
return
}

res, err := handler.userService.UpdateUser(&userInfo, token, handler.checkRBACForUserUpdate)
res, err := handler.userService.UpdateUser(&userInfo, token, handler.checkRBACForUserUpdate, handler.CheckManagerAuth)
if err != nil {
handler.logger.Errorw("service err, UpdateUser", "err", err, "payload", userInfo)
common.WriteJsonResp(w, err, "", http.StatusInternalServerError)
Expand Down Expand Up @@ -243,18 +243,9 @@ func (handler UserRestHandlerImpl) GetById(w http.ResponseWriter, r *http.Reques
// RBAC enforcer applying
filteredRoleFilter := make([]bean.RoleFilter, 0)
if res.RoleFilters != nil && len(res.RoleFilters) > 0 {
isUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
for _, filter := range res.RoleFilters {
authPass := true
if len(filter.Team) > 0 {
if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionGet, filter.Team); !ok {
authPass = false
}
}
if filter.Entity == bean2.CLUSTER_ENTITIY {
if ok := handler.userCommonService.CheckRbacForClusterEntity(filter.Cluster, filter.Namespace, filter.Group, filter.Kind, filter.Resource, token, handler.CheckManagerAuth); !ok {
authPass = false
}
}
authPass := handler.checkRbacForFilter(token, filter, isUserSuperAdmin)
if authPass {
filteredRoleFilter = append(filteredRoleFilter, filter)
}
Expand Down Expand Up @@ -578,18 +569,9 @@ func (handler UserRestHandlerImpl) FetchRoleGroupById(w http.ResponseWriter, r *
token := r.Header.Get("token")
filteredRoleFilter := make([]bean.RoleFilter, 0)
if res.RoleFilters != nil && len(res.RoleFilters) > 0 {
isUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
for _, filter := range res.RoleFilters {
authPass := true
if len(filter.Team) > 0 {
if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionGet, filter.Team); !ok {
authPass = false
}
}
if filter.Entity == bean2.CLUSTER_ENTITIY {
if isValidAuth := handler.userCommonService.CheckRbacForClusterEntity(filter.Cluster, filter.Namespace, filter.Group, filter.Kind, filter.Resource, token, handler.CheckManagerAuth); !isValidAuth {
authPass = false
}
}
authPass := handler.checkRbacForFilter(token, filter, isUserSuperAdmin)
if authPass {
filteredRoleFilter = append(filteredRoleFilter, filter)
}
Expand All @@ -610,6 +592,35 @@ func (handler UserRestHandlerImpl) FetchRoleGroupById(w http.ResponseWriter, r *
common.WriteJsonResp(w, err, res, http.StatusOK)
}

func (handler UserRestHandlerImpl) checkRbacForFilter(token string, filter bean.RoleFilter, isUserSuperAdmin bool) bool {
isAuthorised := true
switch {
case isUserSuperAdmin:
isAuthorised = true
case filter.AccessType == bean2.APP_ACCESS_TYPE_HELM || filter.Entity == bean2.EntityJobs:
if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok {
isAuthorised = false
}

case len(filter.Team) > 0:
// this is case of devtron app
if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionGet, filter.Team); !ok {
isAuthorised = false
}

case filter.Entity == bean.CLUSTER_ENTITIY:
isValidAuth := handler.userCommonService.CheckRbacForClusterEntity(filter.Cluster, filter.Namespace, filter.Group, filter.Kind, filter.Resource, token, handler.CheckManagerAuth)
if !isValidAuth {
isAuthorised = false
}
case filter.Entity == bean.CHART_GROUP_ENTITY:
isAuthorised = true
default:
isAuthorised = false
}
return isAuthorised
}

func (handler UserRestHandlerImpl) CreateRoleGroup(w http.ResponseWriter, r *http.Request) {
decoder := json.NewDecoder(r.Body)
userId, err := handler.userService.GetLoggedInUser(r)
Expand Down Expand Up @@ -698,7 +709,7 @@ func (handler UserRestHandlerImpl) UpdateRoleGroup(w http.ResponseWriter, r *htt
return
}

res, err := handler.roleGroupService.UpdateRoleGroup(&request, token, handler.checkRBACForRoleGroupUpdate)
res, err := handler.roleGroupService.UpdateRoleGroup(&request, token, handler.checkRBACForRoleGroupUpdate, handler.CheckManagerAuth)
if err != nil {
handler.logger.Errorw("service err, UpdateRoleGroup", "err", err, "payload", request)
common.WriteJsonResp(w, err, "", http.StatusInternalServerError)
Expand Down Expand Up @@ -908,7 +919,7 @@ func (handler UserRestHandlerImpl) DeleteRoleGroup(w http.ResponseWriter, r *htt
return
}
token := r.Header.Get("token")
isAuthorised, err := handler.checkRBACForRoleGroupDelete(token, userGroup.RoleFilters)
isAuthorised, err := handler.checkRBACForRoleGroupDelete(token, userGroup)
if err != nil {
common.WriteJsonResp(w, err, "", http.StatusInternalServerError)
return
Expand Down Expand Up @@ -1138,7 +1149,7 @@ func (handler UserRestHandlerImpl) checkRBACForUserCreate(token string, requestS
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
}
Expand Down Expand Up @@ -1167,7 +1178,7 @@ func (handler UserRestHandlerImpl) checkRBACForUserCreate(token string, requestS
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
} else {
Expand Down Expand Up @@ -1206,7 +1217,7 @@ func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
}
Expand All @@ -1225,7 +1236,7 @@ func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
}
Expand All @@ -1252,7 +1263,7 @@ func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
} else {
Expand All @@ -1263,11 +1274,10 @@ func (handler UserRestHandlerImpl) checkRBACForUserUpdate(token string, userInfo
return isAuthorised, nil
}

func (handler UserRestHandlerImpl) checkRBACForRoleGroupUpdate(token string, groupInfo *bean.RoleGroup,
eliminatedRoleFilters []*repository.RoleModel) (isAuthorised bool, err error) {
func (handler UserRestHandlerImpl) checkRBACForRoleGroupUpdate(token string, groupInfo *bean.RoleGroup, eliminatedRoleFilters []*repository.RoleModel, isRoleGroupAlreadySuperAdmin bool) (isAuthorised bool, err error) {
isActionUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
requestSuperAdmin := groupInfo.SuperAdmin
if requestSuperAdmin && !isActionUserSuperAdmin {
if (requestSuperAdmin || isRoleGroupAlreadySuperAdmin) && !isActionUserSuperAdmin {
//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
return false, nil
}
Expand All @@ -1290,7 +1300,7 @@ func (handler UserRestHandlerImpl) checkRBACForRoleGroupUpdate(token string, gro
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
}
Expand All @@ -1309,20 +1319,23 @@ func (handler UserRestHandlerImpl) checkRBACForRoleGroupUpdate(token string, gro
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
}
}
return isAuthorised, nil
}

func (handler UserRestHandlerImpl) checkRBACForRoleGroupDelete(token string, groupRoles []bean.RoleFilter) (isAuthorised bool, err error) {
func (handler UserRestHandlerImpl) checkRBACForRoleGroupDelete(token string, userGroup *bean.RoleGroup) (isAuthorised bool, err error) {
isActionUserSuperAdmin := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*")
if userGroup.SuperAdmin && !isActionUserSuperAdmin {
return false, nil
}
isAuthorised = isActionUserSuperAdmin
if !isAuthorised {
if groupRoles != nil && len(groupRoles) > 0 { //auth check inside roleFilters
for _, filter := range groupRoles {
if userGroup.RoleFilters != nil && len(userGroup.RoleFilters) > 0 { //auth check inside roleFilters
for _, filter := range userGroup.RoleFilters {
switch {
case filter.Action == bean.ACTION_SUPERADMIN:
isAuthorised = isActionUserSuperAdmin
Expand All @@ -1338,7 +1351,7 @@ func (handler UserRestHandlerImpl) checkRBACForRoleGroupDelete(token string, gro
isAuthorised = false
}
if !isAuthorised {
break
return false, nil
}
}
}
Expand Down
31 changes: 17 additions & 14 deletions api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,28 +714,31 @@ func (handler *PipelineConfigRestHandlerImpl) TriggerCiPipeline(w http.ResponseW
return
}
cdPipelineRbacObjects := make([]string, len(cdPipelines))
rbacObjectToCdPipeline := make(map[string]*pipelineConfig.Pipeline)
rbacObjectCdTriggerTypeMap := make(map[string]pipelineConfig.TriggerType, len(cdPipelines))
for i, cdPipeline := range cdPipelines {
envObject := handler.enforcerUtil.GetAppRBACByAppIdAndPipelineId(cdPipeline.AppId, cdPipeline.Id)
cdPipelineRbacObjects[i] = envObject
rbacObjectToCdPipeline[envObject] = cdPipeline
rbacObjectCdTriggerTypeMap[envObject] = cdPipeline.TriggerType
}

hasAnyEnvTriggerAccess := false
envRbacResultMap := handler.enforcer.EnforceInBatch(token, casbin.ResourceEnvironment, casbin.ActionTrigger, cdPipelineRbacObjects)
for rbacObject, rbacResultOk := range envRbacResultMap {
cdPipeline := rbacObjectToCdPipeline[rbacObject]
if cdPipeline.TriggerType == pipelineConfig.TRIGGER_TYPE_AUTOMATIC && !rbacResultOk {
hasAnyEnvTriggerAccess := len(cdPipelines) == 0 //if no pipelines then appAccess is enough. For jobs also, this will be true
if !hasAnyEnvTriggerAccess {
//cdPipelines present, to check access for cd trigger
envRbacResultMap := handler.enforcer.EnforceInBatch(token, casbin.ResourceEnvironment, casbin.ActionTrigger, cdPipelineRbacObjects)
for rbacObject, rbacResultOk := range envRbacResultMap {
if rbacObjectCdTriggerTypeMap[rbacObject] == pipelineConfig.TRIGGER_TYPE_AUTOMATIC && !rbacResultOk {
common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden)
return
}
if rbacResultOk { //this flow will come if pipeline is automatic and has access or if pipeline is manual,
// by which we can ensure if there are no automatic pipelines then atleast access on one manual is present
hasAnyEnvTriggerAccess = true
}
}
if !hasAnyEnvTriggerAccess {
common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden)
return
}
if rbacResultOk {
hasAnyEnvTriggerAccess = true
}
}
if !hasAnyEnvTriggerAccess {
common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden)
return
}

//RBAC ENDS
Expand Down
4 changes: 2 additions & 2 deletions cmd/external-app/wire_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/apiToken/ApiTokenService.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func (impl ApiTokenServiceImpl) CreateApiToken(request *openapi.CreateApiTokenRe
EmailId: email,
UserType: bean.USER_TYPE_API_TOKEN,
}
createUserResponse, err := impl.userService.CreateUser(&createUserRequest)
createUserResponse, err := impl.userService.CreateUser(&createUserRequest, token, managerAuth)
if err != nil {
impl.logger.Errorw("error while creating user for api-token", "email", email, "error", err)
return nil, err
Expand Down
31 changes: 27 additions & 4 deletions pkg/auth/user/RoleGroupService.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (
type RoleGroupService interface {
CreateRoleGroup(request *bean.RoleGroup) (*bean.RoleGroup, error)
UpdateRoleGroup(request *bean.RoleGroup, token string, checkRBACForGroupUpdate func(token string, groupInfo *bean.RoleGroup,
eliminatedRoleFilters []*repository.RoleModel) (isAuthorised bool, err error)) (*bean.RoleGroup, error)
eliminatedRoleFilters []*repository.RoleModel, isRoleGroupAlreadySuperAdmin bool) (isAuthorised bool, err error), managerAuth func(resource, token string, object string) bool) (*bean.RoleGroup, error)
FetchDetailedRoleGroups(req *bean.ListingRequest) ([]*bean.RoleGroup, error)
FetchRoleGroupsById(id int32) (*bean.RoleGroup, error)
FetchRoleGroups() ([]*bean.RoleGroup, error)
Expand Down Expand Up @@ -368,8 +368,25 @@ func (impl RoleGroupServiceImpl) CreateOrUpdateRoleGroupForJobsEntity(roleFilter
return policiesToBeAdded, nil
}

func (impl RoleGroupServiceImpl) checkIfRoleGroupSuperAdmin(casbinName string) (bool, error) {
rolesModels, err := impl.roleGroupRepository.GetRolesByGroupCasbinNames([]string{casbinName})
if err != nil {
impl.logger.Errorw("error in getting roles by group names", "err", err)
return false, err
}
isSuperAdmin := false
for _, roleModel := range rolesModels {
if roleModel.Action == bean2.SUPER_ADMIN {
isSuperAdmin = true
break
}
}
return isSuperAdmin, nil

}

func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token string, checkRBACForGroupUpdate func(token string, groupInfo *bean.RoleGroup,
eliminatedRoleFilters []*repository.RoleModel) (isAuthorised bool, err error)) (*bean.RoleGroup, error) {
eliminatedRoleFilters []*repository.RoleModel, isRoleGroupAlreadySuperAdmin bool) (isAuthorised bool, err error), managerAuth func(resource, token string, object string) bool) (*bean.RoleGroup, error) {
dbConnection := impl.roleGroupRepository.GetConnection()
tx, err := dbConnection.Begin()
if err != nil {
Expand All @@ -384,6 +401,12 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token
return nil, err
}

isRGSuperAdmin, err := impl.checkIfRoleGroupSuperAdmin(roleGroup.CasbinName)
if err != nil {
impl.logger.Errorw("error encountered in UpdateRoleGroup", "error", err, "roleGroupId", roleGroup.Id)
return nil, err
}

//policyGroup.Name = request.Name
roleGroup.Description = request.Description
roleGroup.UpdatedOn = time.Now()
Expand Down Expand Up @@ -416,7 +439,7 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token

// DELETE PROCESS STARTS

items, eliminatedRoleModels, err = impl.userCommonService.RemoveRolesAndReturnEliminatedPoliciesForGroups(request, existingRoles, eliminatedRoles, tx)
items, eliminatedRoleModels, err = impl.userCommonService.RemoveRolesAndReturnEliminatedPoliciesForGroups(request, existingRoles, eliminatedRoles, tx, token, managerAuth)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -476,7 +499,7 @@ func (impl RoleGroupServiceImpl) UpdateRoleGroup(request *bean.RoleGroup, token
}

if checkRBACForGroupUpdate != nil {
isAuthorised, err := checkRBACForGroupUpdate(token, request, eliminatedRoleModels)
isAuthorised, err := checkRBACForGroupUpdate(token, request, eliminatedRoleModels, isRGSuperAdmin)
if err != nil {
impl.logger.Errorw("error in checking RBAC for role group update", "err", err, "request", request)
return nil, err
Expand Down
Loading