From 831bb88ec43ece675647cf00e3a5c059be1290d9 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Sat, 9 Dec 2023 10:59:51 +0200 Subject: [PATCH] fix: correctly delete sessions created before 2.42 (#7050) * fix: correctly delete sessions created before 2.42 * fix test * fix linting * fixes requested from review --- internal/command/resource_ower_model.go | 41 +++++++++++++++++++ internal/command/session.go | 22 +++++++++- internal/command/session_model.go | 3 +- internal/command/session_test.go | 54 ++++++++++++++++++++++--- 4 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 internal/command/resource_ower_model.go diff --git a/internal/command/resource_ower_model.go b/internal/command/resource_ower_model.go new file mode 100644 index 0000000000..e5e047c273 --- /dev/null +++ b/internal/command/resource_ower_model.go @@ -0,0 +1,41 @@ +package command + +import ( + "github.com/zitadel/zitadel/internal/eventstore" +) + +// resourceOwnerModel can be used to retrieve the resourceOwner of an aggregate +// by checking the first event it. +type resourceOwnerModel struct { + instanceID string + aggregateType eventstore.AggregateType + aggregateID string + + resourceOwner string +} + +func NewResourceOwnerModel(instanceID string, aggregateType eventstore.AggregateType, aggregateID string) *resourceOwnerModel { + return &resourceOwnerModel{ + instanceID: instanceID, + aggregateType: aggregateType, + aggregateID: aggregateID, + } +} + +func (r *resourceOwnerModel) Reduce() error { + return nil +} +func (r *resourceOwnerModel) AppendEvents(events ...eventstore.Event) { + if len(events) == 1 { + r.resourceOwner = events[0].Aggregate().ResourceOwner + } +} +func (r *resourceOwnerModel) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + InstanceID(r.instanceID). + AddQuery(). + AggregateTypes(r.aggregateType). + AggregateIDs(r.aggregateID). + Builder(). + Limit(1) +} diff --git a/internal/command/session.go b/internal/command/session.go index f359860ba3..147921b550 100644 --- a/internal/command/session.go +++ b/internal/command/session.go @@ -400,7 +400,27 @@ func (c *Commands) checkSessionTerminationPermission(ctx context.Context, model if model.UserID != "" && model.UserID == authz.GetCtxData(ctx).UserID { return nil } - return c.checkPermission(ctx, domain.PermissionSessionDelete, model.UserResourceOwner, model.UserID) + userResourceOwner, err := c.sessionUserResourceOwner(ctx, model) + if err != nil { + return err + } + return c.checkPermission(ctx, domain.PermissionSessionDelete, userResourceOwner, model.UserID) +} + +// sessionUserResourceOwner will return the resourceOwner of the session form the [SessionWriteModel] or by additionally calling the eventstore, +// because before 2.42.0, the resourceOwner of a session used to be the organisation of the creator. +// Further the (checked) users organisation id was not stored. +// To be able to check the permission, we need to get the user's resourceOwner in this case. +func (c *Commands) sessionUserResourceOwner(ctx context.Context, model *SessionWriteModel) (string, error) { + if model.UserID == "" || model.UserResourceOwner != "" { + return model.UserResourceOwner, nil + } + r := NewResourceOwnerModel(authz.GetInstance(ctx).InstanceID(), user.AggregateType, model.UserID) + err := c.eventstore.FilterToQueryReducer(ctx, r) + if err != nil { + return "", err + } + return r.resourceOwner, nil } func sessionTokenCreator(idGenerator id.Generator, sessionAlg crypto.EncryptionAlgorithm) func(sessionID string) (id string, token string, err error) { diff --git a/internal/command/session_model.go b/internal/command/session_model.go index 7059c4ab95..6076461f5a 100644 --- a/internal/command/session_model.go +++ b/internal/command/session_model.go @@ -62,8 +62,7 @@ type SessionWriteModel struct { func NewSessionWriteModel(sessionID string, instanceID string) *SessionWriteModel { return &SessionWriteModel{ WriteModel: eventstore.WriteModel{ - AggregateID: sessionID, - ResourceOwner: instanceID, + AggregateID: sessionID, }, Metadata: make(map[string][]byte), aggregate: &session.NewAggregate(sessionID, instanceID).Aggregate, diff --git a/internal/command/session_test.go b/internal/command/session_test.go index 6862aba3fb..16308432b3 100644 --- a/internal/command/session_test.go +++ b/internal/command/session_test.go @@ -487,11 +487,9 @@ func TestCommands_updateSession(t *testing.T) { }, res{ want: &SessionChanged{ - ObjectDetails: &domain.ObjectDetails{ - ResourceOwner: "instance1", - }, - ID: "sessionID", - NewToken: "", + ObjectDetails: &domain.ObjectDetails{}, + ID: "sessionID", + NewToken: "", }, }, }, @@ -1251,6 +1249,52 @@ func TestCommands_TerminateSession(t *testing.T) { }, }, }, + { + "terminate session owned by org with permission", + fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + session.NewAddedEvent(context.Background(), + &session.NewAggregate("sessionID", "instance1").Aggregate, + &domain.UserAgent{ + FingerprintID: gu.Ptr("fp1"), + IP: net.ParseIP("1.2.3.4"), + Description: gu.Ptr("firefox"), + Header: http.Header{"foo": []string{"bar"}}, + }, + ), + ), + eventFromEventPusher( + session.NewUserCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "org2").Aggregate, + "userID", "", testNow), + ), + eventFromEventPusher( + session.NewTokenSetEvent(context.Background(), &session.NewAggregate("sessionID", "org2").Aggregate, + "tokenID"), + ), + ), + expectFilter( + user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, + "username", "firstname", "lastname", "nickname", "displayname", language.English, domain.GenderUnspecified, "email", false), + ), + expectPush( + session.NewTerminateEvent(authz.NewMockContext("instance1", "org1", "admin1"), &session.NewAggregate("sessionID", "instance1").Aggregate), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args{ + ctx: authz.NewMockContext("instance1", "org1", "admin1"), + sessionID: "sessionID", + sessionToken: "", + }, + res{ + want: &domain.ObjectDetails{ + ResourceOwner: "instance1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {