Activity

1. digitalcortex commented on Apr 3, 2020

   

   digitalcortex

   on Apr 3, 2020

   More actions

   Is the problem still active? How come nobody responded to it?
2. amxmln commented on Apr 28, 2020

   

   amxmln

   on Apr 28, 2020

   Author

   More actions

   I haven't tried it in v5 yet, however I cannot see anything in the changelog that would hint at a mitigation of this issue.

   It might not affect a large userbase, which might explain the slow response—especially since it can be remedied with some additional checks in hooks if the developer is aware of this behaviour.
3. fratzinger commented on May 22, 2022

   

   fratzinger

   on May 22, 2022

   Collaborator

   More actions

   What was your way to get around this? Are you up to add a note about this in the docs? That would be really neat!
4. amxmln commented on May 23, 2022

   

   amxmln

   on May 23, 2022

   Author

   More actions

   I wrote a little Hook that allows whitelisting modifiers:

   // Use this hook to manipulate incoming or outgoing data.
   // For more information on hooks see: http://docs.feathersjs.com/api/hooks.html
   const { BadRequest } = require('@feathersjs/errors');
   
   // eslint-disable-next-line no-unused-vars, arrow-body-style
   module.exports = (...allowed) => {
     return (context) => {
       if (!context.params.provider) return context; // internal calls are fine
       if (context.type !== 'before') throw new Error('The \'allowUpdateOperators\' hook should only be used as a \'before\' hook');
   
       const fields = Object.keys(context.data);
       fields.forEach((field) => {
         if (field.startsWith('$') && !allowed.includes(field)) throw new BadRequest(`The update operator '${field}' is not allowed in this context. (allowUpdateOperators)`);
       });
       return context;
     };
   };

   It’s pretty specific to my use-case, I don’t know how generally applicable it is. I’d be happy to add a note in the docs though if you think it’s useful!
5. fratzinger commented on May 23, 2022

   

   fratzinger

   on May 23, 2022

   Collaborator

   More actions

   Thanks for the quick answer!

   Maybe I oversaw something, but what about this?:

   iff(
     isProvider('external'),
     preventChanges(true, '$push', 'all', 'your', 'other', 'fields')
   )

   Won't that work?
6. amxmln commented on May 24, 2022

   

   amxmln

   on May 24, 2022

   Last edited by amxmln

   Author

   More actions

   I think that might work as well. 🤔 Seems a lot simpler for certain! 😅 Not sure though, it’s been a good while since I’ve actively worked with Feathers.

   As a little addendum to my comment from yesterday: obviously that hook only makes sense if there’s another hook running before it that prevents all update operators:

   // Use this hook to manipulate incoming or outgoing data.
   // For more information on hooks see: http://docs.feathersjs.com/api/hooks.html
   const { BadRequest } = require('@feathersjs/errors');
   // eslint-disable-next-line no-unused-vars, arrow-body-style
   module.exports = (options = {}) => {
     return async (context) => {
       if (context.type !== 'before') throw new Error('The \'preventUpdateOperators\' hook should only be used as a \'before\' hook');
   
       const fields = Object.keys(context.data);
       fields.forEach((field) => {
         if (field.startsWith('$')) throw new BadRequest(`The update operator ${field} is not allowed in this context. (preventUpdateOperators)`);
       });
   
       return context;
     };
   };

   While your solution with preventChanges(true, '$push'); may be simpler, there’s plenty of these operators and having to list them all might be cumbersome / error prone.

   I still think that having logic that handles these cases in the preventChanges hook might make it more in line with user expectations, although obviously it also comes with the tradeoff of cluttering up the hook with database-specific cases. I’m not sure if adapters other than Mongo and Mongoose support these kinds of operators. 🤔

   Edit: now that I think of it, what of cases where $pull might be wanted on one property, but not the other? My hooks would fail in that case. preventChanges could check if a property to prevent changes on is listed in an update operator and block only that property, but not others. (Obviously easier said than done. 😅 )