Fix error messages related to null users as creators of queries. (#1928)

Ensure that the queries page still loads even when users are deleted manually
in the DB.

Fixes #1911
This commit is contained in:
Zachary Wasserman 2018-10-11 12:37:18 -07:00
parent 863677515e
commit be6a16959a
10 changed files with 25 additions and 145 deletions

View file

@ -430,7 +430,7 @@ func testDistributedQueriesForHost(t *testing.T, ds kolide.Datastore) {
q1 := &kolide.Query{
Name: "bar",
Query: "select * from bar",
AuthorID: user.ID,
AuthorID: &user.ID,
}
q1, err = ds.NewQuery(q1)
require.Nil(t, err)
@ -475,7 +475,7 @@ func testDistributedQueriesForHost(t *testing.T, ds kolide.Datastore) {
q2 := &kolide.Query{
Name: "foo",
Query: "select * from foo",
AuthorID: user.ID,
AuthorID: &user.ID,
}
q2, err = ds.NewQuery(q2)
require.Nil(t, err)

View file

@ -31,7 +31,7 @@ func testApplyQueries(t *testing.T, ds kolide.Datastore) {
assert.Equal(t, comp.Name, q.Name)
assert.Equal(t, comp.Description, q.Description)
assert.Equal(t, comp.Query, q.Query)
assert.Equal(t, zwass.ID, q.AuthorID)
assert.Equal(t, &zwass.ID, q.AuthorID)
}
// Victor modifies a query (but also pushes the same version of the
@ -48,7 +48,7 @@ func testApplyQueries(t *testing.T, ds kolide.Datastore) {
assert.Equal(t, comp.Name, q.Name)
assert.Equal(t, comp.Description, q.Description)
assert.Equal(t, comp.Query, q.Query)
assert.Equal(t, groob.ID, q.AuthorID)
assert.Equal(t, &groob.ID, q.AuthorID)
}
// Zach adds a third query (but does not re-apply the others)
@ -67,9 +67,9 @@ func testApplyQueries(t *testing.T, ds kolide.Datastore) {
assert.Equal(t, comp.Description, q.Description)
assert.Equal(t, comp.Query, q.Query)
}
assert.Equal(t, groob.ID, queries[0].AuthorID)
assert.Equal(t, groob.ID, queries[1].AuthorID)
assert.Equal(t, zwass.ID, queries[2].AuthorID)
assert.Equal(t, &groob.ID, queries[0].AuthorID)
assert.Equal(t, &groob.ID, queries[1].AuthorID)
assert.Equal(t, &zwass.ID, queries[2].AuthorID)
}
func testDeleteQuery(t *testing.T, ds kolide.Datastore) {
@ -78,7 +78,7 @@ func testDeleteQuery(t *testing.T, ds kolide.Datastore) {
query := &kolide.Query{
Name: "foo",
Query: "bar",
AuthorID: user.ID,
AuthorID: &user.ID,
}
query, err := ds.NewQuery(query)
require.Nil(t, err)
@ -150,7 +150,7 @@ func testSaveQuery(t *testing.T, ds kolide.Datastore) {
query := &kolide.Query{
Name: "foo",
Query: "bar",
AuthorID: user.ID,
AuthorID: &user.ID,
}
query, err := ds.NewQuery(query)
require.Nil(t, err)
@ -177,7 +177,7 @@ func testListQuery(t *testing.T, ds kolide.Datastore) {
Name: fmt.Sprintf("name%02d", i),
Query: fmt.Sprintf("query%02d", i),
Saved: true,
AuthorID: user.ID,
AuthorID: &user.ID,
})
require.Nil(t, err)
}
@ -187,7 +187,7 @@ func testListQuery(t *testing.T, ds kolide.Datastore) {
Name: "unsaved",
Query: "select * from time",
Saved: false,
AuthorID: user.ID,
AuthorID: &user.ID,
})
require.Nil(t, err)
@ -326,7 +326,7 @@ func testDuplicateNewQuery(t *testing.T, ds kolide.Datastore) {
q1, err := ds.NewQuery(&kolide.Query{
Name: "foo",
Query: "select * from time;",
AuthorID: user.ID,
AuthorID: &user.ID,
})
require.Nil(t, err)
assert.NotZero(t, q1.ID)

View file

@ -71,7 +71,7 @@ func (d *Datastore) Query(id uint) (*kolide.Query, error) {
return nil, notFound("Query").WithID(id)
}
query.AuthorName = d.getUserNameByID(query.AuthorID)
query.AuthorName = d.getUserNameByID(*query.AuthorID)
if err := d.loadPacksForQueries([]*kolide.Query{query}); err != nil {
return nil, errors.Wrap(err, "error fetching query by id")
@ -95,7 +95,7 @@ func (d *Datastore) ListQueries(opt kolide.ListOptions) ([]*kolide.Query, error)
for _, k := range keys {
q := d.queries[uint(k)]
if q.Saved {
q.AuthorName = d.getUserNameByID(q.AuthorID)
q.AuthorName = d.getUserNameByID(*q.AuthorID)
queries = append(queries, q)
}
}

View file

@ -194,7 +194,7 @@ func (d *Datastore) DeleteQueries(ids []uint) (uint, error) {
// exists
func (d *Datastore) Query(id uint) (*kolide.Query, error) {
sql := `
SELECT q.*, COALESCE(NULLIF(u.name, ''), u.username) AS author_name
SELECT q.*, COALESCE(NULLIF(u.name, ''), u.username, '') AS author_name
FROM queries q
LEFT JOIN users u
ON q.author_id = u.id
@ -217,7 +217,7 @@ func (d *Datastore) Query(id uint) (*kolide.Query, error) {
// determined by passed in kolide.ListOptions
func (d *Datastore) ListQueries(opt kolide.ListOptions) ([]*kolide.Query, error) {
sql := `
SELECT q.*, COALESCE(NULLIF(u.name, ''), u.username) AS author_name
SELECT q.*, COALESCE(NULLIF(u.name, ''), u.username, '') AS author_name
FROM queries q
LEFT JOIN users u
ON q.author_id = u.id

View file

@ -4,9 +4,8 @@ import (
"context"
"strings"
"github.com/pkg/errors"
"github.com/ghodss/yaml"
"github.com/pkg/errors"
)
type QueryStore interface {
@ -75,7 +74,7 @@ type Query struct {
Description string `json:"description"`
Query string `json:"query"`
Saved bool `json:"saved"`
AuthorID uint `json:"author_id" db:"author_id"`
AuthorID *uint `json:"author_id" db:"author_id"`
// AuthorName is retrieved with a join to the users table in the MySQL
// backend (using AuthorID)
AuthorName string `json:"author_name" db:"author_name"`

View file

@ -25,6 +25,10 @@ func (svc service) NewDistributedQueryCampaignByNames(ctx context.Context, query
return svc.NewDistributedQueryCampaign(ctx, queryString, hostIDs, labelIDs)
}
func uintPtr(n uint) *uint {
return &n
}
func (svc service) NewDistributedQueryCampaign(ctx context.Context, queryString string, hosts []uint, labels []uint) (*kolide.DistributedQueryCampaign, error) {
vc, ok := viewer.FromContext(ctx)
if !ok {
@ -35,7 +39,7 @@ func (svc service) NewDistributedQueryCampaign(ctx context.Context, queryString
Name: fmt.Sprintf("distributed_%s_%d", vc.Username(), time.Now().Unix()),
Query: queryString,
Saved: false,
AuthorID: vc.UserID(),
AuthorID: uintPtr(vc.UserID()),
})
if err != nil {
return nil, errors.Wrap(err, "new query")

View file

@ -85,7 +85,7 @@ func (svc service) NewQuery(ctx context.Context, p kolide.QueryPayload) (*kolide
vc, ok := viewer.FromContext(ctx)
if ok {
query.AuthorID = vc.UserID()
query.AuthorID = uintPtr(vc.UserID())
query.AuthorName = vc.FullName()
}

View file

@ -1,119 +0,0 @@
package service
import (
"context"
"testing"
"github.com/kolide/fleet/server/config"
"github.com/kolide/fleet/server/contexts/viewer"
"github.com/kolide/fleet/server/datastore/inmem"
"github.com/kolide/fleet/server/kolide"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestListQueries(t *testing.T) {
ds, err := inmem.New(config.TestConfig())
assert.Nil(t, err)
svc, err := newTestService(ds, nil)
assert.Nil(t, err)
ctx := context.Background()
queries, err := svc.ListQueries(ctx, kolide.ListOptions{})
assert.Nil(t, err)
assert.Len(t, queries, 0)
name := "foo"
query := "select * from time"
_, err = svc.NewQuery(ctx, kolide.QueryPayload{
Name: &name,
Query: &query,
})
assert.Nil(t, err)
queries, err = svc.ListQueries(ctx, kolide.ListOptions{})
assert.Nil(t, err)
assert.Len(t, queries, 1)
}
func TestGetQuery(t *testing.T) {
ds, err := inmem.New(config.TestConfig())
assert.Nil(t, err)
svc, err := newTestService(ds, nil)
assert.Nil(t, err)
ctx := context.Background()
query := &kolide.Query{
Name: "foo",
Query: "select * from time;",
}
query, err = ds.NewQuery(query)
assert.Nil(t, err)
assert.NotZero(t, query.ID)
queryVerify, err := svc.GetQuery(ctx, query.ID)
assert.Nil(t, err)
assert.Equal(t, query.ID, queryVerify.ID)
}
func TestNewQuery(t *testing.T) {
ds, err := inmem.New(config.TestConfig())
assert.Nil(t, err)
createTestUsers(t, ds)
svc, err := newTestService(ds, nil)
assert.Nil(t, err)
user, err := ds.User("admin1")
require.Nil(t, err)
ctx := context.Background()
ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
name := "foo"
query := "select * from time;"
q, err := svc.NewQuery(ctx, kolide.QueryPayload{
Name: &name,
Query: &query,
})
assert.Nil(t, err)
assert.Equal(t, "Test Name admin1", q.AuthorName)
assert.Equal(t, []kolide.Pack{}, q.Packs)
queries, err := ds.ListQueries(kolide.ListOptions{})
assert.Nil(t, err)
if assert.Len(t, queries, 1) {
assert.Equal(t, "Test Name admin1", queries[0].AuthorName)
}
}
func TestModifyQuery(t *testing.T) {
ds, err := inmem.New(config.TestConfig())
assert.Nil(t, err)
svc, err := newTestService(ds, nil)
assert.Nil(t, err)
ctx := context.Background()
query := &kolide.Query{
Name: "foo",
Query: "select * from time;",
}
query, err = ds.NewQuery(query)
assert.Nil(t, err)
assert.NotZero(t, query.ID)
newName := "bar"
queryVerify, err := svc.ModifyQuery(ctx, query.ID, kolide.QueryPayload{
Name: &newName,
})
assert.Nil(t, err)
assert.Equal(t, query.ID, queryVerify.ID)
assert.Equal(t, "bar", queryVerify.Name)
}

View file

@ -111,7 +111,3 @@ func stringPtr(s string) *string {
func boolPtr(b bool) *bool {
return &b
}
func uintPtr(n uint) *uint {
return &n
}

View file

@ -12,7 +12,7 @@ func NewQuery(t *testing.T, ds kolide.Datastore, name, q string, authorID uint,
query, err := ds.NewQuery(&kolide.Query{
Name: name,
Query: q,
AuthorID: authorID,
AuthorID: &authorID,
Saved: saved,
})
require.Nil(t, err)