How to test user related security on service?

[soullivaneuh]

Comment/Problem

I made a children service. The create method require a userId field to link the child the user account.

The userId must be the same as the authenticated one if the user is not admin, so I setup a hook like this:

if (params.provider
      && !params.user.isAdmin
      && data.userId !== params.user._id
) {
  throw new BadRequest(`field userId must match the authenticated user`);
}

So I wrote this test:

it('can not create a child with a different user then the authenticated one', async () => {
  await expect(app.service('children').create({
    ...childDefaults,
    userId: parent2._id,
  }, {
    provider: 'rest',
    user: parent1,
  })).rejects.toThrow(BadRequest);
});

But instead of BadRequest, I have NotAuthenticated error. So I added authentication process:

it('can not create a child with a different user then the authenticated one', async () => {
  await app.service('authentication').create({
    strategy: 'local',
    email: parent1.email,
    password: 'test1234',
  }, {});

  await expect(app.service('children').create({
    ...childDefaults,
    userId: parent2._id,
  }, {
    provider: 'rest',
    user: parent1,
  })).rejects.toThrow(BadRequest);
});

But this change not a thing.

I'm not sure to understand all the doc about this. What is the quicker and more reliable way to test authenticated user filtering and authorization?

[daffl]

Don't check for params.provider but for params.user instead:

if (params.user
      && !params.user.isAdmin
      && data.userId !== params.user._id
) {
  throw new BadRequest(`field userId must match the authenticated user`);
}

params.user will always be available for external requests when authentication is enabled.

[soullivaneuh]

Oh ok, so I guess same thing here?

find: [
  (context: HookContext) => {
    const { params } = context;

    if (params.user && !params.user.isAdmin) {
      Object.assign(params.query, {
        userId: params.user._id,
      });
    }
  },
],

Is it clearly indicated somewhere with an example? It would be a great addition. :+1:

[soullivaneuh]

I did the change but this lead to a security issue error:

● 'children' service › can not create a child with a different user then the authenticated one

    expect(received).rejects.toThrow()

    Received promise resolved instead of rejected
    Resolved to value: {"_id": "i8wcWh9QR1NblqmS", "birthday": 2020-06-01T17:34:25.358Z, "boosterSeat": false, "firstName": "John", "lastName": "Doe", "userId": "3WMnevR9T5rRNqwA"}

      59 |     }, {});
      60 | 
    > 61 |     await expect(app.service('children').create({
         |           ^
      62 |       ...childDefaults,
      63 |       userId: parent2._id,
      64 |     })).rejects.toThrow(BadRequest);

      at expect (node_modules/expect/build/index.js:138:15)
      at Object.<anonymous> (test/services/children.test.ts:61:11)

Here is the related test:

it('can not create a child with a different user then the authenticated one', async () => {
  await app.service('authentication').create({
    strategy: 'local',
    email: parent1.email,
    password: 'test1234',
  }, {});

  await expect(app.service('children').create({
    ...childDefaults,
    userId: parent2._id,
  })).rejects.toThrow(BadRequest);
});

Am I missing something? :thinking:

[daffl]

Correct. This is also mentioned in the Testing services chapter of the guide.

If you are testing services through the Feathers client, authentication always has to happen first.

[soullivaneuh]

I did manage to get my test running correctly:

it('can not create a child with a different user then the authenticated one', async () => {
  await expect(app.service('children').create({
    ...childDefaults,
    userId: parent2._id,
  }, {
    user: parent1,
  })).rejects.toThrow(BadRequest);
});

Is it safe enough to test user authorization?

[soullivaneuh]

It works for the create test but not with find:

it('return children of parent 1', () => app.service('children').find({ user: parent1 })
  .then((children) => {
    expect(children.total).toBe(2);
  })
);

Gives:

 FAIL  test/services/children.test.ts
  ● 'children' service › return children of parent 1

    TypeError: Cannot convert undefined or null to object
        at Function.assign (<anonymous>)

Not sure to fully understand what this error means. :thinking:

[soullivaneuh]

Forget it, the failure came from my code.

Thanks for you help! I let the issue open and let you decide if you need to update the documentation about this subject.

[soullivaneuh]

Going back again on this issue.

I am trying isProvider but I'm not sure how to implement it properly to work with the current test.

In a nutshell, I would like a security condition like this:

iff(
  isProvider('server'),
  iff(
    isNot(isAdmin()),
    iff(
      isNot(isSelf()),
      disallow(),
    ),
  ),
),

Where I suppose server means internal requests (BTW, what external means?), but I don't really know how to adapt the test to this.

[soullivaneuh]

I ended up creating my custom hook:

const isInternal = () => (context: HookContext): boolean => typeof context.params.user === 'undefined';

iff(
  isNot(isInternal()),
  iff(
    isNot(isAdmin()),
    iff(
      isNot(isSelf()),
      disallow(),
    ),
  ),
),

But I can't use it with isProvider. I'm not sure this is the best way to do. :thinking: