From 404d49128a9d9ae0c348a1007ca08fef9dc647af Mon Sep 17 00:00:00 2001 From: Michael Stingl Date: Fri, 11 Sep 2026 11:24:39 +0200 Subject: [PATCH 1/4] test(notifications): observe mailbox counts over time An empty mailbox can satisfy an immediate count assertion before an asynchronous notification arrives, hiding unwanted email delivery. Add a bounded observation step using the existing Inbucket helper. Check throughout the observation period and fail when the count differs. --- .../bootstrap/NotificationContext.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/acceptance/bootstrap/NotificationContext.php b/tests/acceptance/bootstrap/NotificationContext.php index 96925d7563..b43d112e91 100644 --- a/tests/acceptance/bootstrap/NotificationContext.php +++ b/tests/acceptance/bootstrap/NotificationContext.php @@ -866,4 +866,38 @@ public function userShouldHaveEmails(string $user, string $count): void { "Expected '$expectedCount' emails for user '$user' but found '" . \count($mailBoxInfo) . "'" ); } + + /** + * Check the mailbox count for the full observation period to detect + * unexpected email that arrives asynchronously. + * + * @param string $user + * @param string $count + * @param string $seconds + * + * @return void + * @throws GuzzleException + */ + #[Then('user :user should keep :count emails for :seconds seconds')] + public function userShouldKeepEmailsForSeconds(string $user, string $count, string $seconds): void { + $duration = (int)$seconds; + Assert::assertGreaterThan(0, $duration, 'The mailbox observation duration must be positive'); + $address = $this->featureContext->getEmailAddressForUser($user); + $this->featureContext->pushEmailRecipientAsMailBox($address); + $mailBox = EmailHelper::getMailBoxFromEmail($address); + $deadline = \hrtime(true) + $duration * 1_000_000_000; + + do { + $mailBoxInfo = EmailHelper::getMailBoxInformation($mailBox, $this->featureContext->getStepLineRef()); + Assert::assertCount( + (int)$count, + $mailBoxInfo, + "Expected '$count' emails for user '$user' throughout '$seconds' seconds but found '" . \count($mailBoxInfo) . "'" + ); + $remaining = $deadline - \hrtime(true); + if ($remaining > 0) { + \usleep((int)\min(250_000, \ceil($remaining / 1_000))); + } + } while ($remaining > 0); + } } From 5fd7b4c09195186e43c0b00b75294b326290e35a Mon Sep 17 00:00:00 2001 From: Michael Stingl Date: Fri, 11 Sep 2026 11:24:39 +0200 Subject: [PATCH 2/4] fix(notifications): skip disabled email recipients Email notifications can reach disabled users because GetUser does not apply the configured LDAP filters for disabled users. Grouped emails also use the recipient address stored when the events were queued. Look up the recipient again through GetUserByClaim before delivery. This lookup applies the LDAP filters and uses the existing lookup cache. Recheck the global notification setting and use the returned email address. Skip unavailable recipients and log lookup failures without stopping delivery to other recipients. ScienceMesh invitations use the recipient email address from the event without looking up the recipient. Test the internal notification handlers and daily and weekly grouped emails with active users, unavailable recipients, and lookup failures. Related: https://github.com/opencloud-eu/opencloud/issues/3513 --- .../pkg/service/delivery_logging_test.go | 91 ++++++ .../pkg/service/delivery_test.go | 307 ++++++++++++++++++ services/notifications/pkg/service/job.go | 18 +- .../notifications/pkg/service/resource.go | 9 +- .../notifications/pkg/service/sciencemesh.go | 3 +- services/notifications/pkg/service/service.go | 74 ++++- .../notifications/pkg/service/service_test.go | 6 + services/notifications/pkg/service/shares.go | 9 +- services/notifications/pkg/service/spaces.go | 9 +- 9 files changed, 502 insertions(+), 24 deletions(-) create mode 100644 services/notifications/pkg/service/delivery_logging_test.go create mode 100644 services/notifications/pkg/service/delivery_test.go diff --git a/services/notifications/pkg/service/delivery_logging_test.go b/services/notifications/pkg/service/delivery_logging_test.go new file mode 100644 index 0000000000..ad831ca962 --- /dev/null +++ b/services/notifications/pkg/service/delivery_logging_test.go @@ -0,0 +1,91 @@ +package service + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "testing" + + user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/opencloud-eu/opencloud/pkg/log" + ehmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/eventhistory/v0" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" +) + +func TestDeliveryFailureLogsEvent(t *testing.T) { + for _, failure := range []string{"lookup", "SMTP"} { + t.Run(failure, func(t *testing.T) { + n, g, _, ch := newDeliveryFixture() + var output bytes.Buffer + n.logger = log.Logger{Logger: zerolog.New(&output)} + if failure == "lookup" { + g.lookupErr = errors.New("directory offline") + } else { + ch.err = errors.New("SMTP offline") + } + n.handleSpaceShared(events.SpaceShared{Executant: &user.UserId{OpaqueId: "alice"}, GranteeUserID: g.recipient.Id, ID: &provider.StorageSpaceId{OpaqueId: "storage!space"}}, "share-event") + require.Empty(t, ch.messages) + var entry map[string]any + require.NoError(t, json.Unmarshal(output.Bytes(), &entry)) + require.Equal(t, "error", entry["level"]) + require.NotEmpty(t, entry["error"]) + require.Equal(t, "SpaceShared", entry["event"]) + require.Equal(t, "share-event", entry["eventId"]) + require.Equal(t, "brian", entry["userId"]) + }) + } +} + +func TestGroupedEmailFailureLogsContextAndChecksStoredEvents(t *testing.T) { + for _, failure := range []string{"gateway", "authentication", "lookup", "SMTP"} { + t.Run(failure, func(t *testing.T) { + n, g, settings, ch := newDeliveryFixture() + settings.interval = "daily" + e := events.SpaceMembershipExpired{GranteeUserID: g.recipient.Id, SpaceID: &provider.StorageSpaceId{OpaqueId: "storage!space"}, SpaceName: "Project"} + body, err := json.Marshal(e) + require.NoError(t, err) + n.registeredEvents = map[string]events.Unmarshaller{"events.SpaceMembershipExpired": events.SpaceMembershipExpired{}} + n.userEventStore.historyClient = deliveryHistory{event: &ehmsg.Event{Type: "events.SpaceMembershipExpired", Event: body}} + n.handleSpaceMembershipExpired(e, "queued-event") + keys, err := n.userEventStore.listKeys("daily") + require.NoError(t, err) + require.Len(t, keys, 1) + var output bytes.Buffer + n.logger = log.Logger{Logger: zerolog.New(&output)} + switch failure { + case "gateway": + n.gatewaySelector = deliverySelector{err: errors.New("no gateway")} + case "authentication": + g.authErr = errors.New("authentication unavailable") + case "lookup": + g.lookupErr = errors.New("directory offline") + case "SMTP": + ch.err = errors.New("SMTP offline") + } + logger := n.logger.With().Str("event", "SendEmailsEvent").Str("eventId", "digest-job").Logger() + n.createGroupedMail(context.Background(), logger, keys[0]) + require.Empty(t, ch.messages) + remaining, err := n.userEventStore.listKeys("daily") + require.NoError(t, err) + if failure == "gateway" || failure == "authentication" { + require.Equal(t, keys, remaining, "gateway or authentication failure must leave events stored") + } else { + require.Empty(t, remaining, "events have already been removed when lookup or SMTP fails") + } + var entry map[string]any + require.NoError(t, json.Unmarshal(output.Bytes(), &entry)) + require.Equal(t, "error", entry["level"]) + require.NotEmpty(t, entry["error"]) + require.Equal(t, "SendEmailsEvent", entry["event"]) + require.Equal(t, "digest-job", entry["eventId"]) + require.Equal(t, keys[0], entry["key"]) + if failure == "lookup" || failure == "SMTP" { + require.Equal(t, "brian", entry["userId"]) + } + }) + } +} diff --git a/services/notifications/pkg/service/delivery_test.go b/services/notifications/pkg/service/delivery_test.go new file mode 100644 index 0000000000..20d5b97abb --- /dev/null +++ b/services/notifications/pkg/service/delivery_test.go @@ -0,0 +1,307 @@ +package service + +import ( + "context" + "encoding/json" + "errors" + "testing" + "time" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + group "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1" + user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + ocEvents "github.com/opencloud-eu/opencloud/pkg/events" + "github.com/opencloud-eu/opencloud/pkg/log" + ehmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/eventhistory/v0" + settingsmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/settings/v0" + ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" + settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" + "github.com/opencloud-eu/opencloud/services/notifications/pkg/channels" + "github.com/opencloud-eu/opencloud/services/settings/pkg/store/defaults" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/store" + "github.com/stretchr/testify/require" + "go-micro.dev/v4/client" + "google.golang.org/grpc" +) + +// These doubles replace remote RPCs and SMTP. Recipient selection, rendering, +// interval splitting and the user event store run through the production code. +type deliveryGateway struct { + gateway.GatewayAPIClient + recipient *user.User + disabled bool + lookupErr error + authErr error + response *user.GetUserByClaimResponse + members []*user.UserId +} + +func (g *deliveryGateway) GetUser(_ context.Context, req *user.GetUserRequest, _ ...grpc.CallOption) (*user.GetUserResponse, error) { + u := g.recipient + if req.GetUserId().GetOpaqueId() == "alice" { + u = &user.User{Id: req.UserId, DisplayName: "Alice", Mail: "alice@example.org"} + } + if req.GetUserId().GetOpaqueId() == "carol" { + u = &user.User{Id: req.UserId, DisplayName: "Carol", Mail: "carol@example.org"} + } + return &user.GetUserResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: u}, nil +} + +func (g *deliveryGateway) GetUserByClaim(_ context.Context, req *user.GetUserByClaimRequest, _ ...grpc.CallOption) (*user.GetUserByClaimResponse, error) { + if req.GetClaim() == "userid" && req.GetValue() == "carol" { + return &user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: &user.User{Id: &user.UserId{OpaqueId: "carol"}, Mail: "carol@example.org"}}, nil + } + if g.response != nil { + return g.response, nil + } + if g.lookupErr != nil { + return nil, g.lookupErr + } + if req.GetClaim() != "userid" || req.GetValue() != "brian" { + return nil, errors.New("unexpected recipient lookup") + } + if g.disabled { + return &user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_NOT_FOUND}}, nil + } + return &user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: g.recipient}, nil +} + +func (g *deliveryGateway) Authenticate(context.Context, *gateway.AuthenticateRequest, ...grpc.CallOption) (*gateway.AuthenticateResponse, error) { + if g.authErr != nil { + return nil, g.authErr + } + return &gateway.AuthenticateResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, Token: "service-token", User: &user.User{Id: &user.UserId{OpaqueId: "service"}}}, nil +} + +func (g *deliveryGateway) Stat(context.Context, *provider.StatRequest, ...grpc.CallOption) (*provider.StatResponse, error) { + return &provider.StatResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, Info: &provider.ResourceInfo{ + Id: &provider.ResourceId{StorageId: "storage", SpaceId: "space", OpaqueId: "file"}, Name: "file.txt", Space: &provider.StorageSpace{Name: "Project"}, + }}, nil +} + +func (g *deliveryGateway) GetGroup(context.Context, *group.GetGroupRequest, ...grpc.CallOption) (*group.GetGroupResponse, error) { + if g.members != nil { + return &group.GetGroupResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, Group: &group.Group{Members: g.members}}, nil + } + return &group.GetGroupResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, Group: &group.Group{Members: []*user.UserId{{OpaqueId: "brian"}}}}, nil +} + +type deliverySelector struct { + client gateway.GatewayAPIClient + err error +} + +func (s deliverySelector) Next(...pool.Option) (gateway.GatewayAPIClient, error) { + return s.client, s.err +} + +type deliverySettings struct { + settingssvc.ValueService + interval string + optOut bool +} + +func (s *deliverySettings) GetValueByUniqueIdentifiers(_ context.Context, req *settingssvc.GetValueByUniqueIdentifiersRequest, _ ...client.CallOption) (*settingssvc.GetValueResponse, error) { + v := &settingsmsg.Value{} + switch req.GetSettingId() { + case defaults.SettingUUIDProfileEmailSendingInterval: + v.Value = &settingsmsg.Value_StringValue{StringValue: s.interval} + case defaults.SettingUUIDProfileDisableNotifications: + v.Value = &settingsmsg.Value_BoolValue{BoolValue: s.optOut} + default: + v.Value = &settingsmsg.Value_CollectionValue{CollectionValue: &settingsmsg.CollectionValue{Values: []*settingsmsg.CollectionOption{{Key: "mail", Option: &settingsmsg.CollectionOption_BoolValue{BoolValue: true}}}}} + } + return &settingssvc.GetValueResponse{Value: &settingsmsg.ValueWithIdentifier{Value: v}}, nil +} + +type deliveryChannel struct { + messages []*channels.Message + err error +} + +func (c *deliveryChannel) SendMessage(_ context.Context, m *channels.Message) error { + if c.err != nil { + return c.err + } + c.messages = append(c.messages, m) + return nil +} + +type deliveryHistory struct { + ehsvc.EventHistoryService + event *ehmsg.Event +} + +func (h deliveryHistory) GetEvents(context.Context, *ehsvc.GetEventsRequest, ...client.CallOption) (*ehsvc.GetEventsResponse, error) { + return &ehsvc.GetEventsResponse{Events: []*ehmsg.Event{h.event}}, nil +} + +func newDeliveryFixture() (eventsNotifier, *deliveryGateway, *deliverySettings, *deliveryChannel) { + g := &deliveryGateway{recipient: &user.User{Id: &user.UserId{OpaqueId: "brian"}, DisplayName: "Brian", Mail: "brian@example.org"}} + v := &deliverySettings{interval: "instant"} + c := &deliveryChannel{} + l := log.NewLogger() + n := eventsNotifier{logger: l, channel: c, gatewaySelector: deliverySelector{client: g}, valueService: v, openCloudURL: "https://cloud.example.org", + filter: newNotificationFilter(l, v), splitter: newIntervalSplitter(l, v), userEventStore: newUserEventStore(l, store.Create(), nil)} + return n, g, v, c +} + +func TestDeliverySkipsDisabledUsersAcrossHandlers(t *testing.T) { + alice, brian := &user.UserId{OpaqueId: "alice"}, &user.UserId{OpaqueId: "brian"} + space := &provider.StorageSpaceId{OpaqueId: "storage!space"} + file := &provider.ResourceId{StorageId: "storage", SpaceId: "space", OpaqueId: "file"} + handlers := map[string]func(eventsNotifier){ + "SpaceShared": func(n eventsNotifier) { + n.handleSpaceShared(events.SpaceShared{Executant: alice, GranteeUserID: brian, ID: space}, "event") + }, + "SpaceShared/group": func(n eventsNotifier) { + n.handleSpaceShared(events.SpaceShared{Executant: alice, GranteeGroupID: &group.GroupId{OpaqueId: "group"}, ID: space}, "event") + }, + "SpaceUnshared": func(n eventsNotifier) { + n.handleSpaceUnshared(events.SpaceUnshared{Executant: alice, GranteeUserID: brian, ID: space}, "event") + }, + "SpaceMembershipExpired": func(n eventsNotifier) { + n.handleSpaceMembershipExpired(events.SpaceMembershipExpired{GranteeUserID: brian, SpaceID: space}, "event") + }, + "ShareCreated": func(n eventsNotifier) { + n.handleShareCreated(events.ShareCreated{Sharer: alice, GranteeUserID: brian, ItemID: file}, "event") + }, + "ShareExpired": func(n eventsNotifier) { + n.handleShareExpired(events.ShareExpired{ShareOwner: alice, GranteeUserID: brian, ItemID: file}, "event") + }, + "ShareRemoved": func(n eventsNotifier) { + n.handleShareRemoved(events.ShareRemoved{Executant: alice, GranteeUserID: brian, ItemID: file}, "event") + }, + "ResourceMention": func(n eventsNotifier) { + n.handleResourceMention(ocEvents.ResourceMention{Executant: alice, UserIDs: []*user.UserId{brian}, Ref: &provider.Reference{ResourceId: file}}, "event") + }, + } + for name, handle := range handlers { + t.Run(name, func(t *testing.T) { + for _, disabled := range []bool{false, true} { + n, g, _, ch := newDeliveryFixture() + g.disabled = disabled + handle(n) + if disabled { + require.Empty(t, ch.messages, "disabled user must not receive email") + } else { + require.Len(t, ch.messages, 1, "active user must receive email") + require.Equal(t, []string{"brian@example.org"}, ch.messages[0].Recipient) + } + } + }) + } +} + +func TestGroupedEmailRechecksRecipientAfterQueueing(t *testing.T) { + for _, interval := range []string{"daily", "weekly"} { + for _, disabled := range []bool{false, true} { + t.Run(interval+"/"+map[bool]string{false: "active", true: "disabled"}[disabled], func(t *testing.T) { + n, g, settings, ch := newDeliveryFixture() + settings.interval = interval + e := events.SpaceMembershipExpired{GranteeUserID: g.recipient.Id, SpaceID: &provider.StorageSpaceId{OpaqueId: "storage!space"}, SpaceName: "Project", ExpiredAt: time.Now()} + body, err := json.Marshal(e) + require.NoError(t, err) + n.registeredEvents = map[string]events.Unmarshaller{"events.SpaceMembershipExpired": events.SpaceMembershipExpired{}} + n.userEventStore.historyClient = deliveryHistory{event: &ehmsg.Event{Type: "events.SpaceMembershipExpired", Event: body}} + n.handleSpaceMembershipExpired(e, "event") + require.Empty(t, ch.messages, "grouped email must wait for its job") + keys, err := n.userEventStore.listKeys(interval) + require.NoError(t, err) + require.Len(t, keys, 1) + g.disabled = disabled + g.recipient = &user.User{Id: g.recipient.Id, Mail: "new-address@example.org", DisplayName: "Brian"} + n.createGroupedMail(context.Background(), n.logger.Logger, keys[0]) + if disabled { + require.Empty(t, ch.messages) + } else { + require.Len(t, ch.messages, 1) + require.Equal(t, []string{"new-address@example.org"}, ch.messages[0].Recipient, "email must use the address returned by the recipient lookup") + } + }) + } + } +} + +func TestScienceMeshInvitationDoesNotLookUpRecipient(t *testing.T) { + n, g, _, ch := newDeliveryFixture() + g.lookupErr = errors.New("recipient lookup must not be called") + n.handleScienceMeshInviteTokenGenerated(events.ScienceMeshInviteTokenGenerated{Sharer: &user.UserId{OpaqueId: "alice"}, RecipientMail: "external@example.net", Token: "invitation"}) + require.Len(t, ch.messages, 1) + require.Equal(t, []string{"external@example.net"}, ch.messages[0].Recipient) +} + +func TestDeliveryKeepsActiveGroupMember(t *testing.T) { + for _, failedLookup := range []bool{false, true} { + n, g, _, ch := newDeliveryFixture() + g.disabled = true + if failedLookup { + g.lookupErr = errors.New("brian lookup failed") + } + g.members = []*user.UserId{{OpaqueId: "brian"}, {OpaqueId: "carol"}} + n.handleSpaceShared(events.SpaceShared{Executant: &user.UserId{OpaqueId: "alice"}, GranteeGroupID: &group.GroupId{OpaqueId: "group"}, ID: &provider.StorageSpaceId{OpaqueId: "storage!space"}}, "event") + require.Len(t, ch.messages, 1) + require.Equal(t, []string{"carol@example.org"}, ch.messages[0].Recipient) + } +} + +func TestDeliveryRejectsUnavailableOrInvalidRecipient(t *testing.T) { + for _, test := range []struct { + name string + change func(*deliveryGateway, *deliverySettings) + }{ + {"global opt out", func(_ *deliveryGateway, s *deliverySettings) { s.optOut = true }}, + {"empty address", func(g *deliveryGateway, _ *deliverySettings) { g.recipient.Mail = " " }}, + {"blocked status", func(g *deliveryGateway, _ *deliverySettings) { + g.recipient.Status = user.UserStatus_USER_STATUS_BLOCKED + }}, + {"transport error", func(g *deliveryGateway, _ *deliverySettings) { g.lookupErr = errors.New("offline") }}, + {"missing user", func(g *deliveryGateway, _ *deliverySettings) { + g.response = &user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}} + }}, + {"missing status", func(g *deliveryGateway, _ *deliverySettings) { + g.response = &user.GetUserByClaimResponse{User: g.recipient} + }}, + {"provider error", func(g *deliveryGateway, _ *deliverySettings) { + g.response = &user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_INTERNAL}} + }}, + {"wrong user", func(g *deliveryGateway, _ *deliverySettings) { g.recipient.Id = &user.UserId{OpaqueId: "other"} }}, + } { + t.Run(test.name, func(t *testing.T) { + n, g, s, ch := newDeliveryFixture() + test.change(g, s) + n.send(context.Background(), n.logger.Logger, []recipientMessage{{recipient: &user.UserId{OpaqueId: "brian"}, message: &channels.Message{Recipient: []string{"stale@example.org"}}}}) + require.Empty(t, ch.messages) + }) + } +} + +func TestDeliveryChecksRecipientIdentity(t *testing.T) { + for _, test := range []struct { + name string + requested *user.UserId + resolved *user.UserId + wantMail bool + }{ + {"matching full identity", &user.UserId{OpaqueId: "brian", Idp: "idp", TenantId: "tenant"}, &user.UserId{OpaqueId: "brian", Idp: "idp", TenantId: "tenant"}, true}, + {"wrong IDP", &user.UserId{OpaqueId: "brian", Idp: "idp", TenantId: "tenant"}, &user.UserId{OpaqueId: "brian", Idp: "other", TenantId: "tenant"}, false}, + {"wrong tenant", &user.UserId{OpaqueId: "brian", Idp: "idp", TenantId: "tenant"}, &user.UserId{OpaqueId: "brian", Idp: "idp", TenantId: "other"}, false}, + {"unspecified IDP and tenant", &user.UserId{OpaqueId: "brian"}, &user.UserId{OpaqueId: "brian", Idp: "idp", TenantId: "tenant"}, true}, + } { + t.Run(test.name, func(t *testing.T) { + n, g, _, ch := newDeliveryFixture() + g.recipient.Id = test.resolved + n.send(context.Background(), n.logger.Logger, []recipientMessage{{recipient: test.requested, message: &channels.Message{}}}) + if test.wantMail { + require.Len(t, ch.messages, 1) + require.Equal(t, []string{"brian@example.org"}, ch.messages[0].Recipient) + } else { + require.Empty(t, ch.messages) + } + }) + } +} diff --git a/services/notifications/pkg/service/job.go b/services/notifications/pkg/service/job.go index 96884865e6..1c47845198 100644 --- a/services/notifications/pkg/service/job.go +++ b/services/notifications/pkg/service/job.go @@ -5,9 +5,9 @@ import ( "github.com/opencloud-eu/opencloud/pkg/l10n" ehmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/eventhistory/v0" - "github.com/opencloud-eu/opencloud/services/notifications/pkg/channels" "github.com/opencloud-eu/opencloud/services/notifications/pkg/email" "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/utils" "github.com/rs/zerolog" ) @@ -44,9 +44,20 @@ func (s eventsNotifier) sendGroupedEmailsJob(sendEmailsEvent events.SendEmailsEv } func (s eventsNotifier) createGroupedMail(ctx context.Context, logger zerolog.Logger, key string) { + logger = logger.With().Str("key", key).Logger() + gw, err := s.gatewaySelector.Next() + if err != nil { + logger.Error().Err(err).Msg("could not select gateway for grouped email") + return + } + ctx, err = utils.GetServiceUserContextWithContext(ctx, gw, s.serviceAccountID, s.serviceAccountSecret) + if err != nil { + logger.Error().Err(err).Msg("could not authenticate grouped email job") + return + } userEvents, err := s.userEventStore.pop(ctx, key) if err != nil { - logger.Error().Err(err).Str("key", key).Msg("could not pop user events") + logger.Error().Err(err).Msg("could not pop user events") return } @@ -160,8 +171,7 @@ func (s eventsNotifier) createGroupedMail(ctx context.Context, logger zerolog.Lo return } rendered.Sender = s.defaultEmailSender - rendered.Recipient = []string{userEvents.User.GetMail()} - s.send(ctx, []*channels.Message{rendered}) + s.send(ctx, logger, []recipientMessage{{message: rendered, recipient: userEvents.User.GetId()}}) } func (s eventsNotifier) unwrapEvent(logger zerolog.Logger, e *ehmsg.Event) any { diff --git a/services/notifications/pkg/service/resource.go b/services/notifications/pkg/service/resource.go index 1d81bf8814..81fda9bab7 100644 --- a/services/notifications/pkg/service/resource.go +++ b/services/notifications/pkg/service/resource.go @@ -9,7 +9,6 @@ import ( ocEvents "github.com/opencloud-eu/opencloud/pkg/events" "github.com/opencloud-eu/opencloud/pkg/l10n" - "github.com/opencloud-eu/opencloud/services/notifications/pkg/channels" "github.com/opencloud-eu/opencloud/services/notifications/pkg/email" "github.com/opencloud-eu/opencloud/services/settings/pkg/store/defaults" ) @@ -17,6 +16,7 @@ import ( func (s eventsNotifier) handleResourceMention(e ocEvents.ResourceMention, eventId string) { logger := s.logger.With(). Str("event", "Mention"). + Str("eventId", eventId). Str("resourceid", e.Ref.GetResourceId().GetOpaqueId()). Logger() gatewayClient, err := s.gatewaySelector.Next() @@ -82,7 +82,7 @@ func (s eventsNotifier) handleResourceMention(e ocEvents.ResourceMention, eventI return } - messages := make([]*channels.Message, len(data.recipients)) + messages := make([]recipientMessage, len(data.recipients)) for i, recipient := range data.recipients { locale := l10n.MustGetUserLocale(ctx, recipient.GetId().GetOpaqueId(), "", s.valueService) message, err := email.RenderEmailTemplate(email.Mention, locale, s.defaultLanguage, s.emailTemplatePath, s.translationPath, map[string]string{ @@ -97,9 +97,8 @@ func (s eventsNotifier) handleResourceMention(e ocEvents.ResourceMention, eventI } message.Sender = data.author.GetDisplayName() - message.Recipient = []string{recipient.GetMail()} - messages[i] = message + messages[i] = recipientMessage{message: message, recipient: recipient.GetId()} } - s.send(ctx, messages) + s.send(ctx, logger, messages) } diff --git a/services/notifications/pkg/service/sciencemesh.go b/services/notifications/pkg/service/sciencemesh.go index f198648323..fe9c5c5520 100644 --- a/services/notifications/pkg/service/sciencemesh.go +++ b/services/notifications/pkg/service/sciencemesh.go @@ -3,7 +3,6 @@ package service import ( "context" - "github.com/opencloud-eu/opencloud/services/notifications/pkg/channels" "github.com/opencloud-eu/opencloud/services/notifications/pkg/email" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/utils" @@ -78,5 +77,5 @@ func (s eventsNotifier) handleScienceMeshInviteTokenGenerated(e events.ScienceMe msg.Sender = owner.GetDisplayName() msg.Recipient = []string{e.RecipientMail} - s.send(ctx, []*channels.Message{msg}) + s.sendMessage(ctx, logger, msg) } diff --git a/services/notifications/pkg/service/service.go b/services/notifications/pkg/service/service.go index 74b2e491e8..21fb155c84 100644 --- a/services/notifications/pkg/service/service.go +++ b/services/notifications/pkg/service/service.go @@ -26,6 +26,7 @@ import ( "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/rs/zerolog" "github.com/opencloud-eu/opencloud/pkg/l10n" "github.com/opencloud-eu/opencloud/pkg/log" @@ -160,10 +161,17 @@ func (s eventsNotifier) Close() { } } +// recipientMessage keeps the recipient's user ID with the rendered message. +// send resolves the email address before passing the message to the channel. +type recipientMessage struct { + message *channels.Message + recipient *user.UserId +} + func (s eventsNotifier) render(ctx context.Context, template email.MessageTemplate, - granteeFieldName string, fields map[string]string, granteeList []*user.User, sender string) ([]*channels.Message, error) { + granteeFieldName string, fields map[string]string, granteeList []*user.User, sender string) ([]recipientMessage, error) { // Render the Email Template for each user - messageList := make([]*channels.Message, len(granteeList)) + messageList := make([]recipientMessage, len(granteeList)) for i, usr := range granteeList { locale := l10n.MustGetUserLocale(ctx, usr.GetId().GetOpaqueId(), "", s.valueService) fields[granteeFieldName] = usr.GetDisplayName() @@ -173,18 +181,70 @@ func (s eventsNotifier) render(ctx context.Context, template email.MessageTempla return nil, err } rendered.Sender = sender - rendered.Recipient = []string{usr.GetMail()} - messageList[i] = rendered + messageList[i] = recipientMessage{message: rendered, recipient: usr.GetId()} } return messageList, nil } -func (s eventsNotifier) send(ctx context.Context, emails []*channels.Message) { +func (s eventsNotifier) send(ctx context.Context, logger zerolog.Logger, emails []recipientMessage) { for _, r := range emails { - err := s.channel.SendMessage(ctx, r) + logger := logger.With().Str("userId", r.recipient.GetOpaqueId()).Logger() + usr, err := s.getDeliveryRecipient(ctx, r.recipient) + if errors.Is(err, errDeliveryRecipientUnavailable) { + continue + } if err != nil { - s.logger.Error().Err(err).Str("event", "SendEmail").Msg("failed to send a message") + logger.Error().Err(err).Msg("could not resolve notification recipient") + continue } + r.message.Recipient = []string{usr.GetMail()} + s.sendMessage(ctx, logger, r.message) + } +} + +var errDeliveryRecipientUnavailable = errors.New("notification recipient unavailable") + +func (s eventsNotifier) getDeliveryRecipient(ctx context.Context, id *user.UserId) (*user.User, error) { + if id.GetOpaqueId() == "" { + return nil, errors.New("notification recipient ID is missing") + } + gw, err := s.gatewaySelector.Next() + if err != nil { + return nil, err + } + // Unlike GetUser, GetUserByClaim applies the configured LDAP filter for + // disabled users. The lookup can return an earlier user state until its + // cache entry expires. + r, err := gw.GetUserByClaim(ctx, &user.GetUserByClaimRequest{Claim: "userid", Value: id.GetOpaqueId(), SkipFetchingUserGroups: true}) + if err != nil { + return nil, err + } + if r.GetStatus() == nil { + return nil, errors.New("recipient lookup returned no status") + } + if r.GetStatus().GetCode() == rpc.Code_CODE_NOT_FOUND { + return nil, errDeliveryRecipientUnavailable + } + if r.GetStatus().GetCode() != rpc.Code_CODE_OK { + return nil, fmt.Errorf("recipient lookup failed: %d: %s", r.GetStatus().GetCode(), r.GetStatus().GetMessage()) + } + u := r.GetUser() + if u.GetId().GetOpaqueId() != id.GetOpaqueId() || + (id.GetIdp() != "" && u.GetId().GetIdp() != id.GetIdp()) || + (id.GetTenantId() != "" && u.GetId().GetTenantId() != id.GetTenantId()) { + return nil, errors.New("recipient lookup returned a missing or mismatched identity") + } + if u.GetStatus() == user.UserStatus_USER_STATUS_BLOCKED || strings.TrimSpace(u.GetMail()) == "" || s.disableEmails(ctx, id) { + return nil, errDeliveryRecipientUnavailable + } + return u, nil +} + +// sendMessage sends a rendered message without looking up the recipient. +// Callers must check the recipient first, except for ScienceMesh invitations. +func (s eventsNotifier) sendMessage(ctx context.Context, logger zerolog.Logger, message *channels.Message) { + if err := s.channel.SendMessage(ctx, message); err != nil { + logger.Error().Err(err).Msg("failed to send a message") } } diff --git a/services/notifications/pkg/service/service_test.go b/services/notifications/pkg/service/service_test.go index 0605b9ba76..11cec6828b 100644 --- a/services/notifications/pkg/service/service_test.go +++ b/services/notifications/pkg/service/service_test.go @@ -71,6 +71,9 @@ var _ = Describe("Notifications", func() { gatewayClient.On("GetUser", mock.Anything, mock.MatchedBy(func(req *user.GetUserRequest) bool { return req.GetUserId().GetOpaqueId() == sharee.GetId().GetOpaqueId() })).Return(&user.GetUserResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: sharee}, nil) + gatewayClient.On("GetUserByClaim", mock.Anything, mock.MatchedBy(func(req *user.GetUserByClaimRequest) bool { + return req.GetClaim() == "userid" && req.GetValue() == sharee.GetId().GetOpaqueId() + })).Return(&user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: sharee}, nil) gatewayClient.On("Authenticate", mock.Anything, mock.Anything).Return(&gateway.AuthenticateResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: sharer}, nil) gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, Info: &provider.ResourceInfo{Name: "secrets of the board", Space: &provider.StorageSpace{Name: "secret space"}}}, nil) vs = &settingsmocks.ValueService{} @@ -307,6 +310,9 @@ var _ = Describe("Notifications X-Site Scripting", func() { gatewayClient.On("GetUser", mock.Anything, mock.MatchedBy(func(req *user.GetUserRequest) bool { return req.GetUserId().GetOpaqueId() == sharee.GetId().GetOpaqueId() })).Return(&user.GetUserResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: sharee}, nil) + gatewayClient.On("GetUserByClaim", mock.Anything, mock.MatchedBy(func(req *user.GetUserByClaimRequest) bool { + return req.GetClaim() == "userid" && req.GetValue() == sharee.GetId().GetOpaqueId() + })).Return(&user.GetUserByClaimResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: sharee}, nil) gatewayClient.On("Authenticate", mock.Anything, mock.Anything).Return(&gateway.AuthenticateResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}, User: sharer}, nil) gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ Status: &rpc.Status{Code: rpc.Code_CODE_OK}, diff --git a/services/notifications/pkg/service/shares.go b/services/notifications/pkg/service/shares.go index aa905b630a..337437718e 100644 --- a/services/notifications/pkg/service/shares.go +++ b/services/notifications/pkg/service/shares.go @@ -15,6 +15,7 @@ import ( func (s eventsNotifier) handleShareCreated(e events.ShareCreated, eventId string) { logger := s.logger.With(). Str("event", "ShareCreated"). + Str("eventId", eventId). Str("itemid", e.ItemID.OpaqueId). Logger() @@ -46,7 +47,7 @@ func (s eventsNotifier) handleShareCreated(e events.ShareCreated, eventId string logger.Error().Err(err).Msg("could not get render the email") return } - s.send(ctx, emails) + s.send(ctx, logger, emails) } func (s eventsNotifier) prepareShareCreated(logger zerolog.Logger, e events.ShareCreated) (owner *user.User, shareFolder, shareLink string, ctx context.Context, err error) { @@ -93,6 +94,7 @@ func (s eventsNotifier) prepareShareCreated(logger zerolog.Logger, e events.Shar func (s eventsNotifier) handleShareExpired(e events.ShareExpired, eventId string) { logger := s.logger.With(). Str("event", "ShareExpired"). + Str("eventId", eventId). Str("itemid", e.ItemID.GetOpaqueId()). Logger() @@ -134,7 +136,7 @@ func (s eventsNotifier) handleShareExpired(e events.ShareExpired, eventId string logger.Error().Err(err).Msg("could not get render the email") return } - s.send(ctx, emails) + s.send(ctx, logger, emails) } func (s eventsNotifier) prepareShareExpired(logger zerolog.Logger, e events.ShareExpired) (shareFolder string, ctx context.Context, err error) { @@ -165,6 +167,7 @@ func (s eventsNotifier) prepareShareExpired(logger zerolog.Logger, e events.Shar func (s eventsNotifier) handleShareRemoved(e events.ShareRemoved, eventId string) { logger := s.logger.With(). Str("event", "ShareRemoved"). + Str("eventId", eventId). Str("itemid", e.ItemID.OpaqueId). Logger() @@ -196,7 +199,7 @@ func (s eventsNotifier) handleShareRemoved(e events.ShareRemoved, eventId string logger.Error().Err(err).Msg("could not get render the email") return } - s.send(ctx, emails) + s.send(ctx, logger, emails) } func (s eventsNotifier) prepareShareRemoved(logger zerolog.Logger, e events.ShareRemoved) (executant *user.User, shareFolder string, ctx context.Context, err error) { diff --git a/services/notifications/pkg/service/spaces.go b/services/notifications/pkg/service/spaces.go index 1cb30782ee..755677b0b5 100644 --- a/services/notifications/pkg/service/spaces.go +++ b/services/notifications/pkg/service/spaces.go @@ -15,6 +15,7 @@ import ( func (s eventsNotifier) handleSpaceShared(e events.SpaceShared, eventId string) { logger := s.logger.With(). Str("event", "SpaceShared"). + Str("eventId", eventId). Str("itemid", e.ID.OpaqueId). Logger() executant, spaceName, shareLink, ctx, err := s.prepareSpaceShared(logger, e) @@ -45,7 +46,7 @@ func (s eventsNotifier) handleSpaceShared(e events.SpaceShared, eventId string) logger.Error().Err(err).Msg("could not get render the email") return } - s.send(ctx, emails) + s.send(ctx, logger, emails) } func (s eventsNotifier) prepareSpaceShared(logger zerolog.Logger, e events.SpaceShared) (executant *user.User, spaceName, shareLink string, ctx context.Context, err error) { @@ -99,6 +100,7 @@ func (s eventsNotifier) prepareSpaceShared(logger zerolog.Logger, e events.Space func (s eventsNotifier) handleSpaceUnshared(e events.SpaceUnshared, eventId string) { logger := s.logger.With(). Str("event", "SpaceUnshared"). + Str("eventId", eventId). Str("itemid", e.ID.OpaqueId). Logger() @@ -130,7 +132,7 @@ func (s eventsNotifier) handleSpaceUnshared(e events.SpaceUnshared, eventId stri logger.Error().Err(err).Msg("Could not get render the email") return } - s.send(ctx, emails) + s.send(ctx, logger, emails) } func (s eventsNotifier) prepareSpaceUnshared(logger zerolog.Logger, e events.SpaceUnshared) (executant *user.User, spaceName, shareLink string, ctx context.Context, err error) { @@ -184,6 +186,7 @@ func (s eventsNotifier) prepareSpaceUnshared(logger zerolog.Logger, e events.Spa func (s eventsNotifier) handleSpaceMembershipExpired(e events.SpaceMembershipExpired, eventId string) { logger := s.logger.With(). Str("event", "SpaceMembershipExpired"). + Str("eventId", eventId). Str("itemid", e.SpaceID.GetOpaqueId()). Logger() @@ -213,7 +216,7 @@ func (s eventsNotifier) handleSpaceMembershipExpired(e events.SpaceMembershipExp logger.Error().Err(err).Msg("could not get render the email") return } - s.send(ctx, emails) + s.send(ctx, logger, emails) } func (s eventsNotifier) prepareSpaceMembershipExpired(logger zerolog.Logger, e events.SpaceMembershipExpired) (spaceName string, ctx context.Context, err error) { From 422c43e5e61be7bd00c14bf007b9605e4266d4d3 Mon Sep 17 00:00:00 2001 From: Michael Stingl Date: Fri, 11 Sep 2026 12:56:22 +0200 Subject: [PATCH 3/4] test(notifications): remove stale decomposed mail failures The earlier notification fixes removed these expected failures from the POSIX list but left the two email cases in the decomposed list. Both scenarios pass on the unpatched source base, so the acceptance runner rejects them as unexpected successes. Match the decomposed email expectations to POSIX. Keep the separate in-app notification entry, which the email tests do not cover. Related: https://github.com/opencloud-eu/opencloud/pull/3257 --- tests/acceptance/expected-failures-decomposed-storage.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/acceptance/expected-failures-decomposed-storage.md b/tests/acceptance/expected-failures-decomposed-storage.md index 09e35ea1fb..51cfa108fd 100644 --- a/tests/acceptance/expected-failures-decomposed-storage.md +++ b/tests/acceptance/expected-failures-decomposed-storage.md @@ -327,8 +327,6 @@ _ocdav: api compatibility, return correct status code_ ### notification issue #323 -- [apiNotification/emailNotification.feature:280](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/apiNotification/emailNotification.feature#L280) -- [apiNotification/emailNotification.feature:298](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/apiNotification/emailNotification.feature#L298) - [apiNotification/spaceNotification.feature:463](https://github.com/opencloud-eu/opencloud/blob/main/tests/acceptance/features/apiNotification/spaceNotification.feature#L463) ### Won't fix From fffa68b9f7022bc63b610360958aea26ef747295 Mon Sep 17 00:00:00 2001 From: Michael Stingl Date: Fri, 11 Sep 2026 11:54:07 +0200 Subject: [PATCH 4/4] test(notifications): cover disabled email delivery paths The existing email scenarios do not check delivery after a user is disabled. An immediate empty-mailbox assertion can also miss email that arrives asynchronously. Extend the existing email feature to cover direct and group shares with cold and expired lookup caches, daily and weekly grouped emails, and file-share removal. Check that active users still receive email and observe disabled recipients' mailbox counts for a bounded period. Use a Graph mention step to check active delivery and the existing API rejection of disabled recipients. The unit tests separately check that events are queued before the recipient is disabled. Related: https://github.com/opencloud-eu/opencloud/issues/3513 --- .../bootstrap/NotificationContext.php | 35 +++ .../apiNotification/emailNotification.feature | 220 ++++++++++++++++++ 2 files changed, 255 insertions(+) diff --git a/tests/acceptance/bootstrap/NotificationContext.php b/tests/acceptance/bootstrap/NotificationContext.php index b43d112e91..65ad8ea764 100644 --- a/tests/acceptance/bootstrap/NotificationContext.php +++ b/tests/acceptance/bootstrap/NotificationContext.php @@ -14,6 +14,7 @@ use TestHelpers\EmailHelper; use TestHelpers\OcsApiHelper; use TestHelpers\GraphHelper; +use TestHelpers\HttpRequestHelper; use TestHelpers\SettingsHelper; use TestHelpers\BehatHelper; use Behat\Step\Given; @@ -867,6 +868,40 @@ public function userShouldHaveEmails(string $user, string $count): void { ); } + /** + * @param string $author + * @param string $recipient + * @param string $file + * @param string $space + * + * @return void + * @throws GuzzleException + */ + #[When('user :author mentions user :recipient on file :file in space :space using the Graph API')] + public function userMentionsUserOnFile(string $author, string $recipient, string $file, string $space): void { + $author = $this->featureContext->getActualUsername($author); + $recipientId = $this->featureContext->getAttributeOfCreatedUser($recipient, 'id'); + $url = GraphHelper::getFullUrl( + $this->featureContext->getBaseUrl(), + 'users/' . \rawurlencode($recipientId) . '/teamwork/sendActivityNotification' + ); + $body = [ + 'topic' => ['source' => 'text', 'value' => $this->spacesContext->getFileId($author, $space, $file)], + 'activityType' => 'mentioned', + 'teamsAppId' => '8d1c9c88-9e2c-4d0b-9a1e-6a9de1cb9d3c', + ]; + $response = HttpRequestHelper::sendRequest( + $url, + $this->featureContext->getStepLineRef(), + 'POST', + $author, + $this->featureContext->getPasswordForUser($author), + ['Content-Type' => 'application/json'], + \json_encode($body) + ); + $this->featureContext->setResponse($response); + } + /** * Check the mailbox count for the full observation period to detect * unexpected email that arrives asynchronously. diff --git a/tests/acceptance/features/apiNotification/emailNotification.feature b/tests/acceptance/features/apiNotification/emailNotification.feature index 56e77944f7..6941c13861 100644 --- a/tests/acceptance/features/apiNotification/emailNotification.feature +++ b/tests/acceptance/features/apiNotification/emailNotification.feature @@ -29,6 +29,113 @@ Feature: Email notification Click here to view it: %base_url%/f/%space_id% """ + @issue-3513 + Scenario: disabled user does not get an email notification when someone shares a project space + Given the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API + And user "Alice" has created a space "new-space" with the default quota using the Graph API + And user "Carol" has been created with default attributes + And user "Alice" sends the following space share invitation using root endpoint of the Graph API: + | space | new-space | + | sharee | Carol | + | shareType | user | + | permissionsRole | Space Viewer | + And user "Carol" should have received the following email from user "Alice" about the share of project space "new-space" + """ + Hello Carol King, + + %displayname% has invited you to join "new-space". + + Click here to view it: %base_url%/f/%space_id% + """ + And the user "Admin" has disabled user "Brian" + When user "Alice" sends the following space share invitation using root endpoint of the Graph API: + | space | new-space | + | sharee | Brian | + | shareType | user | + | permissionsRole | Space Editor | + Then the HTTP status code should be "200" + And user "Brian" should keep "0" emails for "5" seconds + + @issue-3513 + Scenario: disabled group members do not get an email notification when someone shares a project space with the group + Given the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API + And user "Alice" has created a space "new-space" with the default quota using the Graph API + And user "Carol" has been created with default attributes + And group "group1" has been created + And user "Brian" has been added to group "group1" + And user "Carol" has been added to group "group1" + And the user "Admin" has disabled user "Brian" + When user "Alice" sends the following space share invitation using root endpoint of the Graph API: + | space | new-space | + | sharee | group1 | + | shareType | group | + | permissionsRole | Space Viewer | + Then the HTTP status code should be "200" + And user "Carol" should have received the following email from user "Alice" about the share of project space "new-space" + """ + Hello Carol King, + + %displayname% has invited you to join "new-space". + + Click here to view it: %base_url%/f/%space_id% + """ + And user "Brian" should keep "0" emails for "5" seconds + + @issue-3513 + Scenario Outline: a previously notified user does not receive another share email after being disabled and the LDAP lookup cache expires + Given these users have been created with default attributes: + | username | displayname | email | + | | Alice Hansen | @example.org | + | | Brian Murphy | @example.org | + | | Carol King | @example.org | + And the administrator has assigned the role "Space Admin" to user "" using the Graph API + And user "" has created a space "warm-up-space" with the default quota using the Graph API + And user "" has created a space "new-space" with the default quota using the Graph API + And group "" has been created + And user "" has been added to group "" + And user "" sends the following space share invitation using root endpoint of the Graph API: + | space | warm-up-space | + | sharee | | + | shareType | user | + | permissionsRole | Space Viewer | + And user "" should have received the following email from user "" about the share of project space "warm-up-space" + """ + Hello Brian Murphy, + + %displayname% has invited you to join "warm-up-space". + + Click here to view it: %base_url%/f/%space_id% + """ + And user "" should have "1" emails + And the user "Admin" has disabled user "" + And the user waits for "12" seconds + When user "" sends the following space share invitation using root endpoint of the Graph API: + | space | new-space | + | sharee | | + | shareType | | + | permissionsRole | Space Viewer | + Then the HTTP status code should be "200" + When user "" sends the following space share invitation using root endpoint of the Graph API: + | space | new-space | + | sharee | | + | shareType | user | + | permissionsRole | Space Viewer | + Then the HTTP status code should be "200" + And user "" should have received the following email from user "" about the share of project space "new-space" + """ + Hello Carol King, + + %displayname% has invited you to join "new-space". + + Click here to view it: %base_url%/f/%space_id% + """ + And user "" should keep "1" emails for "5" seconds + + Examples: + | author | recipient | control | group | sharee | shareType | + | cache-author-direct | cache-recipient-direct | cache-control-direct | cache-direct | cache-recipient-direct | user | + | cache-author-group | cache-recipient-group | cache-control-group | cache-group | cache-group | group | + Scenario: user gets an email notification when someone shares a file Given user "Alice" has uploaded file with content "sample text" to "lorem.txt" @@ -364,3 +471,116 @@ Feature: Email notification | interval | | daily | | weekly | + + @issue-3513 + Scenario Outline: a disabled user does not receive the grouped email + Given these users have been created with default attributes: + | username | + | digest-author- | + | digest-target- | + | digest-active- | + And user "digest-target-" has set the email sending interval to "" using the settings API + And user "digest-active-" has set the email sending interval to "" using the settings API + And user "digest-author-" has uploaded file with content "digest content" to "digest.txt" + And user "digest-author-" has sent the following resource share invitation: + | resource | digest.txt | + | space | Personal | + | sharee | digest-target- | + | shareType | user | + | permissionsRole | Viewer | + And user "digest-author-" has sent the following resource share invitation: + | resource | digest.txt | + | space | Personal | + | sharee | digest-active- | + | shareType | user | + | permissionsRole | Viewer | + And user "digest-target-" should keep "0" emails for "5" seconds + And user "digest-active-" should have "0" emails + And the user "Admin" has disabled user "digest-target-" + And the user waits for "12" seconds + When the administrator sends the grouped "" email notifications using the CLI + Then user "digest-active-" should have received the following email from user "digest-author-" + """ + %displayname% has shared "digest.txt" with you. + """ + And user "digest-active-" should have "1" emails + And user "digest-target-" should keep "0" emails for "5" seconds + Examples: + | interval | + | daily | + | weekly | + + @issue-3513 + Scenario: a disabled user does not receive an email when a file share is removed + Given these users have been created with default attributes: + | username | displayname | + | revoke-author | Alice Hansen | + | revoke-target | Brian Murphy | + | revoke-active | Carol King | + And user "revoke-author" has uploaded file with content "shared content" to "revoked.txt" + And user "revoke-author" has sent the following resource share invitation: + | resource | revoked.txt | + | space | Personal | + | sharee | revoke-target | + | shareType | user | + | permissionsRole | Viewer | + And user "revoke-target" should have "1" emails + And the user "Admin" has disabled user "revoke-target" + And the user waits for "12" seconds + When user "revoke-author" has removed the access of user "revoke-target" from resource "revoked.txt" of space "Personal" + And user "revoke-author" has sent the following resource share invitation: + | resource | revoked.txt | + | space | Personal | + | sharee | revoke-active | + | shareType | user | + | permissionsRole | Viewer | + And user "revoke-active" should have "1" emails + And user "revoke-author" has removed the access of user "revoke-active" from resource "revoked.txt" of space "Personal" + Then user "revoke-active" should have received the following email from user "revoke-author" + """ + %displayname% has unshared 'revoked.txt' with you. + """ + And user "revoke-active" should have "2" emails + And user "revoke-target" should keep "1" emails for "5" seconds + + @issue-3513 + Scenario: mention emails reach active users but the API rejects a disabled recipient + Given these users have been created with default attributes: + | username | + | mention-author | + | mention-target | + | mention-active | + And user "mention-author" has uploaded file with content "mention content" to "mentioned.txt" + And user "mention-author" has sent the following resource share invitation: + | resource | mentioned.txt | + | space | Personal | + | sharee | mention-target | + | shareType | user | + | permissionsRole | Viewer | + And user "mention-author" has sent the following resource share invitation: + | resource | mentioned.txt | + | space | Personal | + | sharee | mention-active | + | shareType | user | + | permissionsRole | Viewer | + And user "mention-target" should have "1" emails + And user "mention-active" should have "1" emails + When user "mention-author" mentions user "mention-target" on file "mentioned.txt" in space "Personal" using the Graph API + Then the HTTP status code should be "202" + And user "mention-target" should have received the following email from user "mention-author" + """ + %displayname% mentioned you in "mentioned.txt". + """ + And user "mention-target" should have "2" emails + Given the user "Admin" has disabled user "mention-target" + And the user waits for "12" seconds + When user "mention-author" mentions user "mention-target" on file "mentioned.txt" in space "Personal" using the Graph API + Then the HTTP status code should be "404" + When user "mention-author" mentions user "mention-active" on file "mentioned.txt" in space "Personal" using the Graph API + Then the HTTP status code should be "202" + And user "mention-active" should have received the following email from user "mention-author" + """ + %displayname% mentioned you in "mentioned.txt". + """ + And user "mention-active" should have "2" emails + And user "mention-target" should keep "2" emails for "5" seconds