From 5fed7166b458a8067a655a989f1b1ba189d75723 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Mon, 23 Sep 2024 14:41:52 +0200 Subject: [PATCH 1/2] feat(alog/pp): introduce impersonating users Signed-off-by: jkoberg --- services/activitylog/pkg/service/http.go | 28 +++++------ services/activitylog/pkg/service/response.go | 28 +++++++---- .../pkg/postprocessing/postprocessing.go | 47 ++++++++++--------- .../postprocessing/pkg/service/service.go | 32 +++++++------ 4 files changed, 74 insertions(+), 61 deletions(-) diff --git a/services/activitylog/pkg/service/http.go b/services/activitylog/pkg/service/http.go index 3405d82e0f9..d888b9052a6 100644 --- a/services/activitylog/pkg/service/http.go +++ b/services/activitylog/pkg/service/http.go @@ -134,48 +134,48 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h message = MessageResourceUpdated } ts = utils.TSToTime(ev.Timestamp) - vars, err = s.GetVars(ctx, WithResource(ev.FileRef, true), WithUser(ev.ExecutingUser.GetId(), ev.ExecutingUser.GetDisplayName()), WithSpace(toSpace(ev.FileRef))) + vars, err = s.GetVars(ctx, WithResource(ev.FileRef, true), WithUser(nil, ev.ExecutingUser, ev.ImpersonatingUser), WithSpace(toSpace(ev.FileRef))) case events.FileTouched: message = MessageResourceCreated ts = utils.TSToTime(ev.Timestamp) - vars, err = s.GetVars(ctx, WithResource(ev.Ref, true), WithUser(ev.Executant, ""), WithSpace(toSpace(ev.Ref))) + vars, err = s.GetVars(ctx, WithResource(ev.Ref, true), WithUser(ev.Executant, nil, ev.ImpersonatingUser), WithSpace(toSpace(ev.Ref))) case events.ContainerCreated: message = MessageResourceCreated ts = utils.TSToTime(ev.Timestamp) - vars, err = s.GetVars(ctx, WithResource(ev.Ref, true), WithUser(ev.Executant, ""), WithSpace(toSpace(ev.Ref))) + vars, err = s.GetVars(ctx, WithResource(ev.Ref, true), WithUser(ev.Executant, nil, ev.ImpersonatingUser), WithSpace(toSpace(ev.Ref))) case events.ItemTrashed: message = MessageResourceTrashed ts = utils.TSToTime(ev.Timestamp) - vars, err = s.GetVars(ctx, WithTrashedResource(ev.Ref, ev.ID), WithUser(ev.Executant, ""), WithSpace(toSpace(ev.Ref))) + vars, err = s.GetVars(ctx, WithTrashedResource(ev.Ref, ev.ID), WithUser(ev.Executant, nil, ev.ImpersonatingUser), WithSpace(toSpace(ev.Ref))) case events.ItemMoved: switch isRename(ev.OldReference, ev.Ref) { case true: message = MessageResourceRenamed - vars, err = s.GetVars(ctx, WithResource(ev.Ref, false), WithOldResource(ev.OldReference), WithUser(ev.Executant, "")) + vars, err = s.GetVars(ctx, WithResource(ev.Ref, false), WithOldResource(ev.OldReference), WithUser(ev.Executant, nil, ev.ImpersonatingUser)) case false: message = MessageResourceMoved - vars, err = s.GetVars(ctx, WithResource(ev.Ref, true), WithUser(ev.Executant, ""), WithSpace(toSpace(ev.Ref))) + vars, err = s.GetVars(ctx, WithResource(ev.Ref, true), WithUser(ev.Executant, nil, ev.ImpersonatingUser), WithSpace(toSpace(ev.Ref))) } ts = utils.TSToTime(ev.Timestamp) case events.ShareCreated: message = MessageShareCreated ts = utils.TSToTime(ev.CTime) - vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, ""), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) + vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, nil, nil), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) case events.ShareUpdated: if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() == ev.ItemID.GetSpaceId() { continue } message = MessageShareUpdated ts = utils.TSToTime(ev.MTime) - vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, ""), WithTranslation(&t, loc, "field", ev.UpdateMask)) + vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, nil, nil), WithTranslation(&t, loc, "field", ev.UpdateMask)) case events.ShareRemoved: message = MessageShareDeleted ts = ev.Timestamp - vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, ""), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) + vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, nil, nil), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) case events.LinkCreated: message = MessageLinkCreated ts = utils.TSToTime(ev.CTime) - vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, "")) + vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, nil, nil)) case events.LinkUpdated: if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() == ev.ItemID.GetSpaceId() { continue @@ -184,21 +184,21 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h ts = utils.TSToTime(ev.MTime) vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), - WithUser(ev.Executant, ""), + WithUser(ev.Executant, nil, nil), WithTranslation(&t, loc, "field", []string{ev.FieldUpdated}), WithVar("token", ev.ItemID.GetOpaqueId(), ev.Token)) case events.LinkRemoved: message = MessageLinkDeleted ts = utils.TSToTime(ev.Timestamp) - vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, "")) + vars, err = s.GetVars(ctx, WithResource(toRef(ev.ItemID), false), WithUser(ev.Executant, nil, nil)) case events.SpaceShared: message = MessageSpaceShared ts = ev.Timestamp - vars, err = s.GetVars(ctx, WithSpace(ev.ID), WithUser(ev.Executant, ""), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) + vars, err = s.GetVars(ctx, WithSpace(ev.ID), WithUser(ev.Executant, nil, nil), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) case events.SpaceUnshared: message = MessageSpaceUnshared ts = ev.Timestamp - vars, err = s.GetVars(ctx, WithSpace(ev.ID), WithUser(ev.Executant, ""), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) + vars, err = s.GetVars(ctx, WithSpace(ev.ID), WithUser(ev.Executant, nil, nil), WithSharee(ev.GranteeUserID, ev.GranteeGroupID)) } if err != nil { diff --git a/services/activitylog/pkg/service/response.go b/services/activitylog/pkg/service/response.go index 0e790a2b920..72838bd29a7 100644 --- a/services/activitylog/pkg/service/response.go +++ b/services/activitylog/pkg/service/response.go @@ -155,23 +155,31 @@ func WithTrashedResource(ref *provider.Reference, rid *provider.ResourceId) Acti } // WithUser sets the user variable for an Activity -func WithUser(uid *user.UserId, username string) ActivityOption { - return func(_ context.Context, gwc gateway.GatewayAPIClient, vars map[string]interface{}) error { - if username == "" { - u, err := utils.GetUser(uid, gwc) +func WithUser(uid *user.UserId, u *user.User, impersonator *user.User) ActivityOption { + return func(ctx context.Context, gwc gateway.GatewayAPIClient, vars map[string]interface{}) error { + var target *user.User + switch { + case impersonator != nil: + target = impersonator + case u != nil: + target = u + case uid != nil: + us, err := utils.GetUserWithContext(ctx, uid, gwc) + target = us + if err != nil { - vars["user"] = Actor{ - ID: uid.GetOpaqueId(), + target = &user.User{ + Id: uid, DisplayName: "DeletedUser", } - return err } - username = u.GetUsername() + default: + return fmt.Errorf("no user provided") } vars["user"] = Actor{ - ID: uid.GetOpaqueId(), - DisplayName: username, + ID: target.GetId().GetOpaqueId(), + DisplayName: target.GetDisplayName(), } return nil diff --git a/services/postprocessing/pkg/postprocessing/postprocessing.go b/services/postprocessing/pkg/postprocessing/postprocessing.go index 1a27bfd5024..1ce39623575 100644 --- a/services/postprocessing/pkg/postprocessing/postprocessing.go +++ b/services/postprocessing/pkg/postprocessing/postprocessing.go @@ -12,17 +12,18 @@ import ( // Postprocessing handles postprocessing of a file type Postprocessing struct { - ID string - URL string - User *user.User - Filename string - Filesize uint64 - ResourceID *provider.ResourceId - Steps []events.Postprocessingstep - Status Status - Failures int - InitiatorID string - Finished bool + ID string + URL string + User *user.User + ImpersonatingUser *user.User + Filename string + Filesize uint64 + ResourceID *provider.ResourceId + Steps []events.Postprocessingstep + Status Status + Failures int + InitiatorID string + Finished bool config config.Postprocessing } @@ -97,13 +98,14 @@ func (pp *Postprocessing) next(current events.Postprocessingstep) interface{} { func (pp *Postprocessing) step(next events.Postprocessingstep) events.StartPostprocessingStep { pp.Status.CurrentStep = next return events.StartPostprocessingStep{ - UploadID: pp.ID, - URL: pp.URL, - ExecutingUser: pp.User, - Filename: pp.Filename, - Filesize: pp.Filesize, - ResourceID: pp.ResourceID, - StepToStart: next, + UploadID: pp.ID, + URL: pp.URL, + ExecutingUser: pp.User, + Filename: pp.Filename, + Filesize: pp.Filesize, + ResourceID: pp.ResourceID, + StepToStart: next, + ImpersonatingUser: pp.ImpersonatingUser, } } @@ -111,10 +113,11 @@ func (pp *Postprocessing) finished(outcome events.PostprocessingOutcome) events. pp.Status.CurrentStep = events.PPStepFinished pp.Status.Outcome = outcome return events.PostprocessingFinished{ - UploadID: pp.ID, - ExecutingUser: pp.User, - Filename: pp.Filename, - Outcome: outcome, + UploadID: pp.ID, + ExecutingUser: pp.User, + Filename: pp.Filename, + Outcome: outcome, + ImpersonatingUser: pp.ImpersonatingUser, } } diff --git a/services/postprocessing/pkg/service/service.go b/services/postprocessing/pkg/service/service.go index 5e1f03bbf97..13441567d9a 100644 --- a/services/postprocessing/pkg/service/service.go +++ b/services/postprocessing/pkg/service/service.go @@ -95,14 +95,15 @@ func (pps *PostprocessingService) processEvent(e events.Event) error { switch ev := e.Event.(type) { case events.BytesReceived: pp = &postprocessing.Postprocessing{ - ID: ev.UploadID, - URL: ev.URL, - User: ev.ExecutingUser, - Filename: ev.Filename, - Filesize: ev.Filesize, - ResourceID: ev.ResourceID, - Steps: pps.steps, - InitiatorID: e.InitiatorID, + ID: ev.UploadID, + URL: ev.URL, + User: ev.ExecutingUser, + Filename: ev.Filename, + Filesize: ev.Filesize, + ResourceID: ev.ResourceID, + Steps: pps.steps, + InitiatorID: e.InitiatorID, + ImpersonatingUser: ev.ImpersonatingUser, } next = pp.Init(ev) case events.PostprocessingStepFinished: @@ -124,13 +125,14 @@ func (pps *PostprocessingService) processEvent(e events.Event) error { go func() { time.Sleep(backoff) retryEvent := events.StartPostprocessingStep{ - UploadID: pp.ID, - URL: pp.URL, - ExecutingUser: pp.User, - Filename: pp.Filename, - Filesize: pp.Filesize, - ResourceID: pp.ResourceID, - StepToStart: pp.Status.CurrentStep, + UploadID: pp.ID, + URL: pp.URL, + ExecutingUser: pp.User, + Filename: pp.Filename, + Filesize: pp.Filesize, + ResourceID: pp.ResourceID, + StepToStart: pp.Status.CurrentStep, + ImpersonatingUser: pp.ImpersonatingUser, } err := events.Publish(ctx, pps.pub, retryEvent) if err != nil { From f37730a6c268a0d6315f9ca6afc9765ce0dbc25b Mon Sep 17 00:00:00 2001 From: jkoberg Date: Mon, 23 Sep 2024 16:36:33 +0200 Subject: [PATCH 2/2] fix(tests/activities): expect display name Signed-off-by: jkoberg --- .../features/apiActivities/activities.feature | 16 ++++++++-------- .../apiActivities/shareActivities.feature | 18 +++++++++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/acceptance/features/apiActivities/activities.feature b/tests/acceptance/features/apiActivities/activities.feature index d2bbeec280c..29cc288334b 100644 --- a/tests/acceptance/features/apiActivities/activities.feature +++ b/tests/acceptance/features/apiActivities/activities.feature @@ -166,7 +166,7 @@ Feature: check activities "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -326,7 +326,7 @@ Feature: check activities "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -400,7 +400,7 @@ Feature: check activities "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -518,7 +518,7 @@ Feature: check activities "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -628,7 +628,7 @@ Feature: check activities "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -744,7 +744,7 @@ Feature: check activities "pattern": "^%user_id_pattern%$" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -853,7 +853,7 @@ Feature: check activities "pattern": "^%user_id_pattern%$" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -1302,7 +1302,7 @@ Feature: check activities "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } diff --git a/tests/acceptance/features/apiActivities/shareActivities.feature b/tests/acceptance/features/apiActivities/shareActivities.feature index c1022d1b86d..7637bc9d2c8 100644 --- a/tests/acceptance/features/apiActivities/shareActivities.feature +++ b/tests/acceptance/features/apiActivities/shareActivities.feature @@ -102,7 +102,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -241,7 +241,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -346,7 +346,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -471,7 +471,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -576,7 +576,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -775,7 +775,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Brian" + "const": "Brian Murphy" } } } @@ -880,7 +880,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -986,7 +986,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } } @@ -1050,7 +1050,7 @@ Feature: check share activity "pattern": "%user_id_pattern%" }, "displayName": { - "const": "Alice" + "const": "Alice Hansen" } } }