Merge pull request #1876 from molecula/fb1166

fix bug where drop table wasn't being authorized
This commit is contained in:
reese 2022-01-19 14:26:04 -06:00 committed by GitHub
commit 49972939ec
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 90 additions and 5 deletions

View file

@ -120,7 +120,7 @@ func (p *GroupPermissions) IsAdmin(groups []authn.Group) bool {
func (p *GroupPermissions) GetAuthorizedIndexList(groups []authn.Group, desiredPermission Permission) (indexList []string) {
// if user is admin, find all indexes in permissions file and return them
if admin := p.IsAdmin(groups); admin {
if p.IsAdmin(groups) {
for groupId := range p.Permissions {
for index := range p.Permissions[groupId] {
indexList = append(indexList, index)
@ -132,7 +132,7 @@ func (p *GroupPermissions) GetAuthorizedIndexList(groups []authn.Group, desiredP
for _, group := range groups {
if _, ok := p.Permissions[group.GroupID]; ok {
for index, permission := range p.Permissions[group.GroupID] {
if permission >= desiredPermission {
if permission.Satisfies(desiredPermission) {
indexList = append(indexList, index)
}
}

View file

@ -30,6 +30,7 @@ import (
"google.golang.org/grpc/peer"
"google.golang.org/grpc/reflection"
"google.golang.org/grpc/status"
"vitess.io/vitess/go/vt/sqlparser"
)
// GRPCHandler contains methods which handle the various gRPC requests.
@ -174,7 +175,13 @@ func (h *GRPCHandler) QuerySQL(req *pb.QuerySQLRequest, stream pb.Pilosa_QuerySQ
return errors.Wrap(err, "parsing SQL")
}
allowed := h.perms.GetAuthorizedIndexList(uinfo.(*authn.UserInfo).Groups, authz.Read)
perm := authz.Read
switch parsed.Statement.(type) {
case *sqlparser.DDL: // currently only used for DropTable
perm = authz.Admin
}
allowed := h.perms.GetAuthorizedIndexList(uinfo.(*authn.UserInfo).Groups, perm)
if !h.perms.IsAdmin(uinfo.(*authn.UserInfo).Groups) {
if !isAllowed(parsed.Tables, allowed) {
return status.Error(codes.PermissionDenied, "insufficient permissions to access requested tables")
@ -219,9 +226,29 @@ func (h *GRPCHandler) QuerySQL(req *pb.QuerySQLRequest, stream pb.Pilosa_QuerySQ
func (h *GRPCHandler) QuerySQLUnary(ctx context.Context, req *pb.QuerySQLRequest) (*pb.TableResponse, error) {
start := time.Now()
uinfo := ctx.Value("userinfo")
if uinfo != nil && !h.perms.IsAdmin(uinfo.(*authn.UserInfo).Groups) {
ctx = context.WithValue(ctx, "indices", h.perms.GetAuthorizedIndexList(uinfo.(*authn.UserInfo).Groups, authz.Read))
if uinfo != nil {
// authz
m := sql.NewMapper()
parsed, err := m.MapSQL(req.Sql)
if err != nil {
return nil, errors.Wrap(err, "parsing SQL")
}
perm := authz.Read
switch parsed.Statement.(type) {
case *sqlparser.DDL: // currently only used for DropTable
perm = authz.Admin
}
allowed := h.perms.GetAuthorizedIndexList(uinfo.(*authn.UserInfo).Groups, perm)
if !h.perms.IsAdmin(uinfo.(*authn.UserInfo).Groups) {
if !isAllowed(parsed.Tables, allowed) {
return nil, status.Error(codes.PermissionDenied, "insufficient permissions to access requested tables")
}
ctx = context.WithValue(ctx, "indices", allowed)
}
}
results, err := h.execSQL(ctx, req.Sql)
if err != nil {
return nil, err

View file

@ -843,6 +843,8 @@ func TestQuerySQL(t *testing.T) {
{"Table", "string"},
},
rows: []row{
{[]columnResponse{"another_one"}},
{[]columnResponse{"deletable_index"}},
{[]columnResponse{"delete_me"}},
{[]columnResponse{"grouper"}},
{[]columnResponse{"joiner"}},
@ -881,6 +883,9 @@ func TestQuerySQL(t *testing.T) {
{"Table", "string"},
},
rows: []row{
{[]columnResponse{"another_one"}},
{[]columnResponse{"deletable_index"}},
{[]columnResponse{"grouper"}},
{[]columnResponse{"joiner"}},
},
@ -1162,6 +1167,7 @@ admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"`
t.Fatal(err)
}
})
t.Run("test-show-tables-unary-admin", func(t *testing.T) {
response, err := gh.QuerySQLUnary(adminCtx, &pb.QuerySQLRequest{
Sql: "show tables",
@ -1190,6 +1196,55 @@ admin: "ac97c9e2-346b-42a2-b6da-18bcb61a32fe"`
t.Fatal(err)
}
})
t.Run("test-drop-table-unary-read", func(t *testing.T) {
_, err := gh.QuerySQLUnary(readCtx, &pb.QuerySQLRequest{
Sql: "drop table deletable_index",
})
if err == nil {
t.Fatal("expected error but got nil")
}
})
t.Run("test-drop-table-unary-write", func(t *testing.T) {
_, err := gh.QuerySQLUnary(writeCtx, &pb.QuerySQLRequest{
Sql: "drop table deletable_index",
})
if err == nil {
t.Fatal("expected error but got nil")
}
})
t.Run("test-drop-table-unary-admin", func(t *testing.T) {
_, err := gh.QuerySQLUnary(adminCtx, &pb.QuerySQLRequest{
Sql: "drop table deletable_index",
})
if err != nil {
t.Fatalf("expected nil error but got %v", err)
}
})
t.Run("test-drop-table-stream-read", func(t *testing.T) {
mock := &mockPilosa_QuerySQLServer{ctx: readCtx}
err := gh.QuerySQL(&pb.QuerySQLRequest{Sql: "drop table another_one"}, mock)
if err == nil {
t.Fatal("expected error but got nil")
}
})
t.Run("test-drop-table-stream-write", func(t *testing.T) {
mock := &mockPilosa_QuerySQLServer{ctx: writeCtx}
err := gh.QuerySQL(&pb.QuerySQLRequest{Sql: "drop table another_one"}, mock)
if err == nil {
t.Fatal("expected error but got nil")
}
})
t.Run("test-drop-table-stream-admin", func(t *testing.T) {
mock := &mockPilosa_QuerySQLServer{ctx: adminCtx}
err := gh.QuerySQL(&pb.QuerySQLRequest{Sql: "drop table another_one"}, mock)
if err != nil {
t.Fatalf("expected nil error but got %v", err)
}
})
}
func TestCRUDIndexes(t *testing.T) {
@ -1506,6 +1561,9 @@ func setUpTestQuerySQLUnary(ctx context.Context, t *testing.T) (gh *server.GRPCH
// delete_me
m.MustCreateIndex(t, "delete_me", pilosa.IndexOptions{TrackExistence: true})
m.MustCreateIndex(t, "another_one", pilosa.IndexOptions{TrackExistence: true})
m.MustCreateIndex(t, "deletable_index", pilosa.IndexOptions{TrackExistence: true})
return gh, func() {
if err := m.API.DeleteIndex(ctx, joiner.Name()); err != nil {
panic(err)