From 9b4976ef8fa3200a858b0f96423058c4594c9b6c Mon Sep 17 00:00:00 2001
From: Zach Wasserman <zach@fleetdm.com>
Date: Wed, 12 May 2021 08:31:20 -0700
Subject: [PATCH] Prevent modify user to include global and team roles (#695)

A user should have a global role or roles on some teams, but not both.
This ensures that is set properly and does validation.
---
 server/kolide/users.go           | 12 +++++++-----
 server/service/endpoint_setup.go |  4 ++--
 server/service/service_users.go  | 13 ++++++++++---
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/server/kolide/users.go b/server/kolide/users.go
index 413aa32d6d..14d9361d92 100644
--- a/server/kolide/users.go
+++ b/server/kolide/users.go
@@ -135,7 +135,7 @@ type UserPayload struct {
 	InviteToken              *string     `json:"invite_token,omitempty"`
 	SSOInvite                *bool       `json:"sso_invite,omitempty"`
 	SSOEnabled               *bool       `json:"sso_enabled,omitempty"`
-	GlobalRole               null.String `json:"global_role,omitempty"`
+	GlobalRole               *string     `json:"global_role,omitempty"`
 	AdminForcedPasswordReset *bool       `json:"admin_forced_password_reset,omitempty"`
 	Teams                    *[]UserTeam `json:"teams,omitempty"`
 }
@@ -143,10 +143,9 @@ type UserPayload struct {
 // User creates a user from payload.
 func (p UserPayload) User(keySize, cost int) (*User, error) {
 	user := &User{
-		Username:   *p.Username,
-		Email:      *p.Email,
-		Teams:      []UserTeam{},
-		GlobalRole: p.GlobalRole,
+		Username: *p.Username,
+		Email:    *p.Email,
+		Teams:    []UserTeam{},
 	}
 	if err := user.SetPassword(*p.Password, keySize, cost); err != nil {
 		return nil, err
@@ -171,6 +170,9 @@ func (p UserPayload) User(keySize, cost int) (*User, error) {
 	if p.Teams != nil {
 		user.Teams = *p.Teams
 	}
+	if p.GlobalRole != nil {
+		user.GlobalRole = null.StringFrom(*p.GlobalRole)
+	}
 
 	return user, nil
 }
diff --git a/server/service/endpoint_setup.go b/server/service/endpoint_setup.go
index 55e5722bb9..b9241d0aa0 100644
--- a/server/service/endpoint_setup.go
+++ b/server/service/endpoint_setup.go
@@ -6,7 +6,6 @@ import (
 	"github.com/fleetdm/fleet/server/kolide"
 	"github.com/go-kit/kit/endpoint"
 	"github.com/pkg/errors"
-	"gopkg.in/guregu/null.v3"
 )
 
 type setupRequest struct {
@@ -64,7 +63,8 @@ func makeSetupEndpoint(svc kolide.Service) endpoint.Endpoint {
 			return setupResponse{Err: err}, nil
 		}
 		// Make the user an admin
-		adminPayload.GlobalRole = null.StringFrom("admin")
+		adminStr := "admin"
+		adminPayload.GlobalRole = &adminStr
 		admin, err = svc.CreateUser(ctx, adminPayload)
 		if err != nil {
 			return setupResponse{Err: err}, nil
diff --git a/server/service/service_users.go b/server/service/service_users.go
index 9db618f4ab..cbd7dcded2 100644
--- a/server/service/service_users.go
+++ b/server/service/service_users.go
@@ -11,6 +11,7 @@ import (
 	"github.com/fleetdm/fleet/server/kolide"
 	"github.com/fleetdm/fleet/server/mail"
 	"github.com/pkg/errors"
+	"gopkg.in/guregu/null.v3"
 )
 
 func (svc service) CreateUserWithInvite(ctx context.Context, p kolide.UserPayload) (*kolide.User, error) {
@@ -20,7 +21,7 @@ func (svc service) CreateUserWithInvite(ctx context.Context, p kolide.UserPayloa
 	}
 
 	// set the payload role property based on an existing invite.
-	p.GlobalRole = invite.GlobalRole
+	p.GlobalRole = invite.GlobalRole.Ptr()
 	p.Teams = &invite.Teams
 
 	user, err := svc.newUser(p)
@@ -103,11 +104,17 @@ func (svc service) ModifyUser(ctx context.Context, userID uint, p kolide.UserPay
 	}
 
 	if p.Teams != nil {
+		if p.GlobalRole != nil {
+			return nil, newInvalidArgumentError("teams", "may not be specified with global_role")
+		}
+
 		user.Teams = *p.Teams
+		user.GlobalRole = null.StringFromPtr(nil)
 	}
 
-	if p.GlobalRole.Valid {
-		user.GlobalRole = p.GlobalRole
+	if p.GlobalRole != nil {
+		user.GlobalRole = null.StringFrom(*p.GlobalRole)
+		user.Teams = []kolide.UserTeam{}
 	}
 
 	err = svc.saveUser(user)