From 927a881c2fe57a2969065c92534cc4f2cdfd3c21 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Mon, 9 Jan 2023 15:57:33 -0500 Subject: [PATCH 01/12] Add validation to member, project, and topic models --- pkg/tenant/db/errors.go | 10 +++++++--- pkg/tenant/db/members.go | 24 ++++++++++++++++++++++++ pkg/tenant/db/members_test.go | 6 +++++- pkg/tenant/db/projects.go | 25 +++++++++++++++++++++++++ pkg/tenant/db/projects_test.go | 3 +-- pkg/tenant/db/topics.go | 24 ++++++++++++++++++++++++ pkg/tenant/db/topics_test.go | 3 +-- 7 files changed, 87 insertions(+), 8 deletions(-) diff --git a/pkg/tenant/db/errors.go b/pkg/tenant/db/errors.go index dd1095229..7779d1875 100644 --- a/pkg/tenant/db/errors.go +++ b/pkg/tenant/db/errors.go @@ -3,7 +3,11 @@ package db import "errors" var ( - ErrNotConnected = errors.New("not connected to trtl database") - ErrNotFound = errors.New("object not found for the specified key") - ErrMissingID = errors.New("object requires id for serialization") + ErrNotConnected = errors.New("not connected to trtl database") + ErrNotFound = errors.New("object not found for the specified key") + ErrMissingID = errors.New("object requires id for serialization") + ErrMissingTenantID = errors.New("object requires tenant id for serialization") + ErrMissingProjectID = errors.New("object requires project id for serialization") + ErrNumberFirstCharacter = errors.New("name cannot begin with a number") + ErrSpecialCharacters = errors.New("name cannot include special characters") ) diff --git a/pkg/tenant/db/members.go b/pkg/tenant/db/members.go index 58971eb2a..b0730aa5c 100644 --- a/pkg/tenant/db/members.go +++ b/pkg/tenant/db/members.go @@ -3,6 +3,7 @@ package db import ( "context" "fmt" + "strings" "time" "github.com/oklog/ulid/v2" @@ -52,11 +53,34 @@ func (m *Member) UnmarshalValue(data []byte) error { return msgpack.Unmarshal(data, m) } +func (m *Member) Validate() error { + if m.TenantID.Compare(ulid.ULID{}) == 0 { + return ErrMissingTenantID + } + + memberName := m.Name + + if strings.ContainsAny(string(memberName[0]), "0123456789") { + return ErrNumberFirstCharacter + } + + if strings.ContainsAny(memberName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { + return ErrSpecialCharacters + } + + return nil +} + // CreateMember adds a new Member to the database. // Note: If a memberID is not passed in by the User, a new member id will be generated. func CreateMember(ctx context.Context, member *Member) (err error) { // TODO: Use crypto rand and monotonic entropy with ulid.New + // Validate project data. + if err = member.Validate(); err != nil { + return err + } + if member.ID.Compare(ulid.ULID{}) == 0 { member.ID = ulid.Make() } diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 180a8a394..5840d3d61 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -46,7 +46,11 @@ func TestMemberModel(t *testing.T) { func (s *dbTestSuite) TestCreateMember() { require := s.Require() ctx := context.Background() - member := &db.Member{Name: "member-example"} + member := &db.Member{ + TenantID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), + Name: "member001", + Role: "role-example", + } // Call OnPut method from mock trtl database s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { diff --git a/pkg/tenant/db/projects.go b/pkg/tenant/db/projects.go index aa24f49c3..deea64ca7 100644 --- a/pkg/tenant/db/projects.go +++ b/pkg/tenant/db/projects.go @@ -1,6 +1,7 @@ package db import ( + "strings" "time" "github.com/oklog/ulid/v2" @@ -50,9 +51,33 @@ func (p *Project) UnmarshalValue(data []byte) error { return msgpack.Unmarshal(data, p) } +func (p *Project) Validate() error { + if p.TenantID.Compare(ulid.ULID{}) == 0 { + return ErrMissingTenantID + } + + projectName := p.Name + + if strings.ContainsAny(string(projectName[0]), "0123456789") { + return ErrNumberFirstCharacter + } + + if strings.ContainsAny(projectName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { + return ErrSpecialCharacters + } + + return nil +} + // CreateProject adds a new project to the database. // Note: If a project id is not passed in by the User, a new project id will be generated. func CreateProject(ctx context.Context, project *Project) (err error) { + // Validate project data. + if err = project.Validate(); err != nil { + return err + } + + // TODO: Use crypto rand and monotonic entropy with ulid.New if project.ID.Compare(ulid.ULID{}) == 0 { project.ID = ulid.Make() } diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index 8effbfa68..70c586e6c 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -47,8 +47,7 @@ func (s *dbTestSuite) TestCreateProject() { ctx := context.Background() project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), - ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", } s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { diff --git a/pkg/tenant/db/topics.go b/pkg/tenant/db/topics.go index f96f6ab7a..87f2071f9 100644 --- a/pkg/tenant/db/topics.go +++ b/pkg/tenant/db/topics.go @@ -2,6 +2,7 @@ package db import ( "context" + "strings" "time" "github.com/oklog/ulid/v2" @@ -51,8 +52,31 @@ func (t *Topic) UnmarshalValue(data []byte) error { return msgpack.Unmarshal(data, t) } +func (t *Topic) Validate() error { + if t.ProjectID.Compare(ulid.ULID{}) == 0 { + return ErrMissingID + } + + name := t.Name + + if strings.ContainsAny(string(name[0]), "0123456789") { + return ErrNumberFirstCharacter + } + + if strings.ContainsAny(name, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { + return ErrSpecialCharacters + } + return nil +} + // CreateTopic adds a new topic to the database. func CreateTopic(ctx context.Context, topic *Topic) (err error) { + // Validate topic data. + if err = topic.Validate(); err != nil { + return err + } + + // TODO: Use crypto rand and monotonic entropy with ulid.New if topic.ID.Compare(ulid.ULID{}) == 0 { topic.ID = ulid.Make() } diff --git a/pkg/tenant/db/topics_test.go b/pkg/tenant/db/topics_test.go index e42f5180c..5ab3c62f5 100644 --- a/pkg/tenant/db/topics_test.go +++ b/pkg/tenant/db/topics_test.go @@ -45,8 +45,7 @@ func (s *dbTestSuite) TestCreateTopic() { ctx := context.Background() topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), - ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", } s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { From cadadf9e10685b1c5b1a956e9eeef5253c4d8c51 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Mon, 9 Jan 2023 16:28:19 -0500 Subject: [PATCH 02/12] Test validation and add validate method to update functions --- pkg/tenant/db/members_test.go | 5 ++++- pkg/tenant/db/projects_test.go | 5 ++++- pkg/tenant/db/topics.go | 6 ++++++ pkg/tenant/db/topics_test.go | 7 +++++-- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 5840d3d61..641205d21 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -52,6 +52,9 @@ func (s *dbTestSuite) TestCreateMember() { Role: "role-example", } + err := member.Validate() + require.NoError(err, "could not validate member data") + // Call OnPut method from mock trtl database s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.MembersNamespace { @@ -63,7 +66,7 @@ func (s *dbTestSuite) TestCreateMember() { }, nil } - err := db.CreateMember(ctx, member) + err = db.CreateMember(ctx, member) require.NoError(err, "could not create member") require.NotEqual("", member.ID, "expected non-zero ulid to be populated") diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index 70c586e6c..f95082da2 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -50,6 +50,9 @@ func (s *dbTestSuite) TestCreateProject() { Name: "project001", } + err := project.Validate() + require.NoError(err, "could not validate project data") + s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.ProjectNamespace { return nil, status.Error(codes.FailedPrecondition, "bad Put request") @@ -60,7 +63,7 @@ func (s *dbTestSuite) TestCreateProject() { }, nil } - err := db.CreateProject(ctx, project) + err = db.CreateProject(ctx, project) require.NoError(err, "could not create project") // Verify that below fields have been populated. diff --git a/pkg/tenant/db/topics.go b/pkg/tenant/db/topics.go index 87f2071f9..9df21ceb6 100644 --- a/pkg/tenant/db/topics.go +++ b/pkg/tenant/db/topics.go @@ -132,6 +132,12 @@ func ListTopics(ctx context.Context, projectID ulid.ULID) (topics []*Topic, err // UpdateTopic updates the record of a topic by a given ID. func UpdateTopic(ctx context.Context, topic *Topic) (err error) { + // Validate topic data. + if err = topic.Validate(); err != nil { + return err + } + + // TODO: Use crypto rand and monotonic entropy with ulid.New if topic.ID.Compare(ulid.ULID{}) == 0 { return ErrMissingID } diff --git a/pkg/tenant/db/topics_test.go b/pkg/tenant/db/topics_test.go index 5ab3c62f5..2b319d269 100644 --- a/pkg/tenant/db/topics_test.go +++ b/pkg/tenant/db/topics_test.go @@ -48,6 +48,9 @@ func (s *dbTestSuite) TestCreateTopic() { Name: "topic001", } + err := topic.Validate() + require.NoError(err, "could not validate topic data") + s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.TopicNamespace { return nil, status.Error(codes.FailedPrecondition, "bad Put request") @@ -58,7 +61,7 @@ func (s *dbTestSuite) TestCreateTopic() { }, nil } - err := db.CreateTopic(ctx, topic) + err = db.CreateTopic(ctx, topic) require.NoError(err, "could not create topic") // Verify that below fields have been populated. @@ -170,7 +173,7 @@ func (s *dbTestSuite) TestUpdateTopic() { topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", Created: time.Unix(1672161102, 0), Modified: time.Unix(1672161102, 0), } From b68e1e8d3367f838a57b362a64094c2c37cf22d2 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Tue, 10 Jan 2023 14:04:12 -0500 Subject: [PATCH 03/12] Add validation test to models and update NotFound path tests --- pkg/tenant/db/members.go | 5 +++++ pkg/tenant/db/members_test.go | 20 +++++++++++++------- pkg/tenant/db/projects.go | 6 ++++++ pkg/tenant/db/projects_test.go | 20 +++++++++++++------- pkg/tenant/db/topics_test.go | 16 +++++++++++----- 5 files changed, 48 insertions(+), 19 deletions(-) diff --git a/pkg/tenant/db/members.go b/pkg/tenant/db/members.go index b0730aa5c..7f4522458 100644 --- a/pkg/tenant/db/members.go +++ b/pkg/tenant/db/members.go @@ -136,6 +136,11 @@ func ListMembers(ctx context.Context, tenantID ulid.ULID) (members []*Member, er // UpdateMember updates the record of a member by its id. func UpdateMember(ctx context.Context, member *Member) (err error) { + // Validate member data. + if err = member.Validate(); err != nil { + return err + } + // TODO: Use crypto rand and monotonic entropy with ulid.New // Check if memberID exists and return a missing diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 641205d21..b5f9e436e 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -19,12 +19,15 @@ func TestMemberModel(t *testing.T) { member := &db.Member{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "member-example", + Name: "member001", Role: "role-example", Created: time.Unix(1670424445, 0).In(time.UTC), Modified: time.Unix(1670424445, 0).In(time.UTC), } + err := member.Validate() + require.NoError(t, err, "could not validate member data") + key, err := member.Key() require.NoError(t, err, "could not marshal the key") require.Equal(t, member.TenantID[:], key[0:16], "unexpected marshaling of the tenant id half of the key") @@ -80,7 +83,7 @@ func (s *dbTestSuite) TestRetrieveMember() { member := &db.Member{ TenantID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "member-example", + Name: "member001", Role: "role-example", } @@ -113,7 +116,7 @@ func (s *dbTestSuite) TestRetrieveMember() { require.NoError(err, "could not retrieve member") require.Equal(ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), member.ID, "expected member id to match") - require.Equal("member-example", member.Name, "expected member name to match") + require.Equal("member001", member.Name, "expected member name to match") require.Equal("role-example", member.Role, "expected member role to match") // TODO: Use crypto rand and monotonic entropy with ulid.New @@ -128,7 +131,7 @@ func (s *dbTestSuite) TestListMembers() { member := &db.Member{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "member-example", + Name: "member001", Role: "role-example", Created: time.Unix(1670424445, 0), Modified: time.Unix(1670424445, 0), @@ -171,12 +174,15 @@ func (s *dbTestSuite) TestUpdateMember() { member := &db.Member{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "member-example", + Name: "member001", Role: "role-example", Created: time.Unix(1670424445, 0), Modified: time.Unix(1670424467, 0), } + err := member.Validate() + require.NoError(err, "could not validate member data") + // Call OnPut method from mock trtl database s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.MembersNamespace { @@ -196,7 +202,7 @@ func (s *dbTestSuite) TestUpdateMember() { }, nil } - err := db.UpdateMember(ctx, member) + err = db.UpdateMember(ctx, member) require.NoError(err, "could not update member") require.Equal(ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), member.ID, "member ID should not have changed") @@ -205,7 +211,7 @@ func (s *dbTestSuite) TestUpdateMember() { // Test NotFound path // TODO: Use crypto rand and monotonic entropy with ulid.New - err = db.UpdateMember(ctx, &db.Member{ID: ulid.Make()}) + err = db.UpdateMember(ctx, &db.Member{TenantID: ulid.Make(), ID: ulid.Make(), Name: "member002"}) require.ErrorIs(err, db.ErrNotFound) } diff --git a/pkg/tenant/db/projects.go b/pkg/tenant/db/projects.go index deea64ca7..c118c6401 100644 --- a/pkg/tenant/db/projects.go +++ b/pkg/tenant/db/projects.go @@ -133,6 +133,12 @@ func ListProjects(ctx context.Context, tenantID ulid.ULID) (projects []*Project, // UpdateProject updates the record of a project by its id. func UpdateProject(ctx context.Context, project *Project) (err error) { + // Validate project data. + if err = project.Validate(); err != nil { + return err + } + + // TODO: Use crypto rand and monotonic entropy with ulid.New if project.ID.Compare(ulid.ULID{}) == 0 { return ErrMissingID } diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index f95082da2..be6dfc715 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -19,11 +19,14 @@ func TestProjectModel(t *testing.T) { project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", Created: time.Unix(1670424445, 0).In(time.UTC), Modified: time.Unix(1670424445, 0).In(time.UTC), } + err := project.Validate() + require.NoError(t, err, "could not validate project data") + key, err := project.Key() require.NoError(t, err, "could not marshal the project") require.Equal(t, project.TenantID[:], key[0:16], "unexpected marshaling of the tenant id half of the key") @@ -78,7 +81,7 @@ func (s *dbTestSuite) TestRetrieveProject() { project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", Created: time.Unix(1670424445, 0), Modified: time.Unix(1670424445, 0), } @@ -115,7 +118,7 @@ func (s *dbTestSuite) TestRetrieveProject() { // Verify the fields below have been populated. require.Equal(ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), project.ID, "expected project id to match") - require.Equal("project-example", project.Name, "expected project name to match") + require.Equal("project001", project.Name, "expected project name to match") require.Equal(time.Unix(1670424445, 0), project.Created, "expected created timestamp to not have changed") require.True(time.Unix(1670424444, 0).Before(project.Modified), "expected modified timestamp to be updated") @@ -132,7 +135,7 @@ func (s *dbTestSuite) TestListProjects() { project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", Created: time.Unix(1670424445, 0).In(time.UTC), Modified: time.Unix(1670424445, 0).In(time.UTC), } @@ -174,11 +177,14 @@ func (s *dbTestSuite) TestUpdateProject() { project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", Created: time.Unix(1668660681, 0), Modified: time.Unix(1668660681, 0), } + err := project.Validate() + require.NoError(err, "could not validate project data") + s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.ProjectNamespace { return nil, status.Error(codes.FailedPrecondition, "bad Put request") @@ -192,7 +198,7 @@ func (s *dbTestSuite) TestUpdateProject() { }, nil } - err := db.UpdateProject(ctx, project) + err = db.UpdateProject(ctx, project) require.NoError(err, "could not update project") // The fields below should have been populated @@ -202,7 +208,7 @@ func (s *dbTestSuite) TestUpdateProject() { // Test NotFound path // TODO: Use crypto rand and monotonic entropy with ulid.New - err = db.UpdateProject(ctx, &db.Project{ID: ulid.Make()}) + err = db.UpdateProject(ctx, &db.Project{TenantID: ulid.Make(), ID: ulid.Make(), Name: "project002"}) require.ErrorIs(err, db.ErrNotFound) } diff --git a/pkg/tenant/db/topics_test.go b/pkg/tenant/db/topics_test.go index 2b319d269..0ce4c3e3e 100644 --- a/pkg/tenant/db/topics_test.go +++ b/pkg/tenant/db/topics_test.go @@ -19,11 +19,14 @@ func TestTopicModel(t *testing.T) { topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", Created: time.Unix(1672161102, 0).In(time.UTC), Modified: time.Unix(1672161102, 0).In(time.UTC), } + err := topic.Validate() + require.NoError(t, err, "could not validate topic data") + key, err := topic.Key() require.NoError(t, err, "could not marshal the topic") require.Equal(t, topic.ProjectID[:], key[0:16], "unexpected marshaling of the project id half of the key") @@ -78,7 +81,7 @@ func (s *dbTestSuite) TestRetrieveTopic() { topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", Created: time.Unix(1672161102, 0), Modified: time.Unix(1672161102, 0), } @@ -116,7 +119,7 @@ func (s *dbTestSuite) TestRetrieveTopic() { // Verify the fields below have been populated. require.Equal(ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), topic.ProjectID, "expected project id to match") require.Equal(ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), topic.ID, "expected topic id to match") - require.Equal("topic-example", topic.Name, "expected topic name to match") + require.Equal("topic001", topic.Name, "expected topic name to match") require.Equal(time.Unix(1672161102, 0), topic.Created, "expected created timestamp to have not changed") // Test NotFound path. @@ -131,7 +134,7 @@ func (s *dbTestSuite) TestListTopics() { topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", Created: time.Unix(1672161102, 0), Modified: time.Unix(1672161102, 0), } @@ -178,6 +181,9 @@ func (s *dbTestSuite) TestUpdateTopic() { Modified: time.Unix(1672161102, 0), } + err := topic.Validate() + require.NoError(err, "could not validate topic data") + s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.TopicNamespace { return nil, status.Error(codes.FailedPrecondition, "bad Put request") @@ -191,7 +197,7 @@ func (s *dbTestSuite) TestUpdateTopic() { }, nil } - err := db.UpdateTopic(ctx, topic) + err = db.UpdateTopic(ctx, topic) require.NoError(err, "could not update topic") // Fields below should have been populated. From aefc8e638cb4745c96bb8ddebc068331a1351825 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Tue, 10 Jan 2023 14:16:22 -0500 Subject: [PATCH 04/12] Correct TopicUpdate NotFound test --- pkg/tenant/db/topics_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tenant/db/topics_test.go b/pkg/tenant/db/topics_test.go index 0ce4c3e3e..1811eecb8 100644 --- a/pkg/tenant/db/topics_test.go +++ b/pkg/tenant/db/topics_test.go @@ -208,7 +208,7 @@ func (s *dbTestSuite) TestUpdateTopic() { // Test NotFound path. // TODO: Use crypto rand and monotonic entropy with ulid.New. - err = db.UpdateTopic(ctx, &db.Topic{ID: ulid.Make()}) + err = db.UpdateTopic(ctx, &db.Topic{ProjectID: ulid.Make(), ID: ulid.Make(), Name: "topic002"}) require.ErrorIs(err, db.ErrNotFound) } From 3b5359da3d225564720cd567fa3822b6cdd83d97 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Tue, 10 Jan 2023 14:49:13 -0500 Subject: [PATCH 05/12] Update create and update handler tests to work with db validation --- pkg/tenant/members_test.go | 15 ++++++++------- pkg/tenant/projects_test.go | 18 +++++++++--------- pkg/tenant/topics_test.go | 10 +++++----- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/tenant/members_test.go b/pkg/tenant/members_test.go index 0acd933c5..8f145fa04 100644 --- a/pkg/tenant/members_test.go +++ b/pkg/tenant/members_test.go @@ -50,7 +50,7 @@ func (suite *tenantTestSuite) TestTenantMemberCreate() { // Create a member test fixture req := &api.Member{ - Name: "member-example", + Name: "member001", Role: "Admin", } @@ -89,7 +89,7 @@ func (suite *tenantTestSuite) TestMemberCreate() { // Create a member test fixture req := &api.Member{ - Name: "member-example", + Name: "member001", Role: "Admin", } @@ -150,9 +150,10 @@ func (suite *tenantTestSuite) TestMemberUpdate() { require := suite.Require() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) member := &db.Member{ - ID: ulid.MustParse("01ARZ3NDEKTSV4RRFFQ69G5FAV"), - Name: "member-example", - Role: "Admin", + TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), + ID: ulid.MustParse("01ARZ3NDEKTSV4RRFFQ69G5FAV"), + Name: "member001", + Role: "Admin", } defer cancel() @@ -191,12 +192,12 @@ func (suite *tenantTestSuite) TestMemberUpdate() { suite.requireError(err, http.StatusBadRequest, "member name is required", "expected error when member name does not exist") // Should return an error if the member role does not exist. - _, err = suite.client.MemberUpdate(ctx, &api.Member{ID: "01ARZ3NDEKTSV4RRFFQ69G5FAV", Name: "member-example"}) + _, err = suite.client.MemberUpdate(ctx, &api.Member{ID: "01ARZ3NDEKTSV4RRFFQ69G5FAV", Name: "member001"}) suite.requireError(err, http.StatusBadRequest, "member role is required", "expected error when member role does not exist") req := &api.Member{ ID: "01ARZ3NDEKTSV4RRFFQ69G5FAV", - Name: "member-example", + Name: "member001", Role: "Admin", } diff --git a/pkg/tenant/projects_test.go b/pkg/tenant/projects_test.go index d329dd301..44e3ca975 100644 --- a/pkg/tenant/projects_test.go +++ b/pkg/tenant/projects_test.go @@ -18,7 +18,7 @@ func (suite *tenantTestSuite) TestProjectDetail() { project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", } defer cancel() @@ -50,7 +50,7 @@ func (suite *tenantTestSuite) TestProjectDetail() { // Create a project test fixture. req := &api.Project{ ID: "01GKKYAWC4PA72YC53RVXAEC67", - Name: "project-example", + Name: "project001", } rep, err := suite.client.ProjectDetail(ctx, req.ID) @@ -73,7 +73,7 @@ func (suite *tenantTestSuite) TestProjectUpdate() { project := &db.Project{ TenantID: ulid.MustParse("01GMTWFK4XZY597Y128KXQ4WHP"), ID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), - Name: "project-example", + Name: "project001", } defer cancel() @@ -113,7 +113,7 @@ func (suite *tenantTestSuite) TestProjectUpdate() { req := &api.Project{ ID: "01GKKYAWC4PA72YC53RVXAEC67", - Name: "project-example", + Name: "project001", } rep, err := suite.client.ProjectUpdate(ctx, req) @@ -179,11 +179,11 @@ func (suite *tenantTestSuite) TestTenantProjectCreate() { } // Should return an error if tenant id is not a valid ULID. - _, err := suite.client.TenantProjectCreate(ctx, "tenantID", &api.Project{ID: "", Name: "project-example"}) + _, err := suite.client.TenantProjectCreate(ctx, "tenantID", &api.Project{ID: "", Name: "project001"}) suite.requireError(err, http.StatusBadRequest, "could not parse tenant id", "expected error when tenant id does not exist") // Should return an error if the project ID exists. - _, err = suite.client.TenantProjectCreate(ctx, tenantID, &api.Project{ID: "01GKKYAWC4PA72YC53RVXAEC67", Name: "project-example"}) + _, err = suite.client.TenantProjectCreate(ctx, tenantID, &api.Project{ID: "01GKKYAWC4PA72YC53RVXAEC67", Name: "project001"}) suite.requireError(err, http.StatusBadRequest, "project id cannot be specified on create", "expected error when project id exists") // Should return an error if the project name does not exist. @@ -192,7 +192,7 @@ func (suite *tenantTestSuite) TestTenantProjectCreate() { // Create a project test fixture. req := &api.Project{ - Name: "project-example", + Name: "project001", } project, err := suite.client.TenantProjectCreate(ctx, tenantID, req) @@ -216,7 +216,7 @@ func (suite *tenantTestSuite) TestProjectCreate() { } // Should return an error if a project ID exists. - _, err := suite.client.ProjectCreate(ctx, &api.Project{ID: "01GKKYAWC4PA72YC53RVXAEC67", Name: "project-example"}) + _, err := suite.client.ProjectCreate(ctx, &api.Project{ID: "01GKKYAWC4PA72YC53RVXAEC67", Name: "project001"}) suite.requireError(err, http.StatusBadRequest, "project id cannot be specified on create", "expected error when project id exists") // Should return an error if a project name does not exist. @@ -225,7 +225,7 @@ func (suite *tenantTestSuite) TestProjectCreate() { // Create a project test fixture. req := &api.Project{ - Name: "project-example", + Name: "project001", } project, err := suite.client.ProjectCreate(ctx, req) diff --git a/pkg/tenant/topics_test.go b/pkg/tenant/topics_test.go index c2ab56ca8..bf91894c2 100644 --- a/pkg/tenant/topics_test.go +++ b/pkg/tenant/topics_test.go @@ -18,7 +18,7 @@ func (suite *tenantTestSuite) TestTopicDetail() { topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", } defer cancel() @@ -50,7 +50,7 @@ func (suite *tenantTestSuite) TestTopicDetail() { // Create a topic test fixture. req := &api.Topic{ ID: "01GNA926JCTKDH3VZBTJM8MAF6", - Name: "topic-example", + Name: "topic001", } rep, err := suite.client.TopicDetail(ctx, req.ID) @@ -73,7 +73,7 @@ func (suite *tenantTestSuite) TestTopicUpdate() { topic := &db.Topic{ ProjectID: ulid.MustParse("01GNA91N6WMCWNG9MVSK47ZS88"), ID: ulid.MustParse("01GNA926JCTKDH3VZBTJM8MAF6"), - Name: "topic-example", + Name: "topic001", } defer cancel() @@ -114,7 +114,7 @@ func (suite *tenantTestSuite) TestTopicUpdate() { // Create a topic test fixture. req := &api.Topic{ ID: "01GNA926JCTKDH3VZBTJM8MAF6", - Name: "topic-example", + Name: "topic001", } rep, err := suite.client.TopicUpdate(ctx, req) @@ -192,7 +192,7 @@ func (suite *tenantTestSuite) TestProjectTopicCreate() { suite.requireError(err, http.StatusBadRequest, "topic name is required", "expected error when topic name does not exist") req := &api.Topic{ - Name: "topic-example", + Name: "topic001", } topic, err := suite.client.ProjectTopicCreate(ctx, projectID, req) From 450f1614cdd06f734b0b3c0c7e7edf7bd213e8b0 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Tue, 10 Jan 2023 15:51:45 -0500 Subject: [PATCH 06/12] Create validation for functions that don't require TenantID and update server-side handlers --- pkg/tenant/db/members.go | 43 ++++++++++++++++++++++++++++++++-- pkg/tenant/db/members_test.go | 34 +++++++++++++++++++++++++-- pkg/tenant/db/projects.go | 42 +++++++++++++++++++++++++++++++-- pkg/tenant/db/projects_test.go | 33 ++++++++++++++++++++++++-- pkg/tenant/members.go | 2 +- pkg/tenant/projects.go | 2 +- 6 files changed, 146 insertions(+), 10 deletions(-) diff --git a/pkg/tenant/db/members.go b/pkg/tenant/db/members.go index 7f4522458..1191a79fa 100644 --- a/pkg/tenant/db/members.go +++ b/pkg/tenant/db/members.go @@ -54,6 +54,22 @@ func (m *Member) UnmarshalValue(data []byte) error { } func (m *Member) Validate() error { + memberName := m.Name + + if strings.ContainsAny(string(memberName[0]), "0123456789") { + return ErrNumberFirstCharacter + } + + if strings.ContainsAny(memberName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { + return ErrSpecialCharacters + } + + return nil +} + +// Validates data in the member model when a TenantID is required. +// Ex. CreateTenantProject and UpdateProject +func (m *Member) ValidateWithID() error { if m.TenantID.Compare(ulid.ULID{}) == 0 { return ErrMissingTenantID } @@ -71,7 +87,30 @@ func (m *Member) Validate() error { return nil } -// CreateMember adds a new Member to the database. +// CreateTenantMember adds a new Member to a tenant in the database. +// Note: If a memberID is not passed in by the User, a new member id will be generated. +func CreateTenantMember(ctx context.Context, member *Member) (err error) { + // TODO: Use crypto rand and monotonic entropy with ulid.New + + // Validate tenant project data. + if err = member.ValidateWithID(); err != nil { + return err + } + + if member.ID.Compare(ulid.ULID{}) == 0 { + member.ID = ulid.Make() + } + + member.Created = time.Now() + member.Modified = member.Created + + if err = Put(ctx, member); err != nil { + return err + } + return nil +} + +// CreateMember adds a new Member to an organization in the database. // Note: If a memberID is not passed in by the User, a new member id will be generated. func CreateMember(ctx context.Context, member *Member) (err error) { // TODO: Use crypto rand and monotonic entropy with ulid.New @@ -137,7 +176,7 @@ func ListMembers(ctx context.Context, tenantID ulid.ULID) (members []*Member, er // UpdateMember updates the record of a member by its id. func UpdateMember(ctx context.Context, member *Member) (err error) { // Validate member data. - if err = member.Validate(); err != nil { + if err = member.ValidateWithID(); err != nil { return err } diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index b5f9e436e..22f55253f 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -25,7 +25,7 @@ func TestMemberModel(t *testing.T) { Modified: time.Unix(1670424445, 0).In(time.UTC), } - err := member.Validate() + err := member.ValidateWithID() require.NoError(t, err, "could not validate member data") key, err := member.Key() @@ -46,7 +46,7 @@ func TestMemberModel(t *testing.T) { MembersEqual(t, member, other) } -func (s *dbTestSuite) TestCreateMember() { +func (s *dbTestSuite) TestTenantCreateMember() { require := s.Require() ctx := context.Background() member := &db.Member{ @@ -55,6 +55,36 @@ func (s *dbTestSuite) TestCreateMember() { Role: "role-example", } + err := member.ValidateWithID() + require.NoError(err, "could not validate member data") + + // Call OnPut method from mock trtl database + s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { + if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.MembersNamespace { + return nil, status.Error(codes.FailedPrecondition, "bad Put request") + } + + return &pb.PutReply{ + Success: true, + }, nil + } + + err = db.CreateMember(ctx, member) + require.NoError(err, "could not create member") + + require.NotEqual("", member.ID, "expected non-zero ulid to be populated") + require.NotZero(member.Created, "expected member to have a created timestamp") + require.Equal(member.Created, member.Modified, "expected the same created and modified timestamp") +} + +func (s *dbTestSuite) TestCreateMember() { + require := s.Require() + ctx := context.Background() + member := &db.Member{ + Name: "member001", + Role: "role-example", + } + err := member.Validate() require.NoError(err, "could not validate member data") diff --git a/pkg/tenant/db/projects.go b/pkg/tenant/db/projects.go index c118c6401..fd7e3e526 100644 --- a/pkg/tenant/db/projects.go +++ b/pkg/tenant/db/projects.go @@ -52,6 +52,22 @@ func (p *Project) UnmarshalValue(data []byte) error { } func (p *Project) Validate() error { + projectName := p.Name + + if strings.ContainsAny(string(projectName[0]), "0123456789") { + return ErrNumberFirstCharacter + } + + if strings.ContainsAny(projectName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { + return ErrSpecialCharacters + } + + return nil +} + +// Validates data in the project model when a TenantID is required. +// Ex. CreateTenantProject and UpdateProject +func (p *Project) ValidateWithID() error { if p.TenantID.Compare(ulid.ULID{}) == 0 { return ErrMissingTenantID } @@ -69,7 +85,29 @@ func (p *Project) Validate() error { return nil } -// CreateProject adds a new project to the database. +// CreateTenantProject adds a new project to a tenant in the database. +// Note: If a project id is not passed in by the User, a new project id will be generated. +func CreateTenantProject(ctx context.Context, project *Project) (err error) { + // Validate project data. + if err = project.ValidateWithID(); err != nil { + return err + } + + // TODO: Use crypto rand and monotonic entropy with ulid.New + if project.ID.Compare(ulid.ULID{}) == 0 { + project.ID = ulid.Make() + } + + project.Created = time.Now() + project.Modified = project.Created + + if err = Put(ctx, project); err != nil { + return err + } + return nil +} + +// CreateProject adds a new project to an organization in the database. // Note: If a project id is not passed in by the User, a new project id will be generated. func CreateProject(ctx context.Context, project *Project) (err error) { // Validate project data. @@ -134,7 +172,7 @@ func ListProjects(ctx context.Context, tenantID ulid.ULID) (projects []*Project, // UpdateProject updates the record of a project by its id. func UpdateProject(ctx context.Context, project *Project) (err error) { // Validate project data. - if err = project.Validate(); err != nil { + if err = project.ValidateWithID(); err != nil { return err } diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index be6dfc715..dd42ebeff 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -24,7 +24,7 @@ func TestProjectModel(t *testing.T) { Modified: time.Unix(1670424445, 0).In(time.UTC), } - err := project.Validate() + err := project.ValidateWithID() require.NoError(t, err, "could not validate project data") key, err := project.Key() @@ -45,7 +45,7 @@ func TestProjectModel(t *testing.T) { ProjectsEqual(t, project, other, "unmarshaled project does not match marshaled project") } -func (s *dbTestSuite) TestCreateProject() { +func (s *dbTestSuite) TestCreateTenantProject() { require := s.Require() ctx := context.Background() project := &db.Project{ @@ -53,6 +53,35 @@ func (s *dbTestSuite) TestCreateProject() { Name: "project001", } + err := project.ValidateWithID() + require.NoError(err, "could not validate project data") + + s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { + if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.ProjectNamespace { + return nil, status.Error(codes.FailedPrecondition, "bad Put request") + } + + return &pb.PutReply{ + Success: true, + }, nil + } + + err = db.CreateProject(ctx, project) + require.NoError(err, "could not create project") + + // Verify that below fields have been populated. + require.NotZero(project.ID, "expected non-zero ulid to be populated") + require.NotZero(project.Created, "expected project to have a created timestamp") + require.Equal(project.Created, project.Modified, "expected the same created and modified timestamp") +} + +func (s *dbTestSuite) TestCreateProject() { + require := s.Require() + ctx := context.Background() + project := &db.Project{ + Name: "project001", + } + err := project.Validate() require.NoError(err, "could not validate project data") diff --git a/pkg/tenant/members.go b/pkg/tenant/members.go index 13632e07b..c4a1aa30d 100644 --- a/pkg/tenant/members.go +++ b/pkg/tenant/members.go @@ -68,7 +68,7 @@ func (s *Server) TenantMemberCreate(c *gin.Context) { Role: member.Role, } - if err = db.CreateMember(c.Request.Context(), tmember); err != nil { + if err = db.CreateTenantMember(c.Request.Context(), tmember); err != nil { log.Error().Err(err).Msg("could not create tenant member in the database") c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not add tenant member")) return diff --git a/pkg/tenant/projects.go b/pkg/tenant/projects.go index f48e23113..5ad71026f 100644 --- a/pkg/tenant/projects.go +++ b/pkg/tenant/projects.go @@ -62,7 +62,7 @@ func (s *Server) TenantProjectCreate(c *gin.Context) { } // Add project to the database and return a 500 response if it cannot be added. - if err = db.CreateProject(c.Request.Context(), tproject); err != nil { + if err = db.CreateTenantProject(c.Request.Context(), tproject); err != nil { log.Error().Err(err).Msg("could not create tenant project in the database") c.JSON(http.StatusInternalServerError, api.ErrorResponse("could not add tenant project")) return From eca5e28f9a68453a9b638b14ba8b0ae6a67de6f8 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Wed, 11 Jan 2023 14:07:45 -0500 Subject: [PATCH 07/12] Add check for resource name and separate ID validation from other data --- pkg/tenant/db/errors.go | 3 +++ pkg/tenant/db/members.go | 44 +++++++++++++++++++++------------- pkg/tenant/db/members_test.go | 21 +++++++++++----- pkg/tenant/db/projects.go | 31 ++++++++++++------------ pkg/tenant/db/projects_test.go | 19 +++++++++++---- pkg/tenant/db/topics.go | 10 +++++--- 6 files changed, 82 insertions(+), 46 deletions(-) diff --git a/pkg/tenant/db/errors.go b/pkg/tenant/db/errors.go index 7779d1875..4a1f9fcd0 100644 --- a/pkg/tenant/db/errors.go +++ b/pkg/tenant/db/errors.go @@ -10,4 +10,7 @@ var ( ErrMissingProjectID = errors.New("object requires project id for serialization") ErrNumberFirstCharacter = errors.New("name cannot begin with a number") ErrSpecialCharacters = errors.New("name cannot include special characters") + ErrMissingMemberName = errors.New("member name is required") + ErrMissingProjectName = errors.New("project name is required") + ErrMissingTopicName = errors.New("topic name is required") ) diff --git a/pkg/tenant/db/members.go b/pkg/tenant/db/members.go index 1191a79fa..5dedbdc7f 100644 --- a/pkg/tenant/db/members.go +++ b/pkg/tenant/db/members.go @@ -56,6 +56,10 @@ func (m *Member) UnmarshalValue(data []byte) error { func (m *Member) Validate() error { memberName := m.Name + if memberName == "" { + return ErrMissingMemberName + } + if strings.ContainsAny(string(memberName[0]), "0123456789") { return ErrNumberFirstCharacter } @@ -67,14 +71,24 @@ func (m *Member) Validate() error { return nil } -// Validates data in the member model when a TenantID is required. -// Ex. CreateTenantProject and UpdateProject -func (m *Member) ValidateWithID() error { +func (m *Member) ValidateID() error { if m.TenantID.Compare(ulid.ULID{}) == 0 { return ErrMissingTenantID } - memberName := m.Name + return nil +} + +// CreateTenantMember adds a new Member to a tenant in the database. +// Note: If a memberID is not passed in by the User, a new member id will be generated. +func CreateTenantMember(ctx context.Context, member *Member) (err error) { + // TODO: Use crypto rand and monotonic entropy with ulid.New + + if member.TenantID.Compare(ulid.ULID{}) == 0 { + return ErrMissingTenantID + } + + memberName := member.Name if strings.ContainsAny(string(memberName[0]), "0123456789") { return ErrNumberFirstCharacter @@ -84,18 +98,10 @@ func (m *Member) ValidateWithID() error { return ErrSpecialCharacters } - return nil -} - -// CreateTenantMember adds a new Member to a tenant in the database. -// Note: If a memberID is not passed in by the User, a new member id will be generated. -func CreateTenantMember(ctx context.Context, member *Member) (err error) { - // TODO: Use crypto rand and monotonic entropy with ulid.New - // Validate tenant project data. - if err = member.ValidateWithID(); err != nil { + /* if err = member.Validate(); err != nil { return err - } + } */ if member.ID.Compare(ulid.ULID{}) == 0 { member.ID = ulid.Make() @@ -116,9 +122,9 @@ func CreateMember(ctx context.Context, member *Member) (err error) { // TODO: Use crypto rand and monotonic entropy with ulid.New // Validate project data. - if err = member.Validate(); err != nil { + /* if err = member.Validate(); err != nil { return err - } + } */ if member.ID.Compare(ulid.ULID{}) == 0 { member.ID = ulid.Make() @@ -176,7 +182,11 @@ func ListMembers(ctx context.Context, tenantID ulid.ULID) (members []*Member, er // UpdateMember updates the record of a member by its id. func UpdateMember(ctx context.Context, member *Member) (err error) { // Validate member data. - if err = member.ValidateWithID(); err != nil { + if err = member.ValidateID(); err != nil { + return err + } + + if err = member.Validate(); err != nil { return err } diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 22f55253f..d9cbea0c3 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -25,7 +25,10 @@ func TestMemberModel(t *testing.T) { Modified: time.Unix(1670424445, 0).In(time.UTC), } - err := member.ValidateWithID() + err := member.ValidateID() + require.NoError(t, err, "could not validate tenant id") + + err = member.Validate() require.NoError(t, err, "could not validate member data") key, err := member.Key() @@ -46,7 +49,7 @@ func TestMemberModel(t *testing.T) { MembersEqual(t, member, other) } -func (s *dbTestSuite) TestTenantCreateMember() { +func (s *dbTestSuite) TestCreateTenantMember() { require := s.Require() ctx := context.Background() member := &db.Member{ @@ -55,7 +58,10 @@ func (s *dbTestSuite) TestTenantCreateMember() { Role: "role-example", } - err := member.ValidateWithID() + err := member.ValidateID() + require.NoError(err, "could not validate tenant id") + + err = member.Validate() require.NoError(err, "could not validate member data") // Call OnPut method from mock trtl database @@ -72,7 +78,7 @@ func (s *dbTestSuite) TestTenantCreateMember() { err = db.CreateMember(ctx, member) require.NoError(err, "could not create member") - require.NotEqual("", member.ID, "expected non-zero ulid to be populated") + require.NotEmpty(member.ID, "expected non-zero ulid to be populated") require.NotZero(member.Created, "expected member to have a created timestamp") require.Equal(member.Created, member.Modified, "expected the same created and modified timestamp") } @@ -102,7 +108,7 @@ func (s *dbTestSuite) TestCreateMember() { err = db.CreateMember(ctx, member) require.NoError(err, "could not create member") - require.NotEqual("", member.ID, "expected non-zero ulid to be populated") + require.NotEmpty(member.ID, "expected non-zero ulid to be populated") require.NotZero(member.Created, "expected member to have a created timestamp") require.Equal(member.Created, member.Modified, "expected the same created and modified timestamp") } @@ -210,7 +216,10 @@ func (s *dbTestSuite) TestUpdateMember() { Modified: time.Unix(1670424467, 0), } - err := member.Validate() + err := member.ValidateID() + require.NoError(err, "could not validate tenant id") + + err = member.Validate() require.NoError(err, "could not validate member data") // Call OnPut method from mock trtl database diff --git a/pkg/tenant/db/projects.go b/pkg/tenant/db/projects.go index fd7e3e526..13df9d791 100644 --- a/pkg/tenant/db/projects.go +++ b/pkg/tenant/db/projects.go @@ -54,6 +54,10 @@ func (p *Project) UnmarshalValue(data []byte) error { func (p *Project) Validate() error { projectName := p.Name + if projectName == "" { + return ErrMissingProjectName + } + if strings.ContainsAny(string(projectName[0]), "0123456789") { return ErrNumberFirstCharacter } @@ -65,31 +69,24 @@ func (p *Project) Validate() error { return nil } -// Validates data in the project model when a TenantID is required. -// Ex. CreateTenantProject and UpdateProject -func (p *Project) ValidateWithID() error { +func (p *Project) ValidateID() error { if p.TenantID.Compare(ulid.ULID{}) == 0 { return ErrMissingTenantID } - projectName := p.Name - - if strings.ContainsAny(string(projectName[0]), "0123456789") { - return ErrNumberFirstCharacter - } - - if strings.ContainsAny(projectName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { - return ErrSpecialCharacters - } - return nil } // CreateTenantProject adds a new project to a tenant in the database. // Note: If a project id is not passed in by the User, a new project id will be generated. func CreateTenantProject(ctx context.Context, project *Project) (err error) { + + if err = project.ValidateID(); err != nil { + return err + } + // Validate project data. - if err = project.ValidateWithID(); err != nil { + if err = project.Validate(); err != nil { return err } @@ -172,7 +169,11 @@ func ListProjects(ctx context.Context, tenantID ulid.ULID) (projects []*Project, // UpdateProject updates the record of a project by its id. func UpdateProject(ctx context.Context, project *Project) (err error) { // Validate project data. - if err = project.ValidateWithID(); err != nil { + if err = project.ValidateID(); err != nil { + return err + } + + if err = project.Validate(); err != nil { return err } diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index dd42ebeff..391d177d2 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -24,7 +24,10 @@ func TestProjectModel(t *testing.T) { Modified: time.Unix(1670424445, 0).In(time.UTC), } - err := project.ValidateWithID() + err := project.ValidateID() + require.NoError(t, err, "could not validate tenant id") + + err = project.Validate() require.NoError(t, err, "could not validate project data") key, err := project.Key() @@ -53,7 +56,10 @@ func (s *dbTestSuite) TestCreateTenantProject() { Name: "project001", } - err := project.ValidateWithID() + err := project.ValidateID() + require.NoError(err, "could not validate tenant id") + + err = project.Validate() require.NoError(err, "could not validate project data") s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { @@ -70,7 +76,7 @@ func (s *dbTestSuite) TestCreateTenantProject() { require.NoError(err, "could not create project") // Verify that below fields have been populated. - require.NotZero(project.ID, "expected non-zero ulid to be populated") + require.NotEmpty(project.ID, "expected non-zero ulid to be populated") require.NotZero(project.Created, "expected project to have a created timestamp") require.Equal(project.Created, project.Modified, "expected the same created and modified timestamp") } @@ -99,7 +105,7 @@ func (s *dbTestSuite) TestCreateProject() { require.NoError(err, "could not create project") // Verify that below fields have been populated. - require.NotZero(project.ID, "expected non-zero ulid to be populated") + require.NotEmpty(project.ID, "expected non-zero ulid to be populated") require.NotZero(project.Created, "expected project to have a created timestamp") require.Equal(project.Created, project.Modified, "expected the same created and modified timestamp") } @@ -211,7 +217,10 @@ func (s *dbTestSuite) TestUpdateProject() { Modified: time.Unix(1668660681, 0), } - err := project.Validate() + err := project.ValidateID() + require.NoError(err, "could not validate tenant id") + + err = project.Validate() require.NoError(err, "could not validate project data") s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { diff --git a/pkg/tenant/db/topics.go b/pkg/tenant/db/topics.go index 9df21ceb6..1e9736cf9 100644 --- a/pkg/tenant/db/topics.go +++ b/pkg/tenant/db/topics.go @@ -57,13 +57,17 @@ func (t *Topic) Validate() error { return ErrMissingID } - name := t.Name + topicName := t.Name - if strings.ContainsAny(string(name[0]), "0123456789") { + if topicName == "" { + return ErrMissingTopicName + } + + if strings.ContainsAny(string(topicName[0]), "0123456789") { return ErrNumberFirstCharacter } - if strings.ContainsAny(name, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { + if strings.ContainsAny(topicName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { return ErrSpecialCharacters } return nil From 72bf3991f4e11c04d619c5a60366ff6d1927fa83 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Fri, 13 Jan 2023 11:46:50 -0500 Subject: [PATCH 08/12] Remove ValidateID method --- pkg/tenant/db/members.go | 52 +++++++++------------------------- pkg/tenant/db/members_test.go | 20 ++++--------- pkg/tenant/db/projects.go | 35 +++++++---------------- pkg/tenant/db/projects_test.go | 15 ++-------- 4 files changed, 33 insertions(+), 89 deletions(-) diff --git a/pkg/tenant/db/members.go b/pkg/tenant/db/members.go index 5dedbdc7f..d69d67e73 100644 --- a/pkg/tenant/db/members.go +++ b/pkg/tenant/db/members.go @@ -54,6 +54,10 @@ func (m *Member) UnmarshalValue(data []byte) error { } func (m *Member) Validate() error { + if m.TenantID.Compare(ulid.ULID{}) == 0 { + return ErrMissingTenantID + } + memberName := m.Name if memberName == "" { @@ -71,40 +75,18 @@ func (m *Member) Validate() error { return nil } -func (m *Member) ValidateID() error { - if m.TenantID.Compare(ulid.ULID{}) == 0 { - return ErrMissingTenantID - } - - return nil -} - // CreateTenantMember adds a new Member to a tenant in the database. // Note: If a memberID is not passed in by the User, a new member id will be generated. func CreateTenantMember(ctx context.Context, member *Member) (err error) { // TODO: Use crypto rand and monotonic entropy with ulid.New - if member.TenantID.Compare(ulid.ULID{}) == 0 { - return ErrMissingTenantID - } - - memberName := member.Name - - if strings.ContainsAny(string(memberName[0]), "0123456789") { - return ErrNumberFirstCharacter - } - - if strings.ContainsAny(memberName, " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~") { - return ErrSpecialCharacters + if member.ID.Compare(ulid.ULID{}) == 0 { + member.ID = ulid.Make() } - // Validate tenant project data. - /* if err = member.Validate(); err != nil { + // Validate tenant member data. + if err = member.Validate(); err != nil { return err - } */ - - if member.ID.Compare(ulid.ULID{}) == 0 { - member.ID = ulid.Make() } member.Created = time.Now() @@ -121,11 +103,6 @@ func CreateTenantMember(ctx context.Context, member *Member) (err error) { func CreateMember(ctx context.Context, member *Member) (err error) { // TODO: Use crypto rand and monotonic entropy with ulid.New - // Validate project data. - /* if err = member.Validate(); err != nil { - return err - } */ - if member.ID.Compare(ulid.ULID{}) == 0 { member.ID = ulid.Make() } @@ -181,14 +158,6 @@ func ListMembers(ctx context.Context, tenantID ulid.ULID) (members []*Member, er // UpdateMember updates the record of a member by its id. func UpdateMember(ctx context.Context, member *Member) (err error) { - // Validate member data. - if err = member.ValidateID(); err != nil { - return err - } - - if err = member.Validate(); err != nil { - return err - } // TODO: Use crypto rand and monotonic entropy with ulid.New @@ -198,6 +167,11 @@ func UpdateMember(ctx context.Context, member *Member) (err error) { return ErrMissingID } + // Validate member data. + if err = member.Validate(); err != nil { + return err + } + member.Modified = time.Now() if err = Put(ctx, member); err != nil { diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index d9cbea0c3..00f8c76ca 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -25,10 +25,7 @@ func TestMemberModel(t *testing.T) { Modified: time.Unix(1670424445, 0).In(time.UTC), } - err := member.ValidateID() - require.NoError(t, err, "could not validate tenant id") - - err = member.Validate() + err := member.Validate() require.NoError(t, err, "could not validate member data") key, err := member.Key() @@ -58,10 +55,7 @@ func (s *dbTestSuite) TestCreateTenantMember() { Role: "role-example", } - err := member.ValidateID() - require.NoError(err, "could not validate tenant id") - - err = member.Validate() + err := member.Validate() require.NoError(err, "could not validate member data") // Call OnPut method from mock trtl database @@ -87,8 +81,9 @@ func (s *dbTestSuite) TestCreateMember() { require := s.Require() ctx := context.Background() member := &db.Member{ - Name: "member001", - Role: "role-example", + TenantID: ulid.MustParse("01GKKYAWC4PA72YC53RVXAEC67"), + Name: "member001", + Role: "role-example", } err := member.Validate() @@ -216,10 +211,7 @@ func (s *dbTestSuite) TestUpdateMember() { Modified: time.Unix(1670424467, 0), } - err := member.ValidateID() - require.NoError(err, "could not validate tenant id") - - err = member.Validate() + err := member.Validate() require.NoError(err, "could not validate member data") // Call OnPut method from mock trtl database diff --git a/pkg/tenant/db/projects.go b/pkg/tenant/db/projects.go index 13df9d791..19846167c 100644 --- a/pkg/tenant/db/projects.go +++ b/pkg/tenant/db/projects.go @@ -52,6 +52,10 @@ func (p *Project) UnmarshalValue(data []byte) error { } func (p *Project) Validate() error { + if p.TenantID.Compare(ulid.ULID{}) == 0 { + return ErrMissingTenantID + } + projectName := p.Name if projectName == "" { @@ -69,20 +73,12 @@ func (p *Project) Validate() error { return nil } -func (p *Project) ValidateID() error { - if p.TenantID.Compare(ulid.ULID{}) == 0 { - return ErrMissingTenantID - } - - return nil -} - // CreateTenantProject adds a new project to a tenant in the database. // Note: If a project id is not passed in by the User, a new project id will be generated. func CreateTenantProject(ctx context.Context, project *Project) (err error) { - - if err = project.ValidateID(); err != nil { - return err + // TODO: Use crypto rand and monotonic entropy with ulid.New + if project.ID.Compare(ulid.ULID{}) == 0 { + project.ID = ulid.Make() } // Validate project data. @@ -90,11 +86,6 @@ func CreateTenantProject(ctx context.Context, project *Project) (err error) { return err } - // TODO: Use crypto rand and monotonic entropy with ulid.New - if project.ID.Compare(ulid.ULID{}) == 0 { - project.ID = ulid.Make() - } - project.Created = time.Now() project.Modified = project.Created @@ -168,20 +159,16 @@ func ListProjects(ctx context.Context, tenantID ulid.ULID) (projects []*Project, // UpdateProject updates the record of a project by its id. func UpdateProject(ctx context.Context, project *Project) (err error) { - // Validate project data. - if err = project.ValidateID(); err != nil { - return err + // TODO: Use crypto rand and monotonic entropy with ulid.New + if project.ID.Compare(ulid.ULID{}) == 0 { + return ErrMissingID } + // Validate project data. if err = project.Validate(); err != nil { return err } - // TODO: Use crypto rand and monotonic entropy with ulid.New - if project.ID.Compare(ulid.ULID{}) == 0 { - return ErrMissingID - } - project.Modified = time.Now() if err = Put(ctx, project); err != nil { diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index 391d177d2..2a484d178 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -24,10 +24,7 @@ func TestProjectModel(t *testing.T) { Modified: time.Unix(1670424445, 0).In(time.UTC), } - err := project.ValidateID() - require.NoError(t, err, "could not validate tenant id") - - err = project.Validate() + err := project.Validate() require.NoError(t, err, "could not validate project data") key, err := project.Key() @@ -56,10 +53,7 @@ func (s *dbTestSuite) TestCreateTenantProject() { Name: "project001", } - err := project.ValidateID() - require.NoError(err, "could not validate tenant id") - - err = project.Validate() + err := project.Validate() require.NoError(err, "could not validate project data") s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { @@ -217,10 +211,7 @@ func (s *dbTestSuite) TestUpdateProject() { Modified: time.Unix(1668660681, 0), } - err := project.ValidateID() - require.NoError(err, "could not validate tenant id") - - err = project.Validate() + err := project.Validate() require.NoError(err, "could not validate project data") s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { From 916638462ec13bfde9706f8e2b4535d8670d9db6 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Fri, 13 Jan 2023 11:51:05 -0500 Subject: [PATCH 09/12] Remove validate from CreateProject --- pkg/tenant/db/projects.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/tenant/db/projects.go b/pkg/tenant/db/projects.go index 19846167c..b0e8653ab 100644 --- a/pkg/tenant/db/projects.go +++ b/pkg/tenant/db/projects.go @@ -98,11 +98,6 @@ func CreateTenantProject(ctx context.Context, project *Project) (err error) { // CreateProject adds a new project to an organization in the database. // Note: If a project id is not passed in by the User, a new project id will be generated. func CreateProject(ctx context.Context, project *Project) (err error) { - // Validate project data. - if err = project.Validate(); err != nil { - return err - } - // TODO: Use crypto rand and monotonic entropy with ulid.New if project.ID.Compare(ulid.ULID{}) == 0 { project.ID = ulid.Make() From 60bb506c0786424f3a34399d93f8ead5d65652ab Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Fri, 13 Jan 2023 12:06:12 -0500 Subject: [PATCH 10/12] Remove validation from create tests --- pkg/tenant/db/members_test.go | 5 +---- pkg/tenant/db/projects_test.go | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 00f8c76ca..7ef1f9b8e 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -86,9 +86,6 @@ func (s *dbTestSuite) TestCreateMember() { Role: "role-example", } - err := member.Validate() - require.NoError(err, "could not validate member data") - // Call OnPut method from mock trtl database s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.MembersNamespace { @@ -100,7 +97,7 @@ func (s *dbTestSuite) TestCreateMember() { }, nil } - err = db.CreateMember(ctx, member) + err := db.CreateMember(ctx, member) require.NoError(err, "could not create member") require.NotEmpty(member.ID, "expected non-zero ulid to be populated") diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index 2a484d178..c30eb5601 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -82,9 +82,6 @@ func (s *dbTestSuite) TestCreateProject() { Name: "project001", } - err := project.Validate() - require.NoError(err, "could not validate project data") - s.mock.OnPut = func(ctx context.Context, in *pb.PutRequest) (*pb.PutReply, error) { if len(in.Key) == 0 || len(in.Value) == 0 || in.Namespace != db.ProjectNamespace { return nil, status.Error(codes.FailedPrecondition, "bad Put request") @@ -95,7 +92,7 @@ func (s *dbTestSuite) TestCreateProject() { }, nil } - err = db.CreateProject(ctx, project) + err := db.CreateProject(ctx, project) require.NoError(err, "could not create project") // Verify that below fields have been populated. From 4e834be97af2d82cf6ca296a8cde4d378ea018ff Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Fri, 13 Jan 2023 12:20:38 -0500 Subject: [PATCH 11/12] Minor test update --- pkg/tenant/db/members_test.go | 2 +- pkg/tenant/db/projects_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 7ef1f9b8e..6e45b090c 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -69,7 +69,7 @@ func (s *dbTestSuite) TestCreateTenantMember() { }, nil } - err = db.CreateMember(ctx, member) + err = db.CreateTenantMember(ctx, member) require.NoError(err, "could not create member") require.NotEmpty(member.ID, "expected non-zero ulid to be populated") diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index c30eb5601..cc5cd2332 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -66,7 +66,7 @@ func (s *dbTestSuite) TestCreateTenantProject() { }, nil } - err = db.CreateProject(ctx, project) + err = db.CreateTenantProject(ctx, project) require.NoError(err, "could not create project") // Verify that below fields have been populated. From 442c36daab35e504af442e6498369a0091e94988 Mon Sep 17 00:00:00 2001 From: daniellemaxwell Date: Fri, 13 Jan 2023 16:58:56 -0500 Subject: [PATCH 12/12] Add check for empty name string in tests --- pkg/tenant/db/members_test.go | 1 + pkg/tenant/db/projects_test.go | 1 + pkg/tenant/db/tenants_test.go | 2 +- pkg/tenant/db/topics_test.go | 4 ++-- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/tenant/db/members_test.go b/pkg/tenant/db/members_test.go index 6e45b090c..8877df726 100644 --- a/pkg/tenant/db/members_test.go +++ b/pkg/tenant/db/members_test.go @@ -73,6 +73,7 @@ func (s *dbTestSuite) TestCreateTenantMember() { require.NoError(err, "could not create member") require.NotEmpty(member.ID, "expected non-zero ulid to be populated") + require.NotEmpty(member.Name, "member name is required") require.NotZero(member.Created, "expected member to have a created timestamp") require.Equal(member.Created, member.Modified, "expected the same created and modified timestamp") } diff --git a/pkg/tenant/db/projects_test.go b/pkg/tenant/db/projects_test.go index cc5cd2332..9ba5a8ca4 100644 --- a/pkg/tenant/db/projects_test.go +++ b/pkg/tenant/db/projects_test.go @@ -71,6 +71,7 @@ func (s *dbTestSuite) TestCreateTenantProject() { // Verify that below fields have been populated. require.NotEmpty(project.ID, "expected non-zero ulid to be populated") + require.NotEmpty(project.Name, "project name is required") require.NotZero(project.Created, "expected project to have a created timestamp") require.Equal(project.Created, project.Modified, "expected the same created and modified timestamp") } diff --git a/pkg/tenant/db/tenants_test.go b/pkg/tenant/db/tenants_test.go index c031545fa..ea30af179 100644 --- a/pkg/tenant/db/tenants_test.go +++ b/pkg/tenant/db/tenants_test.go @@ -65,7 +65,7 @@ func (s *dbTestSuite) TestCreateTenant() { require.NoError(err, "could not create tenant") // Fields should have been populated - require.NotZero(tenant.ID, "expected non-zero ulid to be populated") + require.NotEmpty(tenant.ID, "expected non-zero ulid to be populated") require.NotZero(tenant.Created, "expected tenant to have a created timestamp") require.Equal(tenant.Created, tenant.Modified, "expected the same created and modified timestamp") } diff --git a/pkg/tenant/db/topics_test.go b/pkg/tenant/db/topics_test.go index 1811eecb8..e1cb856a7 100644 --- a/pkg/tenant/db/topics_test.go +++ b/pkg/tenant/db/topics_test.go @@ -68,8 +68,8 @@ func (s *dbTestSuite) TestCreateTopic() { require.NoError(err, "could not create topic") // Verify that below fields have been populated. - require.NotZero(topic.ProjectID, "expected non-zero ulid to be populated for project id") - require.NotZero(topic.ID, "expected non-zero ulid to be populated for topic id") + require.NotEmpty(topic.ID, "expected non-zero ulid to be populated for topic id") + require.NotEmpty(topic.Name, "topic name is required") require.NotZero(topic.Created, "expected topic to have a created timestamp") require.Equal(topic.Created, topic.Modified, "expected the same created and modified timestamp")