Fix wrong person being unbanned when unbanning a Matrix user

Fix wrong person being unbanned when unbanning a Matrix user

Fix #2848 (closed)

Problem: What was going wrong before?

When defining multiple conditions for an array of sub-documents, the bans.$ projection produces undefined behavior with no warning 😖. In our flavor of Mongo/Mongoose, this gives the first result that matched 'bans.virtualUser.type': 'matrix',. So we end up unbanned the first Matrix user listed in the list of bans when someone tries to unban and Matrix user.

Example room data

{
    "bans" : [
            {
                    "_id" : ObjectId("62e2f7e16e8030133051e2c5"),
                    "bannedBy" : ObjectId("5f762e89986e461e663059c2"),
                    "userId" : ObjectId("5f762f95986e461e663059d1"),
                    "dateBanned" : ISODate("2022-07-28T20:56:01.641Z")
            },
            {
                    "_id" : ObjectId("62e2f7e86e8030133051e2db"),
                    "bannedBy" : ObjectId("5f762e89986e461e663059c2"),
                    "userId" : ObjectId("5fbc54e8739a8d7d060b8e84"),
                    "dateBanned" : ISODate("2022-07-28T20:56:08.191Z")
            },
            {
                    "_id" : ObjectId("62e2fae96a2deb62e0c0f004"),
                    "bannedBy" : ObjectId("5f762e89986e461e663059c2"),
                    "virtualUser" : {
                            "type" : "matrix",
                            "externalId" : "erictroupetester:my.matrix.host",
                            "_id" : ObjectId("62e2fae96a2deb62e0c0f005")
                    },
                    "dateBanned" : ISODate("2022-07-28T21:08:57.489Z")
            },
            {
                    "_id" : ObjectId("62e2faeb6a2deb62e0c0f012"),
                    "bannedBy" : ObjectId("5f762e89986e461e663059c2"),
                    "virtualUser" : {
                            "type" : "matrix",
                            "externalId" : "root:my.matrix.host",
                            "_id" : ObjectId("62e2faeb6a2deb62e0c0f013")
                    },
                    "dateBanned" : ISODate("2022-07-28T21:08:59.705Z")
            }
    ]
}

$ mongo gitter
> db.troupes.findOne({
      _id: ObjectId('619437ec81afb5b411ad44e6'),
      'bans.virtualUser.type': 'matrix',
      'bans.virtualUser.externalId': 'root:my.matrix.host'
  }, { _id: 0, 'bans.$': 1 })
// Unexpectedly finds `erictroupetester:my.matrix.host` when we asked for `root:my.matrix.host` ❌
{
        "bans" : [
                {
                        "_id" : ObjectId("62e2fae96a2deb62e0c0f004"),
                        "bannedBy" : ObjectId("5f762e89986e461e663059c2"),
                        "virtualUser" : {
                                "type" : "matrix",
                                "externalId" : "erictroupetester:my.matrix.host",
                                "_id" : ObjectId("62e2fae96a2deb62e0c0f005")
                        },
                        "dateBanned" : ISODate("2022-07-28T21:08:57.489Z")
                }
        ]
}

But the undefined behavior is documented in the Mongo docs:

$ (projection)

Array Field Limitations

The query document should only contain a single condition on the array field to which it is applied. Multiple conditions may override each other internally and lead to undefined behavior.

To specify criteria on multiple fields of documents inside that array, use the $elemMatch query operator. The following query returns the first document inside a grades array that has a mean of greater than 70 and a grade of greater than 90.

-- https://www.mongodb.com/docs/manual/reference/operator/projection/positional/#array-field-limitations

Solution

Use $elemMatch as the docs suggest:

$ mongo gitter
> db.troupes.findOne({
      _id: ObjectId('619437ec81afb5b411ad44e6'),
      bans: {
          $elemMatch: {
              'virtualUser.type': 'matrix',
              'virtualUser.externalId': 'root:my.matrix.host'
          }
      }
  }, { _id: 0, 'bans.$': 1 })
// Finds `root:my.matrix.host` as expected ✅
{
        "bans" : [
                {
                        "_id" : ObjectId("62e2faeb6a2deb62e0c0f012"),
                        "bannedBy" : ObjectId("5f762e89986e461e663059c2"),
                        "virtualUser" : {
                                "type" : "matrix",
                                "externalId" : "root:my.matrix.host",
                                "_id" : ObjectId("62e2faeb6a2deb62e0c0f013")
                        },
                        "dateBanned" : ISODate("2022-07-28T21:08:59.705Z")
                }
        ]
}

Dev notes

• Add tests