There may be an interaction between softDelete and authentication

[eddyystop]

Someone reported authentication working with softDelete in feathers-hooks-common v3.2.0 but not v3.3.0. The softDelete was on the users service.

@ekryski is tracking the issue down, and is not positive the problem is with softDelete.

softDelete used to do 2 gets for service.get, one with provider: undefined, the other with provider: hook.para,s.provider. Now it does just one with provider: hook.params.provider.

[eddyystop]

More info #163 (comment)

[ghost]

I don't know if this is related, but I was trying out softDelete and it works as expected, the only problem I'm getting from it is it's still giving me

{
  "name": "NotFound",
  "message": "Item not found.",
  "code": 404,
  "className": "not-found",
  "errors": {}
}

Even though it was able to successfully patch the item with deleted: true. The before hooks from this service looks like this:

  before: {
    all:  [ softDelete() ],
    find: [],
    get: [],
    create: [ 
      authenticate('jwt'),
      associateCurrentUser({ as: 'ownerId' }),
    ],
    update: [...restrict],
    patch: [...restrict],
    remove: [...restrict],
  },

[eddyystop]

Am I correct in understanding you got the NotFound message during the patch call, even though that call changed the record as expected?

softDelete does not contain any after hooks, so it ran nothing after the successful patch call.

That would be the message you'd get if you tried a get after patching the record to be deleted.

What DB are you using? Perhaps you can clarify the situation, or provide some code that shows the situation?

[ghost]

@eddyystop Hi eddy. Yeah sure, it's actually a new toy project, I'm familiarizing myself with Auk so I can update some of our legacy projects that were using older versions/styles. The code is mostly boilerplate code yet from the latest feathers-cli. And no, I'm not trying to do a get after the remove. I just send a delete request from Postman and that's what I get.

https://github.com/johncrisostomo/voting-backend/tree/master/src/services/polls

[eddyystop]

I couldn't find a softDelete in either polls or users in the link above. Could you provide a breaking example and instructions to cause it?

Could you also run with feathers-hooks-common v3.2.0 rather than the current one and if the issue still occurs? Thanks.

[ghost]

Sorry for the late response and sorry because yeah I removed it from that code. I brought it back and tried to roll back feathers-hooks-common to v3.2.0 and it is still the same. You can look at the same repo for reference. I'm just adding a poll and then trying to remove it with a DELETE request from Postman.

[eddyystop]

Could you try this with v3.4.0 which will be published soon?

#197 might affect this issue. In any case I want to eliminate it as a cause.

Thanks.

[jordanbtucker]

Just ran into this problem today, however I am using softDelete as an application hook rather than a service hook. I also have authentication on my service.

app.hooks.js

const {softDelete} = require('feathers-hooks-common')
module.exports = {
	before: {
		all: [softDelete()],
		find: [],
		get: [],
		create: [],
		update: [],
		patch: [],
		remove: [],
	},
...
}

agents.hooks.js

const {authenticate} = require('feathers-authentication').hooks
module.exports = {
	before: {
		all: [authenticate('jwt')],
		find: [],
		get: [],
		create: [],
		update: [],
		patch: [],
		remove: [],
	},
...
}

When I perform a REST DELETE on an agent, I get:

{
    "name": "NotFound",
    "message": "Item not found.",
    "code": 404,
    "className": "not-found",
    "errors": {}
}

and {deleted: true} does not get set on the agent.

When I move softDelete to agents.hooks.js, everything works. Or when I remove authenticate from agents.hooks.js everything works.

Here are my feathers versions:

"feathers": "^2.1.7",
"feathers-authentication": "^1.2.7",
"feathers-authentication-hooks": "^0.1.4",
"feathers-authentication-jwt": "^0.3.2",
"feathers-authentication-local": "^0.4.3",
"feathers-configuration": "^0.4.1",
"feathers-errors": "^2.9.1",
"feathers-hooks": "^2.0.2",
"feathers-hooks-common": "^3.5.5",
"feathers-nedb": "^2.6.2",
"feathers-rest": "^1.8.0",
"feathers-socketio": "^2.0.0",

[eddyystop]

I assume authenticate makes internal service calls itself and softDelete is not compatible with them. I wouldn't know more than that. Perhaps @ekryski can comment.

[eddyystop]

Perhaps you can add the debug() hook before softDelete and perhaps identify what, if any, the unexpected called service is. Then you can instead use when(context => context.path !== '...', softDelete());

Please let me know what happens.

[jordanbtucker]

Added debug hooks to app.hooks.all and agents.hooks.all. Here's the result.

* app.hooks.before.all
type:before, method: remove
query: {}
* app.hooks.before.all
type:before, method: get
query: { '$disableSoftDelete': true }
* agents.hooks.before.all
type:before, method: get
data: {}
query: {}
info: error: agents - Method: get: No auth token
error:  NotAuthenticated: No auth token
    at Error.NotAuthenticated (node_modules\feathers-errors\lib\index.js:100:17)
    at node_modules\feathers-authentication\lib\hooks\authenticate.js:102:31
    at process._tickCallback (internal/process/next_tick.js:109:7)
