Habitica: validation of user / group / challenge / task documents

[Alys]

We sometimes have trouble with players' accounts because they contain objects that are not supposed to be there, or don't contain objects that should be there. It's likely that similar problems could occur with groups, challenges, and tasks.

It would be useful to have a script that could be run over a user / group / challenge / task document (a separate script for each kind) to check that all its attributes were correct and complete and that it didn't contain any attributes that shouldn't exist. The check should be recursive (testing nested attributes all the way down).

The schemas are defined in the files in https://github.com/HabitRPG/habitrpg/blob/develop/website/server/models/

The script should read the schemas from those files, rather than having them duplicated within the script. Alternatively, instead of a stand-alone script, perhaps a method could be added to each of those models files.

If anyone has any ideas for how this would best be done, please post here.

[Hus274]

After doing a little research and looking into the code, I've found that it looks like this is something that Mongoose at least already partially provides. I'm looking into using this more. Not quite sure how expansive it is as the majority of the mongoose documentation is for version 4.x and from what I can tell, habitica is using 2.15. I'll dig into this a little further.

[Hus274]

Here's a sample output from the built in validation:

Challenge Object

{ official: false,
  tasksOrder: { habits: [], dailys: [], todos: [], rewards: [] },
  memberCount: 1,
  prize: 0,
  _id: '2869f67c-1eb9-46e1-a951-0f7955e543aa',
  createdAt: Tue Jun 14 2016 17:40:48 GMT-0700 (PDT),
  updatedAt: Tue Jun 14 2016 17:40:48 GMT-0700 (PDT),
  shortName: 'hi' }

Validation output

{ [ValidationError: Challenge validation failed]
  message: 'Challenge validation failed',
  name: 'ValidationError',
  errors: 
   { group: 
      { [ValidatorError: Path `group` is required.]
        message: 'Path `group` is required.',
        name: 'ValidatorError',
        properties: [Object],
        kind: 'required',
        path: 'group',
        value: undefined },
     leader: 
      { [ValidatorError: Path `leader` is required.]
        message: 'Path `leader` is required.',
        name: 'ValidatorError',
        properties: [Object],
        kind: 'required',
        path: 'leader',
        value: undefined },
     name: 
      { [ValidatorError: Path `name` is required.]
        message: 'Path `name` is required.',
        name: 'ValidatorError',
        properties: [Object],
        kind: 'required',
        path: 'name',
        value: undefined },
     shortName: 
      { [ValidatorError: Path `shortName` (`hi`) is shorter than the minimum allowed length (3).]
        message: 'Path `shortName` (`hi`) is shorter than the minimum allowed length (3).',
        name: 'ValidatorError',
        properties: [Object],
        kind: 'minlength',
        path: 'shortName',
        value: 'hi' } } }

This is easily given by calling verifySync() on the schema object.

[Hus274]

Validated that this works for nested objects in the schema.

[Hus274]

@Alys This looks like what you were asking for, what do you think?

[crookedneighbor]

What makes you think we use Mongoose 2? We're definitely using v4

[Hus274]

oh, my mistake. I was trying to find the version number to look for the documentation and when I did npm -v mongoose it gave me 2.15.0. I'm not quite sure what's going on there then.

[Hus274]

Ahh I think that's giving me the version of npm overall, not the module. That's my bad.

[crookedneighbor]

The real issue here is that calling any of the validation methods on every object one at a time will be extremely slow :/

So the script would need to be written in such a way that it batches the calls. We'd also probably want to run this on a prod backup, rather than prod itself and log the ids of the documents that don't match the schema.

[Alys]

I'd also like to be able to run it manually on a specific user (or group, challenge, task). For example, a user reports that they can't use the mobile app, so we run the validation on their account to see if there's anything corrupted.

@Hus274 Nice work on finding the mongoose validation method - that's looking much easier than I thought it would be!

[paglias]

Mongoose also strips any field that is not defined in the schema before saving so we should be mostly fine. The problem we had a few days ago is probably due to some old v2 code that'll be retired anyway the next month. After it we can run a one time migration to fix anything broken but apart from that all the necessary fields should already be validated and sanitized

[librarianmage]

What is the progress on this?

[Hus274]

