empayre/fleet
14
0
Fork 0
mirror of https://github.com/empayre/fleet.git synced 2024-11-06 08:55:24 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

apply queries spec endpoint missing authorization check (#4068)

Browse Source 
* do authorization check when updating existing query
This commit is contained in:
Michal Nicpon 2022-02-08 09:47:48 -07:00 committed by GitHub
parent 73d4794c55
commit 578a9780f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 25 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changes/issue-3953-apply-queries-missing-authorization Normal file
View File

@ -0,0 +1 @@
* Check that user is authorized to update query in update query specs endpoint

3
cmd/fleetctl/apply_test.go
View File

@ -423,6 +423,9 @@ func TestApplyQueries(t *testing.T) {
_, ds := runServerWithMockedDS(t)
var appliedQueries []*fleet.Query
ds.QueryByNameFunc = func(ctx context.Context, name string, opts ...fleet.OptionalArg) (*fleet.Query, error) {
return nil, sql.ErrNoRows
}
ds.ApplyQueriesFunc = func(ctx context.Context, authorID uint, queries []*fleet.Query) error {
appliedQueries = queries
return nil

23
server/service/queries.go
View File

@ -2,6 +2,8 @@ package service
import (
"context"
"database/sql"
"errors"
"fmt"
"github.com/fleetdm/fleet/v4/server/authz"
@ -459,15 +461,11 @@ func applyQuerySpecsEndpoint(ctx context.Context, request interface{}, svc fleet
}
func (svc *Service) ApplyQuerySpecs(ctx context.Context, specs []*fleet.QuerySpec) error {
// check that the user can create queries
if err := svc.authz.Authorize(ctx, &fleet.Query{}, fleet.ActionWrite); err != nil {
return err
}
vc, ok := viewer.FromContext(ctx)
if !ok {
return ctxerr.New(ctx, "user must be authenticated to apply queries")
}
queries := []*fleet.Query{}
for _, spec := range specs {
queries = append(queries, queryFromSpec(spec))
@ -479,6 +477,21 @@ func (svc *Service) ApplyQuerySpecs(ctx context.Context, specs []*fleet.QuerySpe
message: fmt.Sprintf("query payload verification: %s", err),
})
}
// check that the user can update the query if it already exists
query, err := svc.ds.QueryByName(ctx, query.Name)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
return err
} else if err == nil {
if err := svc.authz.Authorize(ctx, query, fleet.ActionWrite); err != nil {
return err
}
}
}
vc, ok := viewer.FromContext(ctx)
if !ok {
return ctxerr.New(ctx, "user must be authenticated to apply queries")
}
err := svc.ds.ApplyQueries(ctx, vc.UserID(), queries)

9
server/service/queries_test.go
View File

@ -211,15 +211,12 @@ func TestQueryAuth(t *testing.T) {
_, err = svc.ListQueries(ctx, fleet.ListOptions{})
checkAuthErr(t, tt.shouldFailRead, err)
// TODO(mna): the authorization seems wrong here - ApplyQuerySpecs never loads the queries it
// receives, so it just validates that the user has Write access to Queries in general, not
// that it can update those provided in the specs in particular. Here, a team maintainer can
// end up updating a query they did not author. See https://github.com/fleetdm/fleet/issues/3953
//err = svc.ApplyQuerySpecs(ctx, []*fleet.QuerySpec{{Name: queryName[tt.qid], Query: "SELECT 1"}})
//checkAuthErr(t, tt.shouldFailWrite, err)
err = svc.ApplyQuerySpecs(ctx, []*fleet.QuerySpec{{Name: queryName[tt.qid], Query: "SELECT 1"}})
checkAuthErr(t, tt.shouldFailWrite, err)
_, err = svc.GetQuerySpecs(ctx)
checkAuthErr(t, tt.shouldFailRead, err)
_, err = svc.GetQuerySpec(ctx, queryName[tt.qid])
checkAuthErr(t, tt.shouldFailRead, err)
})