Skip to content

Commit a6b6027

Browse files
jessegeensjessegeens
authored
Move notification registration from http to publicstorageprovider (#5427)
1 parent 369d294 commit a6b6027

File tree

8 files changed

+133
-74
lines changed

8 files changed

+133
-74
lines changed

‎changelog/unreleased/fix-upload-notifications.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Bugfix: fix upload notifications
2+
3+
The registration of notifications for uploads in a public link folder was
4+
until now only handled in the OCS HTTP layer; this is the responsibility
5+
of the public share provider. Since it was also missing from the OCGraph
6+
layer, this has been moved to the "gRPC" part of reva
7+
8+
https://github.com/cs3org/reva/pull/5427

‎internal/grpc/services/publicshareprovider/publicshareprovider.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ import (
2626
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
2727
"github.com/cs3org/reva/v3/pkg/appctx"
2828
"github.com/cs3org/reva/v3/pkg/errtypes"
29+
"github.com/cs3org/reva/v3/pkg/notification"
30+
"github.com/cs3org/reva/v3/pkg/notification/notificationhelper"
2931
"github.com/cs3org/reva/v3/pkg/plugin"
3032
"github.com/cs3org/reva/v3/pkg/publicshare"
3133
"github.com/cs3org/reva/v3/pkg/publicshare/manager/registry"
3234
"github.com/cs3org/reva/v3/pkg/rgrpc"
3335
"github.com/cs3org/reva/v3/pkg/rgrpc/status"
3436
"github.com/cs3org/reva/v3/pkg/utils"
3537
"github.com/cs3org/reva/v3/pkg/utils/cfg"
38+
"github.com/pkg/errors"
3639
"google.golang.org/grpc"
3740
)
3841

@@ -49,6 +52,7 @@ type config struct {
4952
Driver string `mapstructure:"driver"`
5053
Drivers map[string]map[string]any `mapstructure:"drivers"`
5154
AllowedPathsForShares []string `mapstructure:"allowed_paths_for_shares"`
55+
Notifications map[string]any `mapstructure:"notifications"`
5256
}
5357

5458
func (c *config) ApplyDefaults() {
@@ -61,6 +65,8 @@ type service struct {
6165
conf *config
6266
sm publicshare.Manager
6367
allowedPathsForShares []*regexp.Regexp
68+
// May be nil if this publicstorageprovider runs without notifications
69+
notificationHelper *notificationhelper.NotificationHelper
6470
}
6571

6672
func getShareManager(ctx context.Context, c *config) (publicshare.Manager, error) {
@@ -110,6 +116,16 @@ func New(ctx context.Context, m map[string]any) (rgrpc.Service, error) {
110116
allowedPathsForShares: allowedPathsForShares,
111117
}
112118

119+
if c.Notifications != nil {
120+
log := appctx.GetLogger(ctx)
121+
nh, err := notificationhelper.New("publicshareprovider", c.Notifications, log)
122+
if err != nil {
123+
log.Error().Err(err).Msgf("Failed to construct notificationhelper")
124+
return nil, errors.Wrap(err, "failed to construct notificationhelper")
125+
}
126+
service.notificationHelper = nh
127+
}
128+
113129
return service, nil
114130
}
115131

@@ -168,8 +184,10 @@ func (s *service) RemovePublicShare(ctx context.Context, req *link.RemovePublicS
168184

169185
user := appctx.ContextMustGetUser(ctx)
170186
err := s.sm.RevokePublicShare(ctx, user, req.Ref)
187+
171188
switch err.(type) {
172189
case nil:
190+
s.notificationHelper.UnregisterNotification(req.Ref.GetId().OpaqueId)
173191
return &link.RemovePublicShareResponse{
174192
Status: status.NewOK(ctx),
175193
}, nil
@@ -273,6 +291,38 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS
273291
log.Error().Msg("error getting user from context")
274292
}
275293

294+
share, err := s.sm.GetPublicShare(ctx, u, req.Ref, false)
295+
if err != nil {
296+
return &link.UpdatePublicShareResponse{
297+
Status: status.NewNotFound(ctx, "share not found"),
298+
}, nil
299+
}
300+
301+
if s.notificationHelper != nil {
302+
if !share.NotifyUploads && req.Update.NotifyUploads {
303+
n := &notification.Notification{
304+
TemplateName: "sharedfolder-upload-mail",
305+
Ref: req.Ref.GetId().OpaqueId,
306+
Recipients: []string{u.Mail},
307+
}
308+
309+
s.notificationHelper.RegisterNotification(n)
310+
} else if share.NotifyUploads && !req.Update.NotifyUploads {
311+
s.notificationHelper.UnregisterNotification(req.Ref.GetId().OpaqueId)
312+
}
313+
314+
if share.NotifyUploadsExtraRecipients != req.Update.NotifyUploadsExtraRecipients {
315+
if len(req.Update.NotifyUploadsExtraRecipients) > 0 {
316+
n := &notification.Notification{
317+
TemplateName: "sharedfolder-upload-mail",
318+
Ref: req.Ref.GetId().OpaqueId,
319+
Recipients: []string{u.Mail, req.Update.NotifyUploadsExtraRecipients},
320+
}
321+
s.notificationHelper.RegisterNotification(n)
322+
}
323+
}
324+
}
325+
276326
updated, err := s.sm.UpdatePublicShare(ctx, u, req, nil)
277327
switch err.(type) {
278328
case nil:

‎internal/http/services/owncloud/ocdav/ocdav.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ type svc struct {
158158
favoritesManager favorite.Manager
159159
myOfficeFilesManager myofficefiles.Manager
160160
client *httpclient.Client
161+
// Can be nil if notifications are not set up
161162
notificationHelper *notificationhelper.NotificationHelper
162163
}
163164

@@ -187,6 +188,8 @@ func New(ctx context.Context, m map[string]any) (global.Service, error) {
187188

188189
log := appctx.GetLogger(ctx)
189190
tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: c.Insecure}}
191+
192+
190193
s := &svc{
191194
c: &c,
192195
webDavHandler: new(WebDavHandler),
@@ -196,10 +199,16 @@ func New(ctx context.Context, m map[string]any) (global.Service, error) {
196199
httpclient.RoundTripper(tr),
197200
),
198201
favoritesManager: fm,
199-
notificationHelper: notificationhelper.New("ocdav", c.Notifications, log),
200202
myOfficeFilesManager: myOfficeFilesManager,
201203
}
202-
204+
if c.Notifications != nil {
205+
nh, err := notificationhelper.New("ocdav", c.Notifications, log)
206+
if err != nil {
207+
return nil, err
208+
}
209+
s.notificationHelper = nh
210+
}
211+
203212
// initialize handlers and set default cigs
204213
if err := s.webDavHandler.init(c.WebdavNamespace, true); err != nil {
205214
return nil, err
@@ -215,7 +224,9 @@ func (s *svc) Prefix() string {
215224
}
216225

217226
func (s *svc) Close() error {
218-
s.notificationHelper.Stop()
227+
if s.notificationHelper != nil {
228+
s.notificationHelper.Stop()
229+
}
219230
return nil
220231
}
221232

‎internal/http/services/owncloud/ocdav/put.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,9 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ
418418
"fileName": shareFileName,
419419
},
420420
}
421-
s.notificationHelper.TriggerNotification(trg)
421+
if s.notificationHelper != nil {
422+
s.notificationHelper.TriggerNotification(trg)
423+
}
422424
}
423425

424426
// file was new

‎internal/http/services/owncloud/ocgraph/drive_permissions.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,7 +890,15 @@ func (s *svc) getLinkUpdates(ctx context.Context, link *linkv1beta1.PublicShare,
890890
NotifyUploads: true,
891891
})
892892
}
893+
} else {
894+
if link.NotifyUploads {
895+
updates = append(updates, &linkv1beta1.UpdatePublicShareRequest_Update{
896+
Type: linkv1beta1.UpdatePublicShareRequest_Update_TYPE_NOTIFYUPLOADS,
897+
NotifyUploads: false,
898+
})
899+
}
893900
}
901+
894902
if len(updates) == 0 {
895903
return nil, errors.New("body contained nothing to update")
896904
}

