Properly destroy a user after a credentials creation failure (#1507)

# Description Here is how a user is created in Kuzzle: 1. Kuzzle asks strategy plugins to check credentials (`validate` functions). If a plugin rejects credentials, the user creation process is aborted 2. Kuzzle creates a global user and attributes it a kuid 3. Kuzzle asks strategy plugins to create credentials 4. If a plugin fails to create credentials, Kuzzle deletes the already created credentials as well as the global user There is a bug in step 4: the incorrect argument is passed to repositories.delete, and the rollbacks does not delete the global user document. This makes Kuzzle consider the user as "already created" and even with fixed credentials, it cannot be created anymore.
kuzzleio · Nov 4, 2019 · 1cce959 · 1cce959
1 parent fbd7e0c
commit 1cce959
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 14 deletions.
diff --git a/lib/api/controllers/securityController.js b/lib/api/controllers/securityController.js
@@ -187,7 +187,7 @@ const persistUser = Bluebird.coroutine(
     }
 
     return kuzzle.repositories.user
-      .delete(pojoUser._id, { refresh: request.input.args.refresh })
+      .delete(pojoUser, { refresh: request.input.args.refresh })
       .finally(() => {
         errorsManager.throw(
           'plugin',

diff --git a/test/api/controllers/securityController/users.test.js b/test/api/controllers/securityController/users.test.js
@@ -462,10 +462,18 @@ describe('Test: security controller - users', () => {
       kuzzle.repositories.user.load.resolves(null);
       kuzzle.pluginsManager.listStrategies.returns(['someStrategy']);
 
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'validate').returns(validateStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'exists').returns(existsStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'create').returns(createStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'delete').returns(deleteStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'validate')
+        .returns(validateStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'exists')
+        .returns(existsStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'create')
+        .returns(createStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'delete')
+        .returns(deleteStub);
 
       securityController.createUser(request)
         .then(() => done('Expected promise to fail'))
@@ -475,7 +483,7 @@ describe('Test: security controller - users', () => {
             should(error.id).eql('plugin.runtime.unexpected_error');
             should(kuzzle.repositories.user.delete)
               .calledOnce()
-              .calledWith('test');
+              .calledWithMatch({_id: 'test'});
 
             done();
           }
@@ -495,10 +503,18 @@ describe('Test: security controller - users', () => {
       kuzzle.repositories.user.load.resolves(null);
       kuzzle.pluginsManager.listStrategies.returns(['someStrategy']);
 
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'validate').returns(validateStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'exists').returns(existsStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'create').returns(createStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'delete').returns(deleteStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'validate')
+        .returns(validateStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'exists')
+        .returns(existsStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'create')
+        .returns(createStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'delete')
+        .returns(deleteStub);
 
       securityController.createUser(request)
         .then(() => done('Expected promise to fail'))
@@ -508,7 +524,7 @@ describe('Test: security controller - users', () => {
             should(error.id).eql('plugin.runtime.unexpected_error');
             should(kuzzle.repositories.user.delete)
               .calledOnce()
-              .calledWith('test');
+              .calledWithMatch({ _id: 'test' });
 
             done();
           }
@@ -529,9 +545,15 @@ describe('Test: security controller - users', () => {
       kuzzle.pluginsManager.listStrategies.returns(['someStrategy']);
       kuzzle.repositories.user.persist.rejects(error);
 
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'validate').returns(validateStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'exists').returns(existsStub);
-      kuzzle.pluginsManager.getStrategyMethod.withArgs('someStrategy', 'create').returns(createStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'validate')
+        .returns(validateStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'exists')
+        .returns(existsStub);
+      kuzzle.pluginsManager.getStrategyMethod
+        .withArgs('someStrategy', 'create')
+        .returns(createStub);
 
       securityController.createUser(request)
         .then(() => done('Expected promise to fail'))