From 4fabb844e2f3b733c3c1ce5b4562dd4d0a2dd75a Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 12 Aug 2022 01:16:01 +0530 Subject: [PATCH 1/3] updated auth for user list fetch --- api/user/UserRestHandler.go | 46 ++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/api/user/UserRestHandler.go b/api/user/UserRestHandler.go index 9ec544e5eb..884dbe1b51 100644 --- a/api/user/UserRestHandler.go +++ b/api/user/UserRestHandler.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "github.com/devtron-labs/devtron/api/restHandler/common" + "github.com/devtron-labs/devtron/pkg/team" "github.com/devtron-labs/devtron/pkg/user/casbin" "net/http" "strconv" @@ -68,12 +69,20 @@ type UserRestHandlerImpl struct { logger *zap.SugaredLogger enforcer casbin.Enforcer roleGroupService user.RoleGroupService + teamRepository team.TeamRepository } func NewUserRestHandlerImpl(userService user.UserService, validator *validator.Validate, - logger *zap.SugaredLogger, enforcer casbin.Enforcer, roleGroupService user.RoleGroupService) *UserRestHandlerImpl { - userAuthHandler := &UserRestHandlerImpl{userService: userService, validator: validator, logger: logger, - enforcer: enforcer, roleGroupService: roleGroupService} + logger *zap.SugaredLogger, enforcer casbin.Enforcer, roleGroupService user.RoleGroupService, + teamRepository team.TeamRepository) *UserRestHandlerImpl { + userAuthHandler := &UserRestHandlerImpl{ + userService: userService, + validator: validator, + logger: logger, + enforcer: enforcer, + roleGroupService: roleGroupService, + teamRepository: teamRepository, + } return userAuthHandler } @@ -261,6 +270,37 @@ func (handler UserRestHandlerImpl) GetById(w http.ResponseWriter, r *http.Reques } func (handler UserRestHandlerImpl) GetAll(w http.ResponseWriter, r *http.Request) { + userId, err := handler.userService.GetLoggedInUser(r) + if userId == 0 || err != nil { + common.WriteJsonResp(w, err, "Unauthorized User", http.StatusUnauthorized) + return + } + + // RBAC enforcer applying + token := r.Header.Get("token") + isAuthorised := false + //checking for superadmin access + if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); ok { + isAuthorised = true + } + //getting all projects + teams, err := handler.teamRepository.FindAllActive() + if err != nil { + handler.logger.Errorw("error in getting all active teams", "err", err) + common.WriteJsonResp(w, err, "", http.StatusInternalServerError) + return + } + for _, team := range teams { + //checking if user has manager access to atleast one team, if yes then the user is authorised + if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionDelete, strings.ToLower(team.Name)); ok { + isAuthorised = true + break + } + } + if !isAuthorised { + common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) + return + } res, err := handler.userService.GetAll() if err != nil { handler.logger.Errorw("service err, GetAll", "err", err) From 32c1071839b199e54d4763a6e5dceb0c2d8e601d Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 12 Aug 2022 17:43:29 +0530 Subject: [PATCH 2/3] updated manager rbac check --- api/user/UserRestHandler.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/api/user/UserRestHandler.go b/api/user/UserRestHandler.go index 884dbe1b51..09086e53bf 100644 --- a/api/user/UserRestHandler.go +++ b/api/user/UserRestHandler.go @@ -282,19 +282,20 @@ func (handler UserRestHandlerImpl) GetAll(w http.ResponseWriter, r *http.Request //checking for superadmin access if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); ok { isAuthorised = true - } - //getting all projects - teams, err := handler.teamRepository.FindAllActive() - if err != nil { - handler.logger.Errorw("error in getting all active teams", "err", err) - common.WriteJsonResp(w, err, "", http.StatusInternalServerError) - return - } - for _, team := range teams { - //checking if user has manager access to atleast one team, if yes then the user is authorised - if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionDelete, strings.ToLower(team.Name)); ok { - isAuthorised = true - break + } else { + //getting all projects + teams, err := handler.teamRepository.FindAllActive() + if err != nil { + handler.logger.Errorw("error in getting all active teams", "err", err) + common.WriteJsonResp(w, err, "", http.StatusInternalServerError) + return + } + for _, team := range teams { + //checking if user has manager access to atleast one team, if yes then the user is authorised + if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionDelete, strings.ToLower(team.Name)); ok { + isAuthorised = true + break + } } } if !isAuthorised { From 1438a3903dcb8a911361b7e0673032bfb0b409ba Mon Sep 17 00:00:00 2001 From: Kartik Singhal Date: Fri, 12 Aug 2022 18:41:43 +0530 Subject: [PATCH 3/3] review change --- api/user/UserRestHandler.go | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/api/user/UserRestHandler.go b/api/user/UserRestHandler.go index 884dbe1b51..977b8d8011 100644 --- a/api/user/UserRestHandler.go +++ b/api/user/UserRestHandler.go @@ -22,7 +22,6 @@ import ( "errors" "fmt" "github.com/devtron-labs/devtron/api/restHandler/common" - "github.com/devtron-labs/devtron/pkg/team" "github.com/devtron-labs/devtron/pkg/user/casbin" "net/http" "strconv" @@ -69,19 +68,16 @@ type UserRestHandlerImpl struct { logger *zap.SugaredLogger enforcer casbin.Enforcer roleGroupService user.RoleGroupService - teamRepository team.TeamRepository } func NewUserRestHandlerImpl(userService user.UserService, validator *validator.Validate, - logger *zap.SugaredLogger, enforcer casbin.Enforcer, roleGroupService user.RoleGroupService, - teamRepository team.TeamRepository) *UserRestHandlerImpl { + logger *zap.SugaredLogger, enforcer casbin.Enforcer, roleGroupService user.RoleGroupService) *UserRestHandlerImpl { userAuthHandler := &UserRestHandlerImpl{ userService: userService, validator: validator, logger: logger, enforcer: enforcer, roleGroupService: roleGroupService, - teamRepository: teamRepository, } return userAuthHandler } @@ -279,22 +275,29 @@ func (handler UserRestHandlerImpl) GetAll(w http.ResponseWriter, r *http.Request // RBAC enforcer applying token := r.Header.Get("token") isAuthorised := false - //checking for superadmin access - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); ok { - isAuthorised = true - } - //getting all projects - teams, err := handler.teamRepository.FindAllActive() + //checking superAdmin access + isAuthorised, err = handler.userService.IsSuperAdmin(int(userId)) if err != nil { - handler.logger.Errorw("error in getting all active teams", "err", err) + handler.logger.Errorw("error in checking superAdmin access of user", "err", err) common.WriteJsonResp(w, err, "", http.StatusInternalServerError) return } - for _, team := range teams { - //checking if user has manager access to atleast one team, if yes then the user is authorised - if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionDelete, strings.ToLower(team.Name)); ok { - isAuthorised = true - break + if !isAuthorised { + user, err := handler.userService.GetById(userId) + if err != nil { + handler.logger.Errorw("error in getting user by id", "err", err) + common.WriteJsonResp(w, err, "", http.StatusInternalServerError) + return + } + if user.RoleFilters != nil && len(user.RoleFilters) > 0 { + for _, filter := range user.RoleFilters { + if len(filter.Team) > 0 { + if ok := handler.enforcer.Enforce(token, casbin.ResourceUser, casbin.ActionGet, strings.ToLower(filter.Team)); ok { + isAuthorised = true + break + } + } + } } } if !isAuthorised {