diff --git a/pkg/api/org.go b/pkg/api/org.go index 2512938837b..d5b5c308059 100644 --- a/pkg/api/org.go +++ b/pkg/api/org.go @@ -34,10 +34,10 @@ func (hs *HTTPServer) GetOrgByName(c *models.ReqContext) response.Response { org, err := hs.SQLStore.GetOrgByName(web.Params(c.Req)[":name"]) if err != nil { if errors.Is(err, models.ErrOrgNotFound) { - return response.Error(404, "Organization not found", err) + return response.Error(http.StatusNotFound, "Organization not found", err) } - return response.Error(500, "Failed to get organization", err) + return response.Error(http.StatusInternalServerError, "Failed to get organization", err) } result := models.OrgDetailsDTO{ Id: org.Id, @@ -60,9 +60,9 @@ func (hs *HTTPServer) getOrgHelper(ctx context.Context, orgID int64) response.Re if err := hs.SQLStore.GetOrgById(ctx, &query); err != nil { if errors.Is(err, models.ErrOrgNotFound) { - return response.Error(404, "Organization not found", err) + return response.Error(http.StatusNotFound, "Organization not found", err) } - return response.Error(500, "Failed to get organization", err) + return response.Error(http.StatusInternalServerError, "Failed to get organization", err) } org := query.Result @@ -90,15 +90,15 @@ func (hs *HTTPServer) CreateOrg(c *models.ReqContext) response.Response { } acEnabled := !hs.AccessControl.IsDisabled() if !acEnabled && !(setting.AllowUserOrgCreate || c.IsGrafanaAdmin) { - return response.Error(403, "Access denied", nil) + return response.Error(http.StatusForbidden, "Access denied", nil) } cmd.UserId = c.UserId if err := hs.SQLStore.CreateOrg(c.Req.Context(), &cmd); err != nil { if errors.Is(err, models.ErrOrgNameTaken) { - return response.Error(409, "Organization name taken", err) + return response.Error(http.StatusConflict, "Organization name taken", err) } - return response.Error(500, "Failed to create organization", err) + return response.Error(http.StatusInternalServerError, "Failed to create organization", err) } metrics.MApiOrgCreate.Inc() @@ -135,9 +135,9 @@ func (hs *HTTPServer) updateOrgHelper(ctx context.Context, form dtos.UpdateOrgFo cmd := models.UpdateOrgCommand{Name: form.Name, OrgId: orgID} if err := hs.SQLStore.UpdateOrg(ctx, &cmd); err != nil { if errors.Is(err, models.ErrOrgNameTaken) { - return response.Error(400, "Organization name taken", err) + return response.Error(http.StatusBadRequest, "Organization name taken", err) } - return response.Error(500, "Failed to update organization", err) + return response.Error(http.StatusInternalServerError, "Failed to update organization", err) } return response.Success("Organization updated") @@ -179,7 +179,7 @@ func (hs *HTTPServer) updateOrgAddressHelper(ctx context.Context, form dtos.Upda } if err := hs.SQLStore.UpdateOrgAddress(ctx, &cmd); err != nil { - return response.Error(500, "Failed to update org address", err) + return response.Error(http.StatusInternalServerError, "Failed to update org address", err) } return response.Success("Address updated") @@ -193,14 +193,14 @@ func (hs *HTTPServer) DeleteOrgByID(c *models.ReqContext) response.Response { } // before deleting an org, check if user does not belong to the current org if c.OrgId == orgID { - return response.Error(400, "Can not delete org for current user", nil) + return response.Error(http.StatusBadRequest, "Can not delete org for current user", nil) } if err := hs.SQLStore.DeleteOrg(c.Req.Context(), &models.DeleteOrgCommand{Id: orgID}); err != nil { if errors.Is(err, models.ErrOrgNotFound) { - return response.Error(404, "Failed to delete organization. ID not found", nil) + return response.Error(http.StatusNotFound, "Failed to delete organization. ID not found", nil) } - return response.Error(500, "Failed to update organization", err) + return response.Error(http.StatusInternalServerError, "Failed to update organization", err) } return response.Success("Organization deleted") } @@ -221,7 +221,7 @@ func (hs *HTTPServer) SearchOrgs(c *models.ReqContext) response.Response { } if err := hs.SQLStore.SearchOrgs(c.Req.Context(), &query); err != nil { - return response.Error(500, "Failed to search orgs", err) + return response.Error(http.StatusInternalServerError, "Failed to search orgs", err) } return response.JSON(http.StatusOK, query.Result) diff --git a/pkg/models/org_user.go b/pkg/models/org_user.go index 0b91f064956..b92a165dc78 100644 --- a/pkg/models/org_user.go +++ b/pkg/models/org_user.go @@ -102,6 +102,9 @@ type AddOrgUserCommand struct { OrgId int64 `json:"-"` UserId int64 `json:"-"` + + // internal use: avoid adding service accounts to orgs via user routes + AllowAddingServiceAccount bool `json:"-"` } type UpdateOrgUserCommand struct { diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 00d09b208fc..f6391889e42 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -32,29 +32,50 @@ func NewServiceAccountsStore(store *sqlstore.SQLStore, kvStore kvstore.KVStore) } // CreateServiceAccount creates service account -func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, name string) (saDTO *serviceaccounts.ServiceAccountDTO, err error) { +func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, name string) (*serviceaccounts.ServiceAccountDTO, error) { generatedLogin := "sa-" + strings.ToLower(name) generatedLogin = strings.ReplaceAll(generatedLogin, " ", "-") - cmd := user.CreateUserCommand{ - Login: generatedLogin, - OrgID: orgId, - Name: name, - IsServiceAccount: true, - } - newuser, err := s.sqlStore.CreateUser(ctx, cmd) - if err != nil { - if errors.Is(err, models.ErrUserAlreadyExists) { + var newSA *user.User + createErr := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) (err error) { + var errUser error + newSA, errUser = s.sqlStore.CreateUser(ctx, user.CreateUserCommand{ + Login: generatedLogin, + OrgID: orgId, + Name: name, + IsServiceAccount: true, + SkipOrgSetup: true, + }) + if errUser != nil { + return errUser + } + + errAddOrgUser := s.sqlStore.AddOrgUser(ctx, &models.AddOrgUserCommand{ + Role: models.ROLE_VIEWER, + OrgId: orgId, + UserId: newSA.ID, + AllowAddingServiceAccount: true, + }) + if errAddOrgUser != nil { + return errAddOrgUser + } + + return nil + }) + + if createErr != nil { + if errors.Is(createErr, models.ErrUserAlreadyExists) { return nil, ErrServiceAccountAlreadyExists } - return nil, fmt.Errorf("failed to create service account: %w", err) + + return nil, fmt.Errorf("failed to create service account: %w", createErr) } return &serviceaccounts.ServiceAccountDTO{ - Id: newuser.ID, - Name: newuser.Name, - Login: newuser.Login, - OrgId: newuser.OrgID, + Id: newSA.ID, + Name: newSA.Name, + Login: newSA.Login, + OrgId: newSA.OrgID, Tokens: 0, }, nil } diff --git a/pkg/services/serviceaccounts/database/database_test.go b/pkg/services/serviceaccounts/database/database_test.go index cb50a593196..d7d292df951 100644 --- a/pkg/services/serviceaccounts/database/database_test.go +++ b/pkg/services/serviceaccounts/database/database_test.go @@ -13,12 +13,28 @@ import ( "github.com/stretchr/testify/require" ) -func TestStore_CreateServiceAccount(t *testing.T) { +// Service Account should not create an org on its own +func TestStore_CreateServiceAccountOrgNonExistant(t *testing.T) { _, store := setupTestDatabase(t) t.Run("create service account", func(t *testing.T) { serviceAccountName := "new Service Account" serviceAccountOrgId := int64(1) + _, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, serviceAccountName) + require.Error(t, err) + }) +} + +func TestStore_CreateServiceAccount(t *testing.T) { + _, store := setupTestDatabase(t) + orgQuery := &models.CreateOrgCommand{Name: sqlstore.MainOrgName} + err := store.sqlStore.CreateOrg(context.Background(), orgQuery) + require.NoError(t, err) + + t.Run("create service account", func(t *testing.T) { + serviceAccountName := "new Service Account" + serviceAccountOrgId := orgQuery.Result.Id + saDTO, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, serviceAccountName) require.NoError(t, err) assert.Equal(t, "sa-new-service-account", saDTO.Login) diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index c7d2c0df7a3..f1fa782b18b 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -16,7 +16,12 @@ func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserComman return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { // check if user exists var user user.User - if exists, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&user); err != nil { + session := sess.ID(cmd.UserId) + if !cmd.AllowAddingServiceAccount { + session = session.Where(notServiceAccountFilter(ss)) + } + + if exists, err := session.Get(&user); err != nil { return err } else if !exists { return models.ErrUserNotFound diff --git a/pkg/services/sqlstore/org_users_test.go b/pkg/services/sqlstore/org_users_test.go index c18a42eaa48..2b9e4ad9d3b 100644 --- a/pkg/services/sqlstore/org_users_test.go +++ b/pkg/services/sqlstore/org_users_test.go @@ -148,6 +148,61 @@ func TestSQLStore_SearchOrgUsers(t *testing.T) { } } +func TestSQLStore_AddOrgUser(t *testing.T) { + var orgID int64 = 1 + store := InitTestDB(t) + + // create org and admin + _, err := store.CreateUser(context.Background(), user.CreateUserCommand{ + Login: "admin", + OrgID: orgID, + }) + require.NoError(t, err) + + // create a service account with no org + sa, err := store.CreateUser(context.Background(), user.CreateUserCommand{ + Login: "sa-no-org", + IsServiceAccount: true, + SkipOrgSetup: true, + }) + + require.NoError(t, err) + require.Equal(t, int64(-1), sa.OrgID) + + // assign the sa to the org but without the override. should fail + err = store.AddOrgUser(context.Background(), &models.AddOrgUserCommand{ + Role: "Viewer", + OrgId: orgID, + UserId: sa.ID, + }) + require.Error(t, err) + + // assign the sa to the org with the override. should succeed + err = store.AddOrgUser(context.Background(), &models.AddOrgUserCommand{ + Role: "Viewer", + OrgId: orgID, + UserId: sa.ID, + AllowAddingServiceAccount: true, + }) + + require.NoError(t, err) + + // assert the org has been correctly set + saFound := new(user.User) + err = store.WithDbSession(context.Background(), func(sess *DBSession) error { + has, err := sess.ID(sa.ID).Get(saFound) + if err != nil { + return err + } else if !has { + return models.ErrUserNotFound + } + return nil + }) + + require.NoError(t, err) + require.Equal(t, saFound.OrgID, orgID) +} + func TestSQLStore_RemoveOrgUser(t *testing.T) { store := InitTestDB(t) diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index ecca42cffaf..9b1cbddc47a 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -68,6 +68,11 @@ func (ss *SQLStore) userCaseInsensitiveLoginConflict(ctx context.Context, sess * } // createUser creates a user in the database +// if autoAssignOrg is enabled then args.OrgID will be used +// to add to an existing Org with id=args.OrgID +// if autoAssignOrg is disabled then args.OrgName will be used +// to create a new Org with name=args.OrgName. +// If a org already exists with that name, it will error func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args user.CreateUserCommand) (user.User, error) { var usr user.User var orgID int64 = -1