Based on paglias's last comment I wasn't really sure how to proceed. Feel free to handle this is you have any ideas.

[Alys]

I would love for this to still be done. I've been troubleshooting a cron bug for a user who had this in one of his Habitis:

{
    "type": "habit",
    "tags": [
        null,
        "b3cdafac-f278-467b-ae62-8ee73a9170fc",
        "d6e5d418-a379-49e9-9c48-28a3194a0dfd"
    ],

The incorrect tag entry (null) was causing cron to fail part way through, resulting in no quest progress every day, and it took me a while to track down (my fault, I should have gone straight to logs instead of assuming it was just a persistent case of https://github.com/HabitRPG/habitica/issues/7727 )

If we had a validation script that we could run over any user and their tasks when an unusual problem is reported, it would me troubleshooting much faster.

It could also help with future bugs that might introduce problems that aren't prevented by mongoose, and could help organisations that run their own installs of Habitica.

However, it's low priority since it would be needed rarely and we do have logs to fall back on.

[Hus274]

The tag field in the schema has a isUUID validator on it. It seems strange to me that it would allow null. Did you try running validate on that object?

[Alys]

@Hus274 No, I didn't bother. I figured that since the data was in the database, the validation had already failed to notice it.

[Hus274]

Ok, I was asking because I was simply curious if it would fail validation. If it did, it seems like that would be a good indicator that there's a bug in how we interact with the database that skips the validation somewhere or that it occurred through a manual db update (which I'm not sure happen).

[paglias]

It's possible the validation is set to allow null values

[Hus274]

So from my understanding in the tag schema file if we add required: true then this could have been prevented?

@Alys my concern with writing a specific validation method outside of mongoose is it's simply duplication of an already existing feature and also prone to bugs like this. It would be hard to manually rewrite a validation framework and input and maintain all the expected values for that when we already have a schema that is supposed to be doing that work.

[paglias]

@Hus274 the tag schema is used in user.tags not in task.tags where only the id is stored

[Hus274]

Ahh I'm sorry, you're totally right. I'm still curious about the same question though, would putting the required: true here have prevented the issue?

[paglias]

@Hus274 I'm not sure... I don't remember exactly Mongoose's validation flow. I think the best thing to do is to tweak the code locally to test it

[Hus274]

Ok, I can definitely look into that. I'll update here with what I find!

[Hus274]

So from what I can see locally, I am unable to save a null tag to the database when using mongoose. When I tried to save the input @Alys provided above I got the following error:

     todo validation failed
  ValidationError: Invalid uuid.
      at MongooseError.ValidationError (node_modules/mongoose/lib/error/validation.js:22:11)
      at model.Document.invalidate (node_modules/mongoose/lib/document.js:1427:32)
      at node_modules/mongoose/lib/document.js:1303:17
      at validate (node_modules/mongoose/lib/schematype.js:705:7)
      at node_modules/mongoose/lib/schematype.js:742:9
      at Array.forEach (native)
      at SchemaString.SchemaType.doValidate (node_modules/mongoose/lib/schematype.js:710:19)
      at node_modules/mongoose/lib/document.js:1301:9
      at _combinedTickCallback (internal/process/next_tick.js:67:7)
      at process._tickDomainCallback (internal/process/next_tick.js:122:9)

And when I ran mongoose's validation method on a task with null in it's tags array, I received the following output:

{ ValidationError: Invalid uuid.
    at MongooseError.ValidationError (/Users/husmant/habitrpg/node_modules/mongoose/lib/error/validation.js:22:11)
    at model.Document.invalidate (/Users/husmant/habitrpg/node_modules/mongoose/lib/document.js:1427:32)
    at /Users/husmant/habitrpg/node_modules/mongoose/lib/document.js:1374:13
    at Array.forEach (native)
    at model.Document.validateSync (/Users/husmant/habitrpg/node_modules/mongoose/lib/document.js:1356:9)
    at _callee8$ (/Users/husmant/habitrpg/test/api/v3/unit/models/task.test.js:132:31)
    at tryCatch (/Users/husmant/habitrpg/node_modules/regenerator-runtime/runtime.js:62:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/Users/husmant/habitrpg/node_modules/regenerator-runtime/runtime.js:336:22)
    at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/Users/husmant/habitrpg/node_modules/regenerator-runtime/runtime.js:95:21)
    at GeneratorFunctionPrototype.tryCatcher (/Users/husmant/habitrpg/node_modules/bluebird/js/release/util.js:16:23)
    at PromiseSpawn._promiseFulfilled (/Users/husmant/habitrpg/node_modules/bluebird/js/release/generators.js:97:49)
    at Promise._settlePromise (/Users/husmant/habitrpg/node_modules/bluebird/js/release/promise.js:572:26)
    at Promise._settlePromise0 (/Users/husmant/habitrpg/node_modules/bluebird/js/release/promise.js:612:10)
    at Promise._settlePromises (/Users/husmant/habitrpg/node_modules/bluebird/js/release/promise.js:691:18)
    at Async._drainQueue (/Users/husmant/habitrpg/node_modules/bluebird/js/release/async.js:138:16)
    at Async._drainQueues (/Users/husmant/habitrpg/node_modules/bluebird/js/release/async.js:148:10)
    at Immediate.Async.drainQueues (/Users/husmant/habitrpg/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:574:20)
    at tryOnImmediate (timers.js:554:5)
    at processImmediate [as _immediateCallback] (timers.js:533:5)
  message: 'todo validation failed',
  name: 'ValidationError',
  errors:
   { 'tags.0':
      { Invalid uuid.
          at MongooseError.ValidatorError (/Users/husmant/habitrpg/node_modules/mongoose/lib/error/validator.js:24:11)
          at validate (/Users/husmant/habitrpg/node_modules/mongoose/lib/schematype.js:775:13)
          at /Users/husmant/habitrpg/node_modules/mongoose/lib/schematype.js:799:9
          at Array.forEach (native)
          at SchemaString.SchemaType.doValidateSync (/Users/husmant/habitrpg/node_modules/mongoose/lib/schematype.js:784:19)
          at /Users/husmant/habitrpg/node_modules/mongoose/lib/document.js:1372:17
          at Array.forEach (native)
          at model.Document.validateSync (/Users/husmant/habitrpg/node_modules/mongoose/lib/document.js:1356:9)
          at _callee8$ (/Users/husmant/habitrpg/test/api/v3/unit/models/task.test.js:132:31)
          at tryCatch (/Users/husmant/habitrpg/node_modules/regenerator-runtime/runtime.js:62:40)
          at GeneratorFunctionPrototype.invoke [as _invoke] (/Users/husmant/habitrpg/node_modules/regenerator-runtime/runtime.js:336:22)
          at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/Users/husmant/habitrpg/node_modules/regenerator-runtime/runtime.js:95:21)
          at GeneratorFunctionPrototype.tryCatcher (/Users/husmant/habitrpg/node_modules/bluebird/js/release/util.js:16:23)
          at PromiseSpawn._promiseFulfilled (/Users/husmant/habitrpg/node_modules/bluebird/js/release/generators.js:97:49)
          at Promise._settlePromise (/Users/husmant/habitrpg/node_modules/bluebird/js/release/promise.js:572:26)
          at Promise._settlePromise0 (/Users/husmant/habitrpg/node_modules/bluebird/js/release/promise.js:612:10)
          at Promise._settlePromises (/Users/husmant/habitrpg/node_modules/bluebird/js/release/promise.js:691:18)
          at Async._drainQueue (/Users/husmant/habitrpg/node_modules/bluebird/js/release/async.js:138:16)
          at Async._drainQueues (/Users/husmant/habitrpg/node_modules/bluebird/js/release/async.js:148:10)
          at Immediate.Async.drainQueues (/Users/husmant/habitrpg/node_modules/bluebird/js/release/async.js:17:14)
          at runCallback (timers.js:574:20)
          at tryOnImmediate (timers.js:554:5)
          at processImmediate [as _immediateCallback] (timers.js:533:5)
        message: 'Invalid uuid.',
        name: 'ValidatorError',
        properties: [Object],
        kind: 'user defined',
        path: 'tags',
        value: null } } }

[Alys]

I'm closing this. I'll assume that all bad data was added to the database before our current validations were added and that I can therefore use mongoose validation to find it. I'll reopen it or make a new issue if I find that the validation isn't enough.