‎internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/public.go‎

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
"github.com/cs3org/reva/v3/internal/http/services/owncloud/ocs/response"
3737
"github.com/cs3org/reva/v3/pkg/appctx"
3838

39-
"github.com/cs3org/reva/v3/pkg/notification"
4039
"github.com/cs3org/reva/v3/pkg/publicshare"
4140
"github.com/cs3org/reva/v3/pkg/rgrpc/todo/pool"
4241
"github.com/pkg/errors"
@@ -407,7 +406,6 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
407406
})
408407
}
409408

410-
h.notificationHelper.UnregisterNotification(shareID)
411409
}
412410
}
413411

@@ -479,59 +477,29 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar
479477
// NotifyUploads
480478
newNotifyUploads, ok := r.Form["notifyUploads"]
481479

482-
if ok {
483-
ok2 := permissionsStayUploader(before, newPermissions)
484-
u, ok3 := appctx.ContextGetUser(r.Context())
485-
486-
if ok2 && ok3 {
487-
notifyUploads, _ := strconv.ParseBool(newNotifyUploads[0])
488-
updatesFound = true
489-
490-
logger.Info().Str("shares", "update").Msgf("notify uploads updated to '%v'", notifyUploads)
491-
updates = append(updates, &link.UpdatePublicShareRequest_Update{
492-
Type: link.UpdatePublicShareRequest_Update_TYPE_NOTIFYUPLOADS,
493-
NotifyUploads: notifyUploads,
494-
})
480+
if ok && permissionsStayUploader(before, newPermissions) {
481+
notifyUploads, _ := strconv.ParseBool(newNotifyUploads[0])
482+
updatesFound = true
495483

496-
if notifyUploads {
497-
n := &notification.Notification{
498-
TemplateName: "sharedfolder-upload-mail",
499-
Ref: shareID,
500-
Recipients: []string{u.Mail},
501-
}
502-
h.notificationHelper.RegisterNotification(n)
503-
} else {
504-
h.notificationHelper.UnregisterNotification(shareID)
505-
}
506-
}
484+
logger.Info().Str("shares", "update").Msgf("notify uploads updated to '%v'", notifyUploads)
485+
updates = append(updates, &link.UpdatePublicShareRequest_Update{
486+
Type: link.UpdatePublicShareRequest_Update_TYPE_NOTIFYUPLOADS,
487+
NotifyUploads: notifyUploads,
488+
})
507489
}
508490

509491
// NotifyUploadsExtraRecipients
510492
newNotifyUploadsExtraRecipients, ok := r.Form["notifyUploadsExtraRecipients"]
511493

512-
if ok {
513-
ok2 := permissionsStayUploader(before, newPermissions)
514-
u, ok3 := appctx.ContextGetUser(r.Context())
515-
516-
if ok2 && ok3 {
517-
notifyUploadsExtraRecipients := newNotifyUploadsExtraRecipients[0]
518-
updatesFound = true
519-
logger.Info().Str("shares", "update").Msgf("notify uploads extra recipients updated to '%v'", notifyUploadsExtraRecipients)
520-
521-
updates = append(updates, &link.UpdatePublicShareRequest_Update{
522-
Type: link.UpdatePublicShareRequest_Update_TYPE_NOTIFYUPLOADSEXTRARECIPIENTS,
523-
NotifyUploadsExtraRecipients: notifyUploadsExtraRecipients,
524-
})
494+
if ok && permissionsStayUploader(before, newPermissions){
495+
notifyUploadsExtraRecipients := newNotifyUploadsExtraRecipients[0]
496+
updatesFound = true
497+
logger.Info().Str("shares", "update").Msgf("notify uploads extra recipients updated to '%v'", notifyUploadsExtraRecipients)
525498

526-
if len(notifyUploadsExtraRecipients) > 0 {
527-
n := &notification.Notification{
528-
TemplateName: "sharedfolder-upload-mail",
529-
Ref: shareID,
530-
Recipients: []string{u.Mail, notifyUploadsExtraRecipients},
531-
}
532-
h.notificationHelper.RegisterNotification(n)
533-
}
534-
}
499+
updates = append(updates, &link.UpdatePublicShareRequest_Update{
500+
Type: link.UpdatePublicShareRequest_Update_TYPE_NOTIFYUPLOADSEXTRARECIPIENTS,
501+
NotifyUploadsExtraRecipients: notifyUploadsExtraRecipients,
502+
})
535503
}
536504

537505
publicShare := before.Share
@@ -616,8 +584,6 @@ func (h *Handler) removePublicShare(w http.ResponseWriter, r *http.Request, shar
616584
return
617585
}
618586

619-
h.notificationHelper.UnregisterNotification(shareID)
620-
621587
response.WriteOCSSuccess(w, r, nil)
622588
}
623589

‎internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import (
6464
"github.com/go-chi/chi/v5"
6565
"github.com/pkg/errors"
6666
"github.com/rs/zerolog"
67+
"github.com/rs/zerolog/log"
6768
)
6869

6970
const (
@@ -72,17 +73,18 @@ const (
7273

7374
// Handler implements the shares part of the ownCloud sharing API.
7475
type Handler struct {
75-
gatewayAddr string
76-
storageRegistryAddr string
77-
publicURL string
78-
sharePrefix string
79-
homeNamespace string
80-
ocmMountPoint string
81-
additionalInfoTemplate *template.Template
82-
userIdentifierCache *ttlcache.Cache
83-
resourceInfoCache cache.ResourceInfoCache
84-
resourceInfoCacheTTL time.Duration
85-
listOCMShares bool
76+
gatewayAddr string
77+
storageRegistryAddr string
78+
publicURL string
79+
sharePrefix string
80+
homeNamespace string
81+
ocmMountPoint string
82+
additionalInfoTemplate *template.Template
83+
userIdentifierCache *ttlcache.Cache
84+
resourceInfoCache cache.ResourceInfoCache
85+
resourceInfoCacheTTL time.Duration
86+
listOCMShares bool
87+
// May be nil if OCS runs without notifications
8688
notificationHelper *notificationhelper.NotificationHelper
8789
Log *zerolog.Logger
8890
EnableSpaces bool
@@ -123,11 +125,19 @@ func (h *Handler) Init(c *config.Config, l *zerolog.Logger) {
123125
h.listOCMShares = c.ListOCMShares
124126
h.EnableSpaces = c.EnableSpaces
125127
h.Log = l
126-
h.notificationHelper = notificationhelper.New("ocs", c.Notifications, l)
127128
h.additionalInfoTemplate, _ = template.New("additionalInfo").Parse(c.AdditionalInfoAttribute)
128129
h.resourceInfoCacheTTL = time.Second * time.Duration(c.ResourceInfoCacheTTL)
129130
h.pubRWLinkMaxExpiration = time.Second * time.Duration(c.PubRWLinkMaxExpiration)
130131
h.pubRWLinkDefaultExpiration = time.Second * time.Duration(c.PubRWLinkDefaultExpiration)
132+
if c.Notifications != nil {
133+
nh, err := notificationhelper.New("ocs", c.Notifications, l)
134+
// no return value :(
135+
if err != nil {
136+
log.Fatal().Msg("Failed to initialize notification handler in OCS - no emails will be sent on share creation!")
137+
} else {
138+
h.notificationHelper = nh
139+
}
140+
}
131141

132142
h.userIdentifierCache = ttlcache.NewCache()
133143
_ = h.userIdentifierCache.SetTTL(time.Second * time.Duration(c.UserIdentifierCacheTTL))
@@ -354,6 +364,9 @@ func (h *Handler) NotifyShare(w http.ResponseWriter, r *http.Request) {
354364

355365
// SendShareNotification sends a notification with information from a Share.
356366
func (h *Handler) SendShareNotification(opaqueID string, granter *userpb.User, grantee any, statInfo *provider.ResourceInfo) string {
367+
if h.notificationHelper == nil {
368+
return ""
369+
}
357370
var granteeDisplayName, granteeName, recipient string
358371
isGranteeGroup := false
359372

0 commit comments

Comments
 (0)