diff --git a/server/stores.go b/server/stores.go index 9237429f20..7a656606a4 100644 --- a/server/stores.go +++ b/server/stores.go @@ -28,17 +28,17 @@ type superAdminKey string const SuperAdminKey = superAdminKey("superadmin") -func hasSuperAdminContext(ctx context.Context) (bool, bool) { +func hasSuperAdminContext(ctx context.Context) bool { // prevents panic in case of nil context if ctx == nil { - return false, false + return false } sa, ok := ctx.Value(SuperAdminKey).(bool) // should never happen if !ok { - return false, false + return false } - return sa, true + return sa } // TODO: Comment @@ -91,7 +91,7 @@ func (s *Store) Layouts(ctx context.Context) chronograf.LayoutsStore { } func (s *Store) Users(ctx context.Context) chronograf.UsersStore { - if superAdmin, ok := hasSuperAdminContext(ctx); ok && superAdmin { + if superAdmin := hasSuperAdminContext(ctx); superAdmin { return s.UsersStore } if org, ok := hasOrganizationContext(ctx); ok { diff --git a/server/users.go b/server/users.go index 4b64888f75..979bd6b8ae 100644 --- a/server/users.go +++ b/server/users.go @@ -196,7 +196,7 @@ func (s *Service) NewUser(w http.ResponseWriter, r *http.Request) { // set the SuperAdmin field. if req.SuperAdmin == true { // Only allow users to set SuperAdmin if they have the superadmin context - if isSuperAdmin, ok := hasSuperAdminContext(ctx); !(ok && isSuperAdmin) { + if isSuperAdmin := hasSuperAdminContext(ctx); !isSuperAdmin { Error(w, http.StatusBadRequest, "Cannot set SuperAdmin", s.Logger) return } @@ -267,6 +267,19 @@ func (s *Service) UpdateUser(w http.ResponseWriter, r *http.Request) { // ValidUpdate should ensure that req.Roles is not nil u.Roles = req.Roles + // If req.SuperAdmin has been set, verify that it was done with the superadmin context. + // Even though req.SuperAdmin == true is logically equivalent to req.SuperAdmin it is + // more clear that this code should only be ran in the case that a user is trying to + // set the SuperAdmin field. + if req.SuperAdmin == true { + // Only allow users to set SuperAdmin if they have the superadmin context + if isSuperAdmin := hasSuperAdminContext(ctx); !isSuperAdmin { + Error(w, http.StatusBadRequest, "Cannot set SuperAdmin", s.Logger) + return + } + u.SuperAdmin = true + } + err = s.Store.Users(ctx).Update(ctx, u) if err != nil { Error(w, http.StatusBadRequest, err.Error(), s.Logger)