Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protect()-hook does not hide the protected field in realtime events when patching multiple records #1809

Closed
Tr3ffel opened this issue Feb 14, 2020 · 4 comments

Comments

@Tr3ffel
Copy link

Tr3ffel commented Feb 14, 2020

I'm currently building a feathersjs application where I need to patch multiple user inside some service method. Because protect('password') is set in the user hooks I would expect that the password would be hidden in the realtime events, which is not the case. (I'm using the mongoose feathers adapter)

Steps to reproduce

users.hooks.js

...
after: {
    all: [ 
      // Make sure the password field is never sent to the client
      // Always must be the last hook
      protect('password')
    ],
....

users.service.js

...
const options = {
    Model,
    paginate,
    multi: true // allowed to patch multiple users
  };
  // Initialize our service with any options it requires
  app.use('/users', new Users(options, app));
...

some-service.class.js

exports.SomeService = class SomeService {
  setup(app) {
    this.app = app;
  }

  async create (data, params) {
    // realtime event returns user without password
    await this.app.service('users').patch(params.user._id, {
      name: 'test'
    });

    // realtime events returns user with password
    await this.app.service('users').patch(null, {
      name: 'qwertz'
    });

    return null;
  }
};

channel.js

...
app.on('login', (authResult, { connection }) => {
    if(connection) {
      const user = connection.user;
      app.channel('anonymous').leave(connection);
      app.channel('authenticated').join(connection);
      app.channel(`userIds/${user._id}`).join(connection);
    }
  });

  app.service('users').publish(user => {
    return [app.channel(`userIds/${user._id}`)];
  });
...

When calling app.service('some-service').create(data, params) the handler on the client side:

app.service('/users').on('patched', (user, context) => { console.log('users was patched', user) })

returns the following user objects:

  1. Event
    image

  2. Event
    image

Expected behavior

I'm not sure if I missed something but i would expect that the password field would also be hidden inside the 2. Event.

I've created a repo where the issue can be reproduced:
https://github.com/Tr3ffel/feathers-update-many-bug

@GautierT
Copy link

GautierT commented Jun 4, 2021

Same problem. Is this an expected behavior ?

Thanks.

@marshallswain
Copy link
Member

@GautierT it's not expected behavior.

@Tr3ffel thanks for the reproduction. When you get a moment if you inspect/debug the response as it comes through this block of code, that will probably help determine the cause of the problem.

The dispatch resolvers in Feathers dove are the official solution to some of the issues with using the protect hook. The dispatch resolvers will even be capable of protecting fields on populated data without having to think about the data structure of your populated data. Even deeply-nested populated data will be properly handled, including arrays.

In the meantime, it seems like protect has a bug. It will speed things along if you can report the results of your debugging on that section of code.

@marshallswain
Copy link
Member

Also, you can see an example dispatch resolver in use, here: https://dove.feathersjs.com/guides/basics/schemas.html#adding-a-user-avatar.

Don't be intimidated by resolvers. They might look complicated because they're different, but they save a lot of code compared to hooks in the right scenarios. Basically, resolvers are great any time you need to modify or populate based on object properties. Generally, they don't cause side effects other than maybe making a request to pull in some data to use for the property value.

Hooks still work the same as they always have, but I've found the majority of my service hooks have been replaced by resolvers. Hooks are great for executing side effects like logging, or dynamically creating a connection to a database.

@daffl
Copy link
Member

daffl commented Feb 24, 2023

This has been fixed via #3073 and published in v4.5.16

@daffl daffl closed this as completed Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants