Allowing restricted field names

[pausenabre]

New Issue Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest version of Parse Server.

Issue Description

We updated Parse Server to version 4.5.0, and included in the latest security changes, now we can not use certain restricted fields like lengthor className as this merged PR explains: #7053

We have a class in production since Parse Server 3.0.0 that has been using the newly restricted field length so now we can not create or edit objects of that class as Parse returns an error: {Invalid field name: length., code=105}

As a workaround, we've tried to unset the length key in a beforeSave hook of that class. That works great for any object which is newly created, but not for object updates (tested with REST API POST and PUT). When updating an object, the beforeSave hook is not called, and Parse Server returns an error before we can unset the key.

This issue attempts to open a discussion about how to work this concept out. As suggested by @mtrezza we could allow the "reserved" field names by encoding fields differently in the Parse Object. #7053 (comment)

Steps to reproduce

Create a class in Parse Server 4.4.1 (or anything lower than 4.5.0) with the field length.
Create some objects for that class.
Update Parse Server to (4.5.0).
Attempt editing those objects or creating new ones by REST API POST and PUT, or from Parse dashboard.

Actual Outcome

The object can not be created or edited: {Invalid field name: length., code=105}

Expected Outcome

The object is successfully created or edited.

Environment

Server

• Parse Server version: 4.5.0
• Operating system: Linux
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): Heroku

Database

• System (MongoDB or Postgres): MongoDB
• Database version: 4.4.3
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): mLab

Client

• SDK (iOS, Android, JavaScript, PHP, Unity, etc): iOS
• SDK version: 1.19.0

Logs

From iOS: Error Domain=Parse Code=105 "Invalid field name: length." UserInfo={error=Invalid field name: length., NSLocalizedDescription=Invalid field name: length., code=105}

[mtrezza]

Thanks for opening this issue.

I support this discussion because it seems best practice to not make the developer having to consider Parse Server's internal keys.

Do you want to look into what is causing the current restriction of the length field (and possibly other fields) and propose a possible concept?

[julianvogels]

We just had a look at the Parse Server code and unfortunately, this is a bit over our heads. The SchemaController's invalidColumns array contains length, so that's the place where we started, but we got lost in the many places the SchemaController is called.

I think a good fix for us would be to ensure that the beforeSave hook is called before the error 105: Invalid field name: length is thrown, so we have a chance to unset the key when the client saves an object with it. This works ok on creation, but not when saving an object that already exists. In the latter case, the error is thrown too early.

Maybe someone has knowledge about the order in which errors and beforeSave hooks are executed and where? A push in the right direction could help us figure out where to make a change. We'd try out best 🤞

[davimacedo]

Are you sure the hook is not being called before the error is thrown? Would you mind to write a failing test case for this scenario?

[julianvogels]

Ok. How?

[mtrezza]

You can take a look at the basic beforeSave tests on how to write such a test:

parse-server/spec/CloudCode.spec.js

Lines 75 to 91 in d47891f

	it('basic beforeSave rejection', function (done) {
	Parse.Cloud.beforeSave('BeforeSaveFail', function () {
	throw new Error('You shall not pass!');
	});
	const obj = new Parse.Object('BeforeSaveFail');
	obj.set('foo', 'bar');
	obj.save().then(
	() => {
	fail('Should not have been able to save BeforeSaveFailure class.');
	done();
	},
	() => {
	done();
	}
	);
	});

Side note, most tests are in the old chained promise syntax, for new tests we should use await/async syntax.

[mtrezza]

One way to separate Parse.Object properties from common JavaScript object properties may to be encapsulate the Parse.Object properties using Symbols.

For example, a ParseObject can have a symbol length without interfering with the common JavaScript object property length. All symbols can be retrieved with ParseObject.getOwnPropertySymbols().

Another, more platform-universal way, may be to just add the properties to an encapsulating super-property of ParseObject.

[therealjmj]

FWIW, I'm attempting to fix this with the following job, which replaces my "length" property. On my end "length" represented a length of time in minutes, so I'm replacing with "lengthInMin". Have to run this job on downgraded Parse Server before making the full upgrade to 4.5. (It's currently running for 200k objects..)

`Parse.Cloud.job('replaceLengthForAllObjects', async request => {

    const objectQuery = new Parse.Query('ClassName');
    objectQuery.exists('length');
    console.log(await objectQuery.count({useMasterKey: true}));

    let count = 0;
    await objectQuery.each(async item => {

        console.log('--')
        console.log(count++)

        const lengthInMin = item.get('length');
        item.set('lengthInMin', lengthInMin);
        item.unset('length');
        item.save({}, {useMasterKey: true});

    }, {useMasterKey: true})
});`

[mtrezza]

You could actually do that with a script directly on the MongoDB server, with a fraction of resources, because you don't need the Parse overhead.