Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

For for #209: createNotifications() is a server-side function now #214

Merged
merged 1 commit into from

2 participants

@yeputons

See #209 . I've made createNotifications() a server-side method, which can be called from server versions of comment and inviteUser only. Therefore, noone can send a notification to user on behalf of Telescope without actually doing some action.

No latency compensation was available for createNotifications, so there was nothing to change in it.

You may find git diff -w command useful (it ignores space characters).

@yeputons yeputons #209: createNotifications() is a server-side function now. It's calle…
…d from 'inviteUser' and 'comment' methods (not from stubs)
2f2b5d4
@SachaG
Owner

Awesome, thanks for your work!

@SachaG SachaG merged commit c536750 into from
@yeputons yeputons referenced this pull request
Closed

Notifications are insecure #209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 5, 2014
  1. @yeputons

    #209: createNotifications() is a server-side function now. It's calle…

    yeputons authored
    …d from 'inviteUser' and 'comment' methods (not from stubs)
This page is out of date. Refresh to see the latest.
Showing with 66 additions and 62 deletions.
  1. +36 −33 collections/comments.js
  2. +1 −1  server/invites.js
  3. +29 −28 server/notifications.js
View
69 collections/comments.js
@@ -81,41 +81,44 @@ Meteor.methods({
properties.parentAuthorId = parentComment.userId;
properties.parentAuthorName = getDisplayName(parentUser);
- // reply notification
- // do not notify users of their own actions (i.e. they're replying to themselves)
- if(parentUser._id != user._id){
- Meteor.call('createNotification', {
- event: 'newReply',
- properties: properties,
- userToNotify: parentUser,
- userDoingAction: user,
- sendEmail: getUserSetting('notifications.replies', false, parentUser)
- });
+ if(!this.isSimulation){
+ // reply notification
+ // do not notify users of their own actions (i.e. they're replying to themselves)
+ if(parentUser._id != user._id){
+ createNotification({
+ event: 'newReply',
+ properties: properties,
+ userToNotify: parentUser,
+ userDoingAction: user,
+ sendEmail: getUserSetting('notifications.replies', false, parentUser)
+ });
+ }
+
+ // comment notification
+ // if the original poster is different from the author of the parent comment, notify them too
+ if(postUser._id != user._id && parentComment.userId != post.userId){
+ createNotification({
+ event: 'newComment',
+ properties: properties,
+ userToNotify: postUser,
+ userDoingAction: user,
+ sendEmail: getUserSetting('notifications.comments', false, postUser)
+ });
+ }
}
-
- // comment notification
- // if the original poster is different from the author of the parent comment, notify them too
- if(postUser._id != user._id && parentComment.userId != post.userId){
- Meteor.call('createNotification', {
- event: 'newComment',
- properties: properties,
- userToNotify: postUser,
- userDoingAction: user,
- sendEmail: getUserSetting('notifications.comments', false, postUser)
- });
- }
-
}else{
- // root comment
- // don't notify users of their own comments
- if(postUser._id != user._id){
- Meteor.call('createNotification', {
- event: 'newComment',
- properties: properties,
- userToNotify: postUser,
- userDoingAction: Meteor.user(),
- sendEmail: getUserSetting('notifications.comments', false, postUser)
- });
+ if(!this.isSimulation){
+ // root comment
+ // don't notify users of their own comments
+ if(postUser._id != user._id){
+ createNotification({
+ event: 'newComment',
+ properties: properties,
+ userToNotify: postUser,
+ userDoingAction: Meteor.user(),
+ sendEmail: getUserSetting('notifications.comments', false, postUser)
+ });
+ }
}
}
}
View
2  server/invites.js
@@ -24,7 +24,7 @@ Meteor.methods({
}});
console.log(a)
- Meteor.call('createNotification', {
+ createNotification({
event: 'accountApproved',
properties: {},
userToNotify: invitedUser,
View
57 server/notifications.js
@@ -2,35 +2,36 @@ getUnsubscribeLink = function(user){
return Meteor.absoluteUrl()+'unsubscribe/'+user.email_hash;
};
-Meteor.methods({
- createNotification: function(options){
- var event = options.event,
- properties = options.properties,
- userToNotify = options.userToNotify,
- userDoingAction = options.userDoingAction,
- sendEmail = options.sendEmail;
- // console.log('adding new notification for:'+getDisplayName(userToNotify)+', for event:'+event);
- // console.log(userToNotify);
- // console.log(userDoingAction);
- // console.log(properties);
- // console.log(sendEmail);
- var notification = {
- timestamp: new Date().getTime(),
- userId: userToNotify._id,
- event: event,
- properties: properties,
- read: false
- }
- var newNotificationId=Notifications.insert(notification);
+createNotification = function(options) {
+ var event = options.event,
+ properties = options.properties,
+ userToNotify = options.userToNotify,
+ userDoingAction = options.userDoingAction,
+ sendEmail = options.sendEmail;
+ // console.log('adding new notification for:'+getDisplayName(userToNotify)+', for event:'+event);
+ // console.log(userToNotify);
+ // console.log(userDoingAction);
+ // console.log(properties);
+ // console.log(sendEmail);
+ var notification = {
+ timestamp: new Date().getTime(),
+ userId: userToNotify._id,
+ event: event,
+ properties: properties,
+ read: false
+ }
+ var newNotificationId=Notifications.insert(notification);
- // send the notification if notifications are activated,
- // the notificationsFrequency is set to 1, or if it's undefined (legacy compatibility)
- if(sendEmail){
- // get specific notification content for "email" context
- var contents = getNotificationContents(notification, 'email');
- sendNotification(contents);
- }
- },
+ // send the notification if notifications are activated,
+ // the notificationsFrequency is set to 1, or if it's undefined (legacy compatibility)
+ if(sendEmail){
+ // get specific notification content for "email" context
+ var contents = getNotificationContents(notification, 'email');
+ sendNotification(contents);
+ }
+}
+
+Meteor.methods({
unsubscribeUser : function(hash){
// TO-DO: currently, if you have somebody's email you can unsubscribe them
// A user-specific salt should be added to the hashing method to prevent this
Something went wrong with that request. Please try again.