From 7520166f1765be61250fc643db935a858db4531b Mon Sep 17 00:00:00 2001
From: Sofia Papagiannaki <papagian@users.noreply.github.com>
Date: Thu, 8 Aug 2019 11:27:47 +0300
Subject: [PATCH] API: Minor fix for team creation endpoint when using API key
 (#18252)

* Fix CreateTeam api endpoint

No team member should be created for requests
authenticated by API tokens.

* Update middleware test

Assert that `isAnonymous` is set for `SignedInUser`
authenticated via API key.

* Add test for team creation

Assert that no team member is created if the signed in user
is anomymous.

* Revert "Fix CreateTeam api endpoint"

This reverts commit 9fcc4e67f589008d7c44097f5cf08438c09c3c05.

* Revert "Update middleware test"

This reverts commit 75f767e58d212e21a351efea14bed79bbf881d2e.

* Fix CreateTeam api endpoint

No team member should be created for requests
authenticated by API tokens.

* Update team test

* Change error to warning and update tests
---
 pkg/api/team.go      | 23 ++++++++-----
 pkg/api/team_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++
 pkg/models/user.go   |  4 +++
 3 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/pkg/api/team.go b/pkg/api/team.go
index ecfd8028c1b..7ba9b738b90 100644
--- a/pkg/api/team.go
+++ b/pkg/api/team.go
@@ -24,15 +24,22 @@ func (hs *HTTPServer) CreateTeam(c *m.ReqContext, cmd m.CreateTeamCommand) Respo
 	}
 
 	if c.OrgRole == m.ROLE_EDITOR && hs.Cfg.EditorsCanAdmin {
-		addMemberCmd := m.AddTeamMemberCommand{
-			UserId:     c.SignedInUser.UserId,
-			OrgId:      cmd.OrgId,
-			TeamId:     cmd.Result.Id,
-			Permission: m.PERMISSION_ADMIN,
-		}
+		// if the request is authenticated using API tokens
+		// the SignedInUser is an empty struct therefore
+		// an additional check whether it is an actual user is required
+		if c.SignedInUser.IsRealUser() {
+			addMemberCmd := m.AddTeamMemberCommand{
+				UserId:     c.SignedInUser.UserId,
+				OrgId:      cmd.OrgId,
+				TeamId:     cmd.Result.Id,
+				Permission: m.PERMISSION_ADMIN,
+			}
 
-		if err := hs.Bus.Dispatch(&addMemberCmd); err != nil {
-			c.Logger.Error("Could not add creator to team.", "error", err)
+			if err := hs.Bus.Dispatch(&addMemberCmd); err != nil {
+				c.Logger.Error("Could not add creator to team.", "error", err)
+			}
+		} else {
+			c.Logger.Warn("Could not add creator to team because is not a real user.")
 		}
 	}
 
diff --git a/pkg/api/team_test.go b/pkg/api/team_test.go
index cab59cc5f98..a52522381fa 100644
--- a/pkg/api/team_test.go
+++ b/pkg/api/team_test.go
@@ -9,9 +9,24 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/models"
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/assert"
+	macaron "gopkg.in/macaron.v1"
+	"net/http"
 )
 
+type testLogger struct {
+	log.Logger
+	warnCalled  bool
+	warnMessage string
+}
+
+func (stub *testLogger) Warn(testMessage string, ctx ...interface{}) {
+	stub.warnCalled = true
+	stub.warnMessage = testMessage
+}
+
 func TestTeamApiEndpoint(t *testing.T) {
 	Convey("Given two teams", t, func() {
 		mockResult := models.SearchTeamQueryResult{
@@ -74,4 +89,67 @@ func TestTeamApiEndpoint(t *testing.T) {
 			})
 		})
 	})
+
+	t.Run("When creating team with api key", func(t *testing.T) {
+		defer bus.ClearBusHandlers()
+
+		hs := &HTTPServer{
+			Cfg: setting.NewCfg(),
+			Bus: bus.GetBus(),
+		}
+		hs.Cfg.EditorsCanAdmin = true
+
+		teamName := "team foo"
+
+		createTeamCalled := 0
+		bus.AddHandler("test", func(cmd *models.CreateTeamCommand) error {
+			createTeamCalled += 1
+			cmd.Result = models.Team{Name: teamName, Id: 42}
+			return nil
+		})
+
+		addTeamMemberCalled := 0
+		bus.AddHandler("test", func(cmd *models.AddTeamMemberCommand) error {
+			addTeamMemberCalled += 1
+			return nil
+		})
+
+		req, _ := http.NewRequest("POST", "/api/teams", nil)
+
+		t.Run("with no real signed in user", func(t *testing.T) {
+			stub := &testLogger{}
+			c := &models.ReqContext{
+				Context: &macaron.Context{
+					Req: macaron.Request{Request: req},
+				},
+				SignedInUser: &models.SignedInUser{},
+				Logger:       stub,
+			}
+			c.OrgRole = models.ROLE_EDITOR
+			cmd := models.CreateTeamCommand{Name: teamName}
+			hs.CreateTeam(c, cmd)
+			assert.Equal(t, createTeamCalled, 1)
+			assert.Equal(t, addTeamMemberCalled, 0)
+			assert.True(t, stub.warnCalled)
+			assert.Equal(t, stub.warnMessage, "Could not add creator to team because is not a real user.")
+		})
+
+		t.Run("with real signed in user", func(t *testing.T) {
+			stub := &testLogger{}
+			c := &models.ReqContext{
+				Context: &macaron.Context{
+					Req: macaron.Request{Request: req},
+				},
+				SignedInUser: &models.SignedInUser{UserId: 42},
+				Logger:       stub,
+			}
+			c.OrgRole = models.ROLE_EDITOR
+			cmd := models.CreateTeamCommand{Name: teamName}
+			createTeamCalled, addTeamMemberCalled = 0, 0
+			hs.CreateTeam(c, cmd)
+			assert.Equal(t, createTeamCalled, 1)
+			assert.Equal(t, addTeamMemberCalled, 1)
+			assert.False(t, stub.warnCalled)
+		})
+	})
 }
diff --git a/pkg/models/user.go b/pkg/models/user.go
index 6096e54765f..491d5e32d5c 100644
--- a/pkg/models/user.go
+++ b/pkg/models/user.go
@@ -210,6 +210,10 @@ func (user *SignedInUser) HasRole(role RoleType) bool {
 	return user.OrgRole.Includes(role)
 }
 
+func (user *SignedInUser) IsRealUser() bool {
+	return user.UserId != 0
+}
+
 type UserProfileDTO struct {
 	Id             int64    `json:"id"`
 	Email          string   `json:"email"`