kuzzleio / kuzzle

Open-source Back-end, self-hostable & ready to use - Real-time, storage, advanced search - Web, Apps, Mobile, IoT -
https://kuzzle.io
Apache License 2.0
1.43k stars 123 forks source link

Possibly superfluous property brought by an update action on User #2446

Closed psam44 closed 1 year ago

psam44 commented 1 year ago

The reported behaviour is coming from a copy of the implementation found in kuzzle-device-manager, lib\core\registers\DecodersRegister.ts, createDefaultUsers().

Here is a User with a sdk.security.createUser(userId, userDefinition): lptm.payload Object id:"lptm.payload" profileIds:Array[1] 0:"lptm.payload" _kuzzle_info:Object author:null updater:null createdAt:1681812575816 updatedAt:null credentials:Object

The same User after a sdk.security.updateUser(userId, userDefinition): lptm.payload Object id:"lptm.payload" profileIds:Array[1] 0:"lptm.payload" _kuzzle_info:Object author:null updater:null createdAt:1681812575816 updatedAt:1681839312658 content:Object profileIds:Array[1] 0:"lptm.payload" credentials:Object

Notice the presence of the extra 'content' property. I don't think it is intentional.

The issue is in lib\api\controllers\securityController.js createUser(request) has: const content = request.getBodyObject("content");

but updateUser(request) has: const content = request.getBody();

Kuzzle version: 2.22.0 Node.js version: 18.13.0 SDK version: 7.10.7

Another example, with a reference to the original KDM: users_contentOnUpdate

Aschen commented 1 year ago

Hello @psam44

It's because the object returned by the SDK is an User object from the SDK. The content property is deprecated thus

psam44 commented 1 year ago

I don't understand what you mean with the SDK.. What I see with a trace in kuzzle\lib\core\security\userRepository.js::persist() is that:

For a create:

User {
  _id: 'lptm.payload',
  profileIds: [ 'lptm.payload' ],
  _kuzzle_info: {
    author: undefined,
    createdAt: 1681839523470,
    updatedAt: null,
    updater: null
  }
}

For an update:

User {
  _id: 'lptm.payload',
  profileIds: [ 'lptm.payload' ],
  _kuzzle_info: { createdAt: 1681812575813, updatedAt: 1681839523500, updater: null },
  content: { profileIds: [ 'lptm.payload' ] }
}

Additional note: strangely, there is no author in this case.

Aschen commented 1 year ago

This behavior is unexpected I conceive but intentional.

The security:createUser action takes 2 arguments in the body:

Credentials are not stored into the user document and instead are considered as linked resources, for example to update credentials you would use security:updateCredentials

The security:updateUser action can only update the user content and that's why it consider the whole body as it.

I was able to reproduce the behavior with this script kourou sdk:execute < script.js

const userDefinition = {
  content: {
    profileIds: ["admin"]
  }
};

try {
  await sdk.security.deleteUser('barfoo')
}
catch {}

await sdk.security.createUser('barfoo', userDefinition)

await sdk.security.updateUser('barfoo', userDefinition);

return sdk.security.getUser('barfoo')
psam44 commented 1 year ago

OK, it becomes clearer now. To be right, your sample code should be:

await sdk.security.updateUser('barfoo', userDefinition.content);

That is the fix I had to set in my code. Before, the traces were:

{ content: { profileIds: [ 'lptm.payload.cooper' ] } }
pids null
User {
  _id: 'lptm.payload.cooper',
  profileIds: [ 'lptm.payload.cooper' ],
  _kuzzle_info: {
    author: null,
    createdAt: 1681812575815,
    updatedAt: 1681987480982,
    updater: null
  },
  content: { profileIds: [ 'lptm.payload.cooper' ] }
}

After the fix:

{ profileIds: [ 'lptm.payload.cooper' ] }
pids [ 'lptm.payload.cooper' ]
User {
  _id: 'lptm.payload.cooper',
  profileIds: [ 'lptm.payload.cooper' ],
  _kuzzle_info: { createdAt: 1681988635402, updatedAt: 1681988672857, updater: null }
}

So the real culprit was in kuzzle-device-manager. My base was version 2.0.3, so it's probably outdated now. It seems that existing users are deleted before to be created again (with 'overwrite' policy).

Aschen commented 1 year ago

So the real culprit was in kuzzle-device-manager. My base was version 2.0.3, so it's probably outdated now. It seems that existing users are deleted before to be created again

In the latest version, existing gateway users are not recreated (see here)

psam44 commented 1 year ago

Closed as no longer relevant.