From e82edc68c862bf90f2d16b46d5a0ecc529116392 Mon Sep 17 00:00:00 2001 From: Yehor Butko Date: Fri, 28 Dec 2018 14:07:35 +0200 Subject: [PATCH] V3-976 Create db query for filtering team mebers (#940) * V3-976 Create db query for filtering team mebers * fixing linter * fixing linter * sql injection fixed * getOrder renamed, tests added --- pkg/satellite/projectmembers.go | 24 ++++- pkg/satellite/satellitedb/db.go | 2 +- pkg/satellite/satellitedb/projectmembers.go | 84 +++++++++++++-- .../satellitedb/projectmembers_test.go | 100 +++++++++++++++--- .../satelliteweb/satelliteql/project.go | 19 +++- pkg/satellite/service.go | 14 +-- 6 files changed, 204 insertions(+), 39 deletions(-) diff --git a/pkg/satellite/projectmembers.go b/pkg/satellite/projectmembers.go index f12b6dd13..5975cfd69 100644 --- a/pkg/satellite/projectmembers.go +++ b/pkg/satellite/projectmembers.go @@ -15,7 +15,7 @@ type ProjectMembers interface { // GetByMemberID is a method for querying project members from the database by memberID. GetByMemberID(ctx context.Context, memberID uuid.UUID) ([]ProjectMember, error) // GetByProjectID is a method for querying project members from the database by projectID, offset and limit. - GetByProjectID(ctx context.Context, projectID uuid.UUID, limit int, offset int64) ([]ProjectMember, error) + GetByProjectID(ctx context.Context, projectID uuid.UUID, pagination Pagination) ([]ProjectMember, error) // Insert is a method for inserting project member into the database. Insert(ctx context.Context, memberID, projectID uuid.UUID) (*ProjectMember, error) // Delete is a method for deleting project member by memberID and projectID from the database. @@ -24,8 +24,6 @@ type ProjectMembers interface { // ProjectMember is a database object that describes ProjectMember entity. type ProjectMember struct { - ID uuid.UUID - // FK on Users table. MemberID uuid.UUID // FK on Projects table. @@ -33,3 +31,23 @@ type ProjectMember struct { CreatedAt time.Time } + +// Pagination defines pagination, filtering and sorting rules +type Pagination struct { + Limit int + Offset int64 + Search string + Order ProjectMemberOrder +} + +// ProjectMemberOrder is used for querying project members in specified order +type ProjectMemberOrder int8 + +const ( + // Name indicates that we should order by first name + Name ProjectMemberOrder = 1 + // Email indicates that we should order by email + Email ProjectMemberOrder = 2 + // Created indicates that we should order by created date + Created ProjectMemberOrder = 3 +) diff --git a/pkg/satellite/satellitedb/db.go b/pkg/satellite/satellitedb/db.go index 37a9928f7..9a48e14b8 100644 --- a/pkg/satellite/satellitedb/db.go +++ b/pkg/satellite/satellitedb/db.go @@ -54,7 +54,7 @@ func (db *Database) Projects() satellite.Projects { // ProjectMembers is a getter for ProjectMembers repository func (db *Database) ProjectMembers() satellite.ProjectMembers { - return &projectMembers{db.methods} + return &projectMembers{db.methods, db.db} } // APIKeys is a getter for APIKeys repository diff --git a/pkg/satellite/satellitedb/projectmembers.go b/pkg/satellite/satellitedb/projectmembers.go index 889f84dfb..02f6a9b2d 100644 --- a/pkg/satellite/satellitedb/projectmembers.go +++ b/pkg/satellite/satellitedb/projectmembers.go @@ -16,12 +16,13 @@ import ( // ProjectMembers exposes methods to manage ProjectMembers table in database. type projectMembers struct { - db dbx.Methods + methods dbx.Methods + db *dbx.DB } // GetByMemberID is a method for querying project member from the database by memberID. func (pm *projectMembers) GetByMemberID(ctx context.Context, memberID uuid.UUID) ([]satellite.ProjectMember, error) { - projectMembersDbx, err := pm.db.All_ProjectMember_By_MemberId(ctx, dbx.ProjectMember_MemberId(memberID[:])) + projectMembersDbx, err := pm.methods.All_ProjectMember_By_MemberId(ctx, dbx.ProjectMember_MemberId(memberID[:])) if err != nil { return nil, err } @@ -30,23 +31,72 @@ func (pm *projectMembers) GetByMemberID(ctx context.Context, memberID uuid.UUID) } // GetByProjectID is a method for querying project members from the database by projectID, offset and limit. -func (pm *projectMembers) GetByProjectID(ctx context.Context, projectID uuid.UUID, limit int, offset int64) ([]satellite.ProjectMember, error) { - projectMembersDbx, err := pm.db.Limited_ProjectMember_By_ProjectId( - ctx, - dbx.ProjectMember_ProjectId(projectID[:]), - limit, - offset) +func (pm *projectMembers) GetByProjectID(ctx context.Context, projectID uuid.UUID, pagination satellite.Pagination) ([]satellite.ProjectMember, error) { + if pagination.Limit < 0 || pagination.Offset < 0 { + return nil, errs.New("invalid pagination argument") + } + + var projectMembers []satellite.ProjectMember + searchSubQuery := "%" + pagination.Search + "%" + //`+getOrder(pagination.Order)+` + + rebindedQuery := pm.db.Rebind(` + SELECT pm.* + FROM project_members pm + INNER JOIN users u ON pm.member_id = u.id + WHERE pm.project_id = ? + AND ( u.email LIKE ? OR + u.first_name LIKE ? OR + u.last_name LIKE ? ) + ORDER BY ` + sanitizedOrderColumnName(pagination.Order) + ` ASC + LIMIT ? OFFSET ? + `) + + rows, err := pm.db.Query(rebindedQuery, projectID[:], searchSubQuery, searchSubQuery, searchSubQuery, pagination.Limit, pagination.Offset) + + defer func() { + err = errs.Combine(err, rows.Close()) + }() if err != nil { return nil, err } - return projectMembersFromDbxSlice(projectMembersDbx) + for rows.Next() { + pm := satellite.ProjectMember{} + var memberIDBytes, projectIDBytes []uint8 + var memberID, projectID uuid.UUID + + scanErr := rows.Scan(&memberIDBytes, &projectIDBytes, &pm.CreatedAt) + if err != nil { + err = errs.Combine(err, scanErr) + continue + } + + memberID, convertErr := bytesToUUID(memberIDBytes) + if convertErr != nil { + err = errs.Combine(err, convertErr) + continue + } + + projectID, convertErr = bytesToUUID(projectIDBytes) + if convertErr != nil { + err = errs.Combine(err, convertErr) + continue + } + + pm.ProjectID = projectID + pm.MemberID = memberID + + projectMembers = append(projectMembers, pm) + } + + return projectMembers, err } // Insert is a method for inserting project member into the database. func (pm *projectMembers) Insert(ctx context.Context, memberID, projectID uuid.UUID) (*satellite.ProjectMember, error) { - createdProjectMember, err := pm.db.Create_ProjectMember(ctx, + createdProjectMember, err := pm.methods.Create_ProjectMember(ctx, dbx.ProjectMember_MemberId(memberID[:]), dbx.ProjectMember_ProjectId(projectID[:])) if err != nil { @@ -58,7 +108,7 @@ func (pm *projectMembers) Insert(ctx context.Context, memberID, projectID uuid.U // Delete is a method for deleting project member by memberID and projectID from the database. func (pm *projectMembers) Delete(ctx context.Context, memberID, projectID uuid.UUID) error { - _, err := pm.db.Delete_ProjectMember_By_MemberId_And_ProjectId( + _, err := pm.methods.Delete_ProjectMember_By_MemberId_And_ProjectId( ctx, dbx.ProjectMember_MemberId(memberID[:]), dbx.ProjectMember_ProjectId(projectID[:]), @@ -90,6 +140,18 @@ func projectMemberFromDBX(projectMember *dbx.ProjectMember) (*satellite.ProjectM }, nil } +// sanitizedOrderColumnName return valid order by column +func sanitizedOrderColumnName(pmo satellite.ProjectMemberOrder) string { + switch pmo { + case 2: + return "u.email" + case 3: + return "u.created_at" + default: + return "u.first_name" + } +} + // projectMembersFromDbxSlice is used for creating []ProjectMember entities from autogenerated []*dbx.ProjectMember struct func projectMembersFromDbxSlice(projectMembersDbx []*dbx.ProjectMember) ([]satellite.ProjectMember, error) { var projectMembers []satellite.ProjectMember diff --git a/pkg/satellite/satellitedb/projectmembers_test.go b/pkg/satellite/satellitedb/projectmembers_test.go index b56ba5eb4..ecc18ef23 100644 --- a/pkg/satellite/satellitedb/projectmembers_test.go +++ b/pkg/satellite/satellitedb/projectmembers_test.go @@ -69,30 +69,70 @@ func TestProjectMembersRepository(t *testing.T) { assert.Nil(t, err) assert.NoError(t, err) - projMember3, err := projectMembers.Insert(ctx, createdUsers[2].ID, createdProjects[1].ID) + projMember3, err := projectMembers.Insert(ctx, createdUsers[3].ID, createdProjects[0].ID) assert.NotNil(t, projMember3) assert.Nil(t, err) assert.NoError(t, err) + + projMember4, err := projectMembers.Insert(ctx, createdUsers[4].ID, createdProjects[0].ID) + assert.NotNil(t, projMember4) + assert.Nil(t, err) + assert.NoError(t, err) + + projMember5, err := projectMembers.Insert(ctx, createdUsers[5].ID, createdProjects[0].ID) + assert.NotNil(t, projMember5) + assert.Nil(t, err) + assert.NoError(t, err) + + projMember6, err := projectMembers.Insert(ctx, createdUsers[2].ID, createdProjects[1].ID) + assert.NotNil(t, projMember6) + assert.Nil(t, err) + assert.NoError(t, err) }) t.Run("Get paged", func(t *testing.T) { - members, err := projectMembers.GetByProjectID(ctx, createdProjects[0].ID, 1, 0) + // sql injection test. F.E '%SomeText%' = > ''%SomeText%' OR 'x' != '%'' will be true + members, err := projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: 6, Offset: 0, Search: "son%' OR 'x' != '", Order: 2}) + assert.Nil(t, err) + assert.NoError(t, err) + assert.Nil(t, members) + assert.Equal(t, 0, len(members)) + + members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: 3, Offset: 0, Search: "", Order: 1}) assert.Nil(t, err) assert.NoError(t, err) assert.NotNil(t, members) - assert.Equal(t, 1, len(members)) + assert.Equal(t, 3, len(members)) - members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, 2, 0) + members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: 2, Offset: 0, Search: "Liam", Order: 2}) assert.Nil(t, err) assert.NoError(t, err) assert.NotNil(t, members) assert.Equal(t, 2, len(members)) - members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, 1, 1) + members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: 2, Offset: 0, Search: "Liam", Order: 1}) assert.Nil(t, err) assert.NoError(t, err) assert.NotNil(t, members) - assert.Equal(t, 1, len(members)) + assert.Equal(t, 2, len(members)) + + members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: 6, Offset: 0, Search: "son", Order: 123}) + assert.Nil(t, err) + assert.NoError(t, err) + assert.NotNil(t, members) + assert.Equal(t, 5, len(members)) + + members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: 6, Offset: 3, Search: "son", Order: 2}) + assert.Nil(t, err) + assert.NoError(t, err) + assert.NotNil(t, members) + assert.Equal(t, 2, len(members)) + + members, err = projectMembers.GetByProjectID(ctx, createdProjects[0].ID, satellite.Pagination{Limit: -123, Offset: -14, Search: "son", Order: 2}) + assert.NotNil(t, err) + assert.Error(t, err) + assert.Nil(t, members) + assert.Equal(t, 0, len(members)) }) t.Run("Get member by memberID success", func(t *testing.T) { @@ -116,20 +156,35 @@ func TestProjectMembersRepository(t *testing.T) { func prepareUsersAndProjects(ctx context.Context, t *testing.T, users satellite.Users, projects satellite.Projects) ([]*satellite.User, []*satellite.Project) { usersList := []*satellite.User{{ - Email: "email1@ukr.net", + Email: "2email2@ukr.net", PasswordHash: []byte("some_readable_hash"), - LastName: "LastName", - FirstName: "FirstName", + LastName: "Liam", + FirstName: "Jameson", }, { - Email: "email2@ukr.net", + Email: "1email1@ukr.net", PasswordHash: []byte("some_readable_hash"), - LastName: "LastName", - FirstName: "FirstName", + LastName: "William", + FirstName: "Noahson", }, { Email: "email3@ukr.net", PasswordHash: []byte("some_readable_hash"), - LastName: "LastName", - FirstName: "FirstName", + LastName: "Mason", + FirstName: "Elijahson", + }, { + Email: "email4@ukr.net", + PasswordHash: []byte("some_readable_hash"), + LastName: "Oliver", + FirstName: "Jacobson", + }, { + Email: "email5@ukr.net", + PasswordHash: []byte("some_readable_hash"), + LastName: "Lucas", + FirstName: "Michaelson", + }, { + Email: "email6@ukr.net", + PasswordHash: []byte("some_readable_hash"), + LastName: "Alexander", + FirstName: "Ethanson", }, } @@ -163,3 +218,20 @@ func prepareUsersAndProjects(ctx context.Context, t *testing.T, users satellite. return usersList, projectList } + +func TestSanitizedOrderColumnName(t *testing.T) { + testCases := [...]struct { + orderNumber int8 + orderColumn string + }{ + 0: {0, "u.first_name"}, + 1: {1, "u.first_name"}, + 2: {2, "u.email"}, + 3: {3, "u.created_at"}, + 4: {4, "u.first_name"}, + } + + for _, tc := range testCases { + assert.Equal(t, tc.orderColumn, sanitizedOrderColumnName(satellite.ProjectMemberOrder(tc.orderNumber))) + } +} diff --git a/pkg/satellite/satelliteweb/satelliteql/project.go b/pkg/satellite/satelliteweb/satelliteql/project.go index df278bead..9caadd935 100644 --- a/pkg/satellite/satelliteweb/satelliteql/project.go +++ b/pkg/satellite/satelliteweb/satelliteql/project.go @@ -22,6 +22,8 @@ const ( limit = "limit" offset = "offset" + search = "search" + order = "order" ) // graphqlProject creates *graphql.Object type representation of satellite.ProjectInfo @@ -53,14 +55,29 @@ func graphqlProject(service *satellite.Service, types Types) *graphql.Object { limit: &graphql.ArgumentConfig{ Type: graphql.NewNonNull(graphql.Int), }, + search: &graphql.ArgumentConfig{ + Type: graphql.String, + }, + order: &graphql.ArgumentConfig{ + Type: graphql.Int, + }, }, Resolve: func(p graphql.ResolveParams) (interface{}, error) { project, _ := p.Source.(*satellite.Project) offs, _ := p.Args[offset].(int64) lim, _ := p.Args[limit].(int) + search, _ := p.Args[search].(string) + order, _ := p.Args[order].(int8) - members, err := service.GetProjectMembers(p.Context, project.ID, lim, offs) + pagination := satellite.Pagination{ + Limit: lim, + Offset: offs, + Search: search, + Order: satellite.ProjectMemberOrder(order), + } + + members, err := service.GetProjectMembers(p.Context, project.ID, pagination) if err != nil { return nil, err } diff --git a/pkg/satellite/service.go b/pkg/satellite/service.go index 12c1c737d..64432f991 100644 --- a/pkg/satellite/service.go +++ b/pkg/satellite/service.go @@ -395,22 +395,18 @@ func (s *Service) DeleteProjectMembers(ctx context.Context, projectID uuid.UUID, } // GetProjectMembers returns ProjectMembers for given Project -func (s *Service) GetProjectMembers(ctx context.Context, projectID uuid.UUID, limit int, offset int64) (pm []ProjectMember, err error) { +func (s *Service) GetProjectMembers(ctx context.Context, projectID uuid.UUID, pagination Pagination) (pm []ProjectMember, err error) { defer mon.Task()(&ctx)(&err) _, err = GetAuth(ctx) if err != nil { return nil, err } - if limit < 0 || offset < 0 { - return nil, errs.New("invalid pagination argument") + if pagination.Limit > maxLimit { + pagination.Limit = maxLimit } - if limit > maxLimit { - limit = maxLimit - } - - return s.store.ProjectMembers().GetByProjectID(ctx, projectID, limit, offset) + return s.store.ProjectMembers().GetByProjectID(ctx, projectID, pagination) } // CreateAPIKey creates new api key @@ -609,5 +605,5 @@ func (s *Service) isProjectMember(ctx context.Context, userID uuid.UUID, project } } - return isProjectMember{}, ErrNoMembership.New("user % is not a member of project %s", userID, project.ID) + return isProjectMember{}, ErrNoMembership.New("user %s is not a member of project %s", userID, project.ID) }