From 78a1b8f0197b10174b6849fd5f7f761ac8cd20c4 Mon Sep 17 00:00:00 2001 From: Silvan Date: Tue, 24 Nov 2020 15:55:38 +0100 Subject: [PATCH] fix: org member change and remove (#1014) * fix: member * fix: test * fix: test * fix: tests --- internal/org/model/org.go | 8 +++---- .../repository/eventsourcing/eventstore.go | 23 +++++++++++-------- .../repository/eventsourcing/org_member.go | 13 ++++++----- .../eventsourcing/org_member_test.go | 16 +++++++++++-- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/internal/org/model/org.go b/internal/org/model/org.go index c5ec7829a3..3a0bdec40e 100644 --- a/internal/org/model/org.go +++ b/internal/org/model/org.go @@ -85,13 +85,13 @@ func (o *Org) GetPrimaryDomain() *OrgDomain { return nil } -func (o *Org) ContainsMember(userID string) bool { - for _, member := range o.Members { +func (o *Org) MemeberByUserID(userID string) (*OrgMember, int) { + for i, member := range o.Members { if member.UserID == userID { - return true + return member, i } } - return false + return nil, -1 } func (o *Org) nameForDomain(iamDomain string) string { diff --git a/internal/org/repository/eventsourcing/eventstore.go b/internal/org/repository/eventsourcing/eventstore.go index c6bfed3618..8db5c785eb 100644 --- a/internal/org/repository/eventsourcing/eventstore.go +++ b/internal/org/repository/eventsourcing/eventstore.go @@ -429,16 +429,20 @@ func (es *OrgEventstore) ChangeOrgMember(ctx context.Context, member *org_model. return nil, errors.ThrowPreconditionFailed(nil, "EVENT-ara6l", "Errors.Org.InvalidMember") } - existingMember, err := es.OrgMemberByIDs(ctx, member) + org, err := es.OrgByID(ctx, &org_model.Org{ObjectRoot: es_models.ObjectRoot{AggregateID: member.AggregateID, Sequence: member.Sequence}}) if err != nil { return nil, err } + existingMember, _ := org.MemeberByUserID(member.UserID) + if existingMember == nil { + return nil, errors.ThrowNotFound(nil, "EVENT-VB2Pn", "Errors.Org.MemberNotExisting") + } - member.ObjectRoot = existingMember.ObjectRoot + repoOrg := model.OrgFromModel(org) repoMember := model.OrgMemberFromModel(member) repoExistingMember := model.OrgMemberFromModel(existingMember) - orgAggregate := orgMemberChangedAggregate(es.Eventstore.AggregateCreator(), repoExistingMember, repoMember) + orgAggregate := orgMemberChangedAggregate(es.Eventstore.AggregateCreator(), repoOrg, repoExistingMember, repoMember) err = es_sdk.Push(ctx, es.PushAggregates, repoMember.AppendEvents, orgAggregate) if err != nil { return nil, err @@ -452,18 +456,19 @@ func (es *OrgEventstore) RemoveOrgMember(ctx context.Context, member *org_model. return errors.ThrowInvalidArgument(nil, "EVENT-d43fs", "Errors.Org.UserIDMissing") } - existingMember, err := es.OrgMemberByIDs(ctx, member) - if errors.IsNotFound(err) { - return nil - } + org, err := es.OrgByID(ctx, &org_model.Org{ObjectRoot: es_models.ObjectRoot{AggregateID: member.AggregateID, Sequence: member.Sequence}}) if err != nil { return err } + existingMember, _ := org.MemeberByUserID(member.UserID) + if existingMember == nil { + return nil + } - member.ObjectRoot = existingMember.ObjectRoot + repoOrg := model.OrgFromModel(org) repoMember := model.OrgMemberFromModel(member) - orgAggregate := orgMemberRemovedAggregate(es.Eventstore.AggregateCreator(), repoMember) + orgAggregate := orgMemberRemovedAggregate(es.Eventstore.AggregateCreator(), repoOrg, repoMember) return es_sdk.Push(ctx, es.PushAggregates, repoMember.AppendEvents, orgAggregate) } diff --git a/internal/org/repository/eventsourcing/org_member.go b/internal/org/repository/eventsourcing/org_member.go index 7034de287e..24b67c2f38 100644 --- a/internal/org/repository/eventsourcing/org_member.go +++ b/internal/org/repository/eventsourcing/org_member.go @@ -2,6 +2,7 @@ package eventsourcing import ( "context" + "github.com/caos/zitadel/internal/org/repository/eventsourcing/model" "github.com/caos/zitadel/internal/errors" @@ -31,9 +32,9 @@ func orgMemberAddedAggregate(ctx context.Context, aggCreator *es_models.Aggregat return agg.SetPrecondition(validationQuery, validation).AppendEvent(model.OrgMemberAdded, member) } -func orgMemberChangedAggregate(aggCreator *es_models.AggregateCreator, existingMember *model.OrgMember, member *model.OrgMember) func(ctx context.Context) (*es_models.Aggregate, error) { +func orgMemberChangedAggregate(aggCreator *es_models.AggregateCreator, org *model.Org, existingMember, member *model.OrgMember) func(ctx context.Context) (*es_models.Aggregate, error) { return func(ctx context.Context) (*es_models.Aggregate, error) { - if member == nil || existingMember == nil { + if member == nil || org == nil || existingMember == nil { return nil, errors.ThrowPreconditionFailed(nil, "EVENT-d34fs", "Errors.Internal") } @@ -42,7 +43,7 @@ func orgMemberChangedAggregate(aggCreator *es_models.AggregateCreator, existingM return nil, errors.ThrowInvalidArgument(nil, "EVENT-VLMGn", "Errors.NoChangesFound") } - agg, err := OrgAggregate(ctx, aggCreator, existingMember.AggregateID, existingMember.Sequence) + agg, err := OrgAggregate(ctx, aggCreator, org.AggregateID, org.Sequence) if err != nil { return nil, err } @@ -50,13 +51,13 @@ func orgMemberChangedAggregate(aggCreator *es_models.AggregateCreator, existingM } } -func orgMemberRemovedAggregate(aggCreator *es_models.AggregateCreator, member *model.OrgMember) func(ctx context.Context) (*es_models.Aggregate, error) { +func orgMemberRemovedAggregate(aggCreator *es_models.AggregateCreator, org *model.Org, member *model.OrgMember) func(ctx context.Context) (*es_models.Aggregate, error) { return func(ctx context.Context) (*es_models.Aggregate, error) { if member == nil { - return nil, errors.ThrowPreconditionFailed(nil, "EVENT-dieu7", "member must not be nil") + return nil, errors.ThrowPreconditionFailed(nil, "EVENT-vNPVX", "Errors.Internal") } - agg, err := OrgAggregate(ctx, aggCreator, member.AggregateID, member.Sequence) + agg, err := OrgAggregate(ctx, aggCreator, org.AggregateID, org.Sequence) if err != nil { return nil, err } diff --git a/internal/org/repository/eventsourcing/org_member_test.go b/internal/org/repository/eventsourcing/org_member_test.go index 9c5c009953..59e571e6f3 100644 --- a/internal/org/repository/eventsourcing/org_member_test.go +++ b/internal/org/repository/eventsourcing/org_member_test.go @@ -78,6 +78,7 @@ func TestOrgMemberChangedAggregate(t *testing.T) { } type args struct { aggCreator *es_models.AggregateCreator + org *model.Org existingMember *model.OrgMember member *model.OrgMember ctx context.Context @@ -92,6 +93,7 @@ func TestOrgMemberChangedAggregate(t *testing.T) { args: args{ aggCreator: es_models.NewAggregateCreator("test"), ctx: authz.NewMockContext("org", "user"), + org: &model.Org{}, member: nil, existingMember: &model.OrgMember{}, }, @@ -104,6 +106,7 @@ func TestOrgMemberChangedAggregate(t *testing.T) { args: args{ aggCreator: es_models.NewAggregateCreator("test"), ctx: authz.NewMockContext("org", "user"), + org: &model.Org{}, existingMember: nil, member: &model.OrgMember{}, }, @@ -122,6 +125,7 @@ func TestOrgMemberChangedAggregate(t *testing.T) { existingMember: &model.OrgMember{ ObjectRoot: es_models.ObjectRoot{AggregateID: "asdf", Sequence: 234}, }, + org: &model.Org{}, }, res: res{ isErr: errors.IsErrorInvalidArgument, @@ -140,6 +144,9 @@ func TestOrgMemberChangedAggregate(t *testing.T) { ObjectRoot: es_models.ObjectRoot{AggregateID: "asdf", Sequence: 234}, Roles: []string{"asdf", "woeri"}, }, + org: &model.Org{ + ObjectRoot: es_models.ObjectRoot{AggregateID: "asdf", Sequence: 234}, + }, }, res: res{ isErr: nil, @@ -149,7 +156,7 @@ func TestOrgMemberChangedAggregate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - aggregateCreator := orgMemberChangedAggregate(tt.args.aggCreator, tt.args.existingMember, tt.args.member) + aggregateCreator := orgMemberChangedAggregate(tt.args.aggCreator, tt.args.org, tt.args.existingMember, tt.args.member) aggregate, err := aggregateCreator(tt.args.ctx) if tt.res.isErr == nil && err != nil { t.Errorf("no error expected got: %v", err) @@ -174,6 +181,7 @@ func TestOrgMemberRemovedAggregate(t *testing.T) { } type args struct { aggCreator *es_models.AggregateCreator + org *model.Org member *model.OrgMember ctx context.Context } @@ -187,6 +195,7 @@ func TestOrgMemberRemovedAggregate(t *testing.T) { args: args{ aggCreator: es_models.NewAggregateCreator("test"), ctx: authz.NewMockContext("org", "user"), + org: &model.Org{}, member: nil, }, res: res{ @@ -198,6 +207,9 @@ func TestOrgMemberRemovedAggregate(t *testing.T) { args: args{ aggCreator: es_models.NewAggregateCreator("test"), ctx: authz.NewMockContext("org", "user"), + org: &model.Org{ + ObjectRoot: es_models.ObjectRoot{AggregateID: "asdf", Sequence: 234}, + }, member: &model.OrgMember{ ObjectRoot: es_models.ObjectRoot{AggregateID: "asdf", Sequence: 234}, }, @@ -210,7 +222,7 @@ func TestOrgMemberRemovedAggregate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - aggregateCreator := orgMemberRemovedAggregate(tt.args.aggCreator, tt.args.member) + aggregateCreator := orgMemberRemovedAggregate(tt.args.aggCreator, tt.args.org, tt.args.member) aggregate, err := aggregateCreator(tt.args.ctx) if tt.res.isErr == nil && err != nil { t.Errorf("no error expected got: %v", err)