info: error: agents - Method: remove: Item not found.
error:  NotFound: Item not found.
    at Error.NotFound (node_modules\feathers-errors\lib\index.js:121:17)
    at node_modules\feathers-hooks-common\lib\services\soft-delete.js:74:15
    at process._tickCallback (internal/process/next_tick.js:109:7)

After reviewing the code for softDelete, it appears that when remove is called, it checks to see if the item has already been deleted, and it does this with a get. Since app hooks are called before service hooks, the user hasn't been authenticated when get is called, and that's where the error occurs.

The soft delete works after moving authenticate to app.hooks.before.all and before softDelete.

So, as long as authenticate comes before softDelete, everything works.

[ksloan]

I have service hooks like this:

module.exports = {
  before: {
    all: [softDelete()],
    find: [],
    get: [],
    create: [authenticate('jwt')],
    update: [authenticate('jwt')],
    patch: [authenticate('jwt')],
    remove: [authenticate('jwt')]
  }
}

In this case I can't run authenticate before softDelete in the before.all hook. Is the following an appropriate way of implementing softDelete hooks?

module.exports = {
  before: {
    all: [],
    find: [softDelete()],
    get: [softDelete()],
    create: [authenticate('jwt'), softDelete()],
    update: [authenticate('jwt'), softDelete()],
    patch: [authenticate('jwt'), softDelete()],
    remove: [authenticate('jwt'), softDelete()]
  }
}

[eddyystop]

I think so. Please inform us of your results. Thanks.

[ksloan]

Seems to be working

[eddyystop]

Good to know. +1

[rcostalenz]

Having the same problem here with the current version

[rcostalenz]

When I soft delete I received an item not found error, but the item is successfully soft deleted at the db. I`m using MongoDB.

[rcostalenz]

The problem happens when restrictToOwner is used at the remove hook with soft delete

[bertho-zero]

@rcostalenz Can you post a minimalist example reproducing this bug? To help identify the problem clearly and find a solution.

[rcostalenz]

Here it is, if you remove the restrictToOwner from the remove hook it works just fine, but with it you will receive an item not found error from the SoftDelete, but the item will be soft deleted at the database, the function returning the error is the throwIfItemDeleted from the soft-delete module.

module.exports = {
  before: {
    all: [ 
      authenticate('jwt'),
      softDelete() ],
    find: [
      authHooks.queryWithCurrentUser({ idField: '_id', as: 'userId' })
    ],
    get: [
      authHooks.restrictToOwner({ idField: '_id', ownerField: 'userId' })
    ],
    create: [
      authHooks.associateCurrentUser({ idField: '_id', as: 'userId' }),
      setNow('createdAt', 'updatedAt')
    ],
    update: [
      authHooks.restrictToOwner({ idField: '_id', ownerField: 'userId' }),
      setNow('updatedAt')
    ],
    patch: [  
      authHooks.restrictToOwner({ idField: '_id', ownerField: 'userId' }),
      setNow('updatedAt')
    ],
    remove: [
      // HERE IS THE PROBLEM:
      // authHooks.restrictToOwner({ idField: '_id', ownerField: 'userId' })
    ]
  },

[eddyystop]

@rcostalenz, thank you for the example. It was the most useful I received.

softDelete does probing get calls and replaces remove calls with patch. restrictToOwner does its own probing get calls using params: context.params with what side effects that may cause. The problem is that each of these "additional" calls runs the hooks defined for that type of call. So a remove call runs the get hooks (for restrictToOwner) and patch hooks (for softDelete) as well as the remove ones.

It was not possible to get around this issue until the Feathers hook handler allowed SKIP to be returned.

softDelete2 is a noticeable improvement over softDelete. However code changes to existing apps are required to use it. It has a substantial test suite to test

      posts.hooks({
        before: {
          all:    [ paramsFromClient('$ignoreDeletedAt'), authenticate('jwt') ],
          get:    [ softDelete2(), restrictToOwner({ idField: 'id', ownerField: 'ownerId' }) ],
          create: [ softDelete2(), restrictToOwner({ idField: 'id', ownerField: 'ownerId' }) ],
          update: [ softDelete2(), restrictToOwner({ idField: 'id', ownerField: 'ownerId' }) ],
          patch:  [ softDelete2(), restrictToOwner({ idField: 'id', ownerField: 'ownerId' }) ],
          remove: [ restrictToOwner({ idField: 'id', ownerField: 'ownerId' }), softDelete2() ]
        },
        after: {
          all: softDelete2()
        }
      });

I'm closing this issue as softDelete2 will be released at the end of this week once its docs are done.