Merge pull request #55 from devtron-labs/user-get-forbidden-issue

User get for super admin forbidden issue

Author: nishant-d

Diff for: api/restHandler/UserRestHandler.go

@@ -192,6 +192,11 @@ func (handler UserRestHandlerImpl) UpdateUser(w http.ResponseWriter, r *http.Req
 }
 
 func (handler UserRestHandlerImpl) GetById(w http.ResponseWriter, r *http.Request) {
+	userId, err := handler.userService.GetLoggedInUser(r)
+	if userId == 0 || err != nil {
+		writeJsonResp(w, err, "Unauthorized User", http.StatusUnauthorized)
+		return
+	}
 	vars := mux.Vars(r)
 	/* #nosec */
 	id, err := strconv.Atoi(vars["id"])
@@ -206,7 +211,12 @@ func (handler UserRestHandlerImpl) GetById(w http.ResponseWriter, r *http.Reques
 		writeJsonResp(w, err, "Failed to get by id", http.StatusInternalServerError)
 		return
 	}
-
+	isActionUserSuperAdmin, err := handler.userService.IsSuperAdmin(int(userId))
+	if err != nil {
+		handler.logger.Errorw("service err, GetById", "err", err, "id", id)
+		writeJsonResp(w, err, "Failed to check is super admin", http.StatusInternalServerError)
+		return
+	}
 	// NOTE: if no role assigned, user will be visible to all manager.
 	// RBAC enforcer applying
 	token := r.Header.Get("token")
@@ -219,6 +229,9 @@ func (handler UserRestHandlerImpl) GetById(w http.ResponseWriter, r *http.Reques
 				}
 			}
 		}
+		if isActionUserSuperAdmin {
+			authPass = true
+		}
 		if authPass == false {
 			response.WriteResponse(http.StatusForbidden, "FORBIDDEN", w, errors.New("unauthorized"))
 			return

Diff for: pkg/user/UserService.go

@@ -49,6 +49,7 @@ type UserService interface {
 	CheckUserRoles(id int32) ([]string, error)
 	SyncOrchestratorToCasbin() (bool, error)
 	GetUserByToken(token string) (int32, error)
+	IsSuperAdmin(userId int) (bool, error)
 }
 
 type UserServiceImpl struct {
@@ -311,41 +312,27 @@ func (impl UserServiceImpl) CreateUser(userInfo *bean.UserInfo) ([]*bean.UserInf
 }
 
 func (impl UserServiceImpl) UpdateUser(userInfo *bean.UserInfo) (*bean.UserInfo, error) {
-
 	//validating if action user is not admin and trying to update user who has super admin polices, return 403
-	isSuperAdmin := false
-	userCasbinRoles, err := impl.CheckUserRoles(userInfo.Id)
+	isUserSuperAdmin, err := impl.IsSuperAdmin(int(userInfo.Id))
 	if err != nil {
 		return nil, err
 	}
-	actionUserRoles, err := impl.CheckUserRoles(userInfo.UserId)
+	isActionPerformingUserSuperAdmin, err := impl.IsSuperAdmin(int(userInfo.UserId))
 	if err != nil {
 		return nil, err
 	}
 	//if request comes to make user as a super admin, action performing user also be super admin
 	if userInfo.SuperAdmin {
-		for _, item := range actionUserRoles {
-			if item == bean.SUPERADMIN {
-				isSuperAdmin = true
-			}
-		}
-		if isSuperAdmin == false {
+		if !isUserSuperAdmin {
 			err = &util.ApiError{HttpStatusCode: http.StatusForbidden, UserMessage: "Invalid request, not allow to update super admin type user"}
 			return nil, err
 		}
 	}
 	//if user which going to updated is super admin, action performing user also be super admin
-	for _, item := range userCasbinRoles {
-		if item == bean.SUPERADMIN {
-			for _, item := range actionUserRoles {
-				if item == bean.SUPERADMIN {
-					isSuperAdmin = true
-				}
-			}
-			if isSuperAdmin == false {
-				err = &util.ApiError{HttpStatusCode: http.StatusForbidden, UserMessage: "Invalid request, not allow to update super admin type user"}
-				return nil, err
-			}
+	if isUserSuperAdmin {
+		if !isActionPerformingUserSuperAdmin {
+			err = &util.ApiError{HttpStatusCode: http.StatusForbidden, UserMessage: "Invalid request, not allow to update super admin type user"}
+			return nil, err
 		}
 	}
 
@@ -525,6 +512,10 @@ func (impl UserServiceImpl) UpdateUser(userInfo *bean.UserInfo) (*bean.UserInfo,
 		//ROLE GROUP SETUP
 		newGroupMap := make(map[string]string)
 		oldGroupMap := make(map[string]string)
+		userCasbinRoles, err := impl.CheckUserRoles(userInfo.Id)
+		if err != nil {
+			return nil, err
+		}
 		for _, oldItem := range userCasbinRoles {
 			oldGroupMap[oldItem] = oldItem
 		}
@@ -944,3 +935,20 @@ func (impl UserServiceImpl) SyncOrchestratorToCasbin() (bool, error) {
 	}*/
 	return true, nil
 }
+
+func (impl UserServiceImpl) IsSuperAdmin(userId int) (bool, error) {
+	//validating if action user is not admin and trying to update user who has super admin polices, return 403
+	isSuperAdmin := false
+	userCasbinRoles, err := impl.CheckUserRoles(int32(userId))
+	if err != nil {
+		return isSuperAdmin, err
+	}
+	//if user which going to updated is super admin, action performing user also be super admin
+	for _, item := range userCasbinRoles {
+		if item == bean.SUPERADMIN {
+			isSuperAdmin = true
+			break
+		}
+	}
+	return isSuperAdmin, nil
+}