Make trigger context accessible in save

[mtrezza]

Is your feature request related to a problem? Please describe.
Sometimes an object should be saved with custom execution in its triggers (e.g. afterSave).

Describe the solution you'd like
It should be possible to pass arguments from the save call to the triggers. There is already a context object that is passed from beforeSave to afterSave. It would be practical to make the context accessible in the save call as an option, e.g.:

object.save(null, {context: {myArg: true}});

Parse.Cloud.beforeSave('MyClass', req => {
    let myArg = req.context.myArg;
});

TODOs:

• Changes in JS SDK add context to Parse.Object.save Parse-SDK-JS#1150
• Changes in Parse Server add context to Parse.Object.save #6626
• Changes in docs Add context to object save in JS SDK docs#730

[jonas-db]

+1. This is a solution to #6512.

[mtrezza]

@jonas-db Good to see that there is demand for such a feature. That gave me some motivation to pick it up again 🙂

[mtrezza]

Added PR for JS SDK parse-community/Parse-SDK-JS#1150

[mtrezza]

Added PR for Parse Server #6626

[mtrezza]

Added PR in docs parse-community/docs#730

[mtrezza]

The intended use case is passing a parameter from Parse.Object.save() to beforeSave / afterSave triggers within Cloud Code.

The proposed change in parse-community/Parse-SDK-JS#1150 exposes the context also to clients outside of Cloud Code. Exposure is unidirectional, i.e. a parameter can only be passed to the trigger context, but any change of context in Cloud Code is not transmitted back to the client. However, it may be a potential security issue.

Proposed solutions:

• Parse Server only accepts context from JS SDK with master key
• Context is only exposed for Parse.Object.save()within Cloud Code

Any thoughts?

[acinader]

@mtrezza can you think of any security concerns?

[mtrezza]

Context is used for internal purposes and the PR is changing exposure of the existing context.
A developer could have implemented code assuming that context is only internal.

Parse.Cloud.afterSave("MyClass", req => {
    const allowed = req.context.allowed;

    if (!allowed) {
      throw new Error("error: allowed==false");
    }
});

By guessing (or by an obvious error message) someone could manipulate the internal context from client side. Or am I being too creative here?

[jonas-db]

I agree this is some form of implicit taint flow. My original idea was to only expose it in the cloud code, and not on the client since any client can inject anything it. (but again, maybe this is the same as parameters in cloud functions, not sure if there is any security built around that?)

[mtrezza]

@jonas-db Good comparison. I think if we decide to expose the context to clients we should explicitly mention that

• in the release nodes, with a visibility similar to a breaking change
• in the parse docs of the context as a warning

_{Sent with GitHawk}

[mtrezza]

No contrary voices, so I assume we'll expose context to the clients. I also just stumbled across #6323 which gives a use case for making the context accessible to clients.

@acinader It would be great if the next release of parse server could be coordinated with the next JS SDK release to coherently introduce this feature. The JS SDK PR has already been merged.

The test cases in #6626 should pass with the next JS SDK release.

[jonas-db]

Just a follow-up since you linked the tracking use case. Maybe, it can be extended to other operations such as find, delete, etc. since these also have triggers and might need a context.
(although i'm unaware of the complexity, the context in save operations is currently sufficient enough for me)

[mtrezza]

@jonas-db Sounds good! Would you be willing to open a PR for this? I assume it is relatively easy as it's just extending existing routes. This PR gives a good blueprint with the save method. Once you've figured out how to do it with one method, it's mostly replicating the same change for the other methods.

[jonas-db]

@mtrezza Sorry, I won't have time right now and the context for "save" is sufficient. However, I will review my project in a few months and will be happy to submit a PR for this :)

[yog27ray]

I have created the PR for extending the context to following webhooks.

1. beforeDelete
2. afterDelete
3. beforeFind
4. Cloud Function

#6666

parse-community/Parse-SDK-JS#1159

[cbaker6]

If I’m understanding the discussion here correctly, context can be sent from the client to inform CloudCode how to save, find, delete; it’s never actually passed to the storage adapter correct?

If so, I think this is extremely useful. For example, if there are multiple versions of an PFObject stored on a parse-server, there are typically two types of query/finds needed:

1. Find all objects would give all versions of it. In beforeFind, context = {findAll:true} will perform a standard query, afterSave do nothing.
2. Find latest objects would give latest version of each object only. beforeFind, same as above, but in afterFind, context = {findAll:false}, so filter latest versions of each object and return those to client

Would context handle this type of scenario?

Also, I’m assuming context would need to be added to PFObject to the parse frameworks of iOS, Android, etc.

As far as security, my initial thought is there aren’t many if any pass whatever security flaws are currently part of the current parse server/client. If a malicious person can manipulate context, they can probably manipulate PFObjects directly, so they can probably find, add, delete already. I can see some mistakes happening on the CloudCode side from the service provider if they setup their CloudCode incorrectly, but I think this will always be the case. If CloudCode doesn’t use context, it simply just operate as standard

[mtrezza]

@yog27ray

I have created the PR for extending the context to following webhooks.

Great job! Can you please open a new issue for these PRs as this issue will be closed with the next release of parse server. And can you please add a PR for the docs if necessary, that would put a smile on @TomWFox's face.

@cbaker6

context (...) it’s never actually passed to the storage adapter

Correct.

if there are multiple versions of an PFObject stored on a parse-server

I don't understand what "multiple versions" means in this context so I can't follow your use case. context is an ephemeral dictionary that is accessible during Cloud Code operations, it is not part of Parse.Object. I think the example in #6459 (comment) describes it well.

[cbaker6]

context is an ephemeral dictionary that is accessible during Cloud Code operations, it is not part of Parse.Object. I think the example in #6459 (comment) describes it well.

@mtrezza How would you pass context from the client? My assumption is that for find, it would be through PFQuery which I believe is related to PFObject

I don't understand what "multiple versions" means in this context so I can't follow your use case.

The use case I mentioned is a custom one. Typically parse keeps the current state of PFObjects in Postgres/Mongo. When a PFObject is updated, the state of that object is updated (and loses previous state) and therefore queried, modified, or deleted. I presented a use-case of maintaining versioned data in parse, by having a uuid field and doing 1-to-many mapping between uuid and related (other versions) of PFObjects. It's not necessary to edit parse-server itself for a case like this, but if context can be passed from the client, it solves my example. There may be an overhead because you query and filter as I mentioned in the example instead of modifying the query for mongo or postgres, but this is okay for me.

From the discussion and other links here, context seems to handle the use-case I presented, assuming context is passed from the client using PFQuery.

[mtrezza]

PFQuery which I believe is related to PFObject

They are related like two cousins rather than parent and child :) The context is passed along with the REST command, that is inside Parse.Object as a private property (for lack of a better way), but only to be removed from the object again upon arrival at Parse Server. You can take a look at the already merged PRs for details.

it solves my example

Glad to hear that.

[cbaker6]

I’m thinking of sending context using parse for iOS, here is why I mention sending through PFQuery. Similarly in Android. I don’t know much about web apps, but it seems the JS framework is able to leverage REST and other things in CloudCode more than the other frameworks.

[mtrezza]

Sounds good! A PR for the Parse iOS or Android SDK would be very welcome.

[marcinjakubowski]

Not sure if it's a bug, or maybe by design, but using obj.save(null, context: { a: 'a' }) from within a trigger will throw an error if obj is being updated. It will work fine if obj is a new object being created though.

So let's consider something like:

Parse.Cloud.beforeSave("Foo", async (request) => {
    console.log(request.context.a)
    request.object.set('a', request.context.a)
})

Parse.Cloud.afterSave("Bar", async (request) => {
    const foo  = new Parse.Object("Foo")
    await foo.save(null, { context: { a: '5' } });
});

This will work fine, if I add or update an object of class Bar, an object of Foo will be created and its a property will be set to 5. But if you attempt the very same save call on a retrieved Foo, it will fail:

Parse.Cloud.beforeSave("Foo", async (request) => {
    console.log(request.context.a)
    request.object.set('a', request.context.a)
})


Parse.Cloud.afterSave("Bar", async (request) => {
    const query = new Parse.Query("Foo")
    const foo = await query.first()
    await foo.save(null, { context: { a: '10' } }); // fails
});

(an object of Foo exists of course, at this point)

The error is one of the two, I'm not sure what determines which one gets called:

error: Parse error: Invalid field name for update: _context 
{"code":105,"stack":"
Error: Invalid field name for update: _context
    at /parse-server/lib/Controllers/DatabaseController.js:545:21
    at Array.forEach (<anonymous>) 
    at /parse-server/lib/Controllers/DatabaseController.js:537:31 
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
"}

or, alternatively:

error: Parse error: Invalid field name: _context.
{"code":105,"stack":"
Error: Invalid field name: _context.
    at SchemaController.enforceFieldExists (/parse-server/lib/Controllers/SchemaController.js:1128:13)
    at SchemaController.validateObject (/parse-server/lib/Controllers/SchemaController.js:1301:28)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
"}

Not sure if I should be opening a new issue if this one's not being closed, so I'm posting here.

[davbeck]

I'm trying to use this with the latest JS SDK and the master version of server and I'm getting the following error:

Parse error: Invalid field name for update: _context

Using this to update a user:

await user.save(null, {useMasterKey: true, context: {skipSync: true}})

[davbeck]

From what I can tell, the context is only handled for new objects because it is handled inside of the !query if statement.

[marcinjakubowski]

Yes, moving it out of the if (!query) in RestWrite.js made it work fine for me.

[davbeck]

Additionally, it appears it does not get passed in correctly if you have related objects being saved.

user.get('profile').set('displayName', 'foobar')
user.save(null, {context: {skipSync: true}})

[mtrezza]

Thanks, well spotted, I'm looking into it.

Additionally, it appears it does not get passed in correctly if you have related objects being saved.

@davbeck Do I understand this correctly that the field profile points to an object in class Profile and you don't see the context passed into the triggers for the Profile class?

[davbeck]

It is a pointer to a Profile object. Surprisingly, I don't get the context object on either of the hooks. Remove the pointer update and the user hook starts getting context again.

[mtrezza]

Since these issues only occur in specific use cases and do not impede the basic functionality of this feature, I have opened a new issue #6736 to track them.

[dplewis]

@mtrezza Sorry for the late reply just getting caught up on the conversations. Will review shortly.

[mtrezza]

With the docs PR merged (see above), feature is complete.

[matheusfrozzi]

Guys, one question, why in the afterFind the context is not available? because sometimes I have to change the result in afterFind, but I can't send anything to afterFind to diff the results. There's another way to pass data between beforeFind and afterFind, or include context is the better way?

[mtrezza]

From what I can tell, the context has not been made accessible in the afterFind trigger.

You can look at #6666 which extended this PR to make context available in beforeFind. It should be a good blueprint to add the context to afterFind. Do you want to open a PR?

[matheusfrozzi]

For sure, I never changed anything on parse server code, but I would love to try.

From what I understand of parse server, to make context accessible in afterFind, needs to change in this file?
https://github.com/dblythy/parse-server/blob/7c87d127bf3c10bec29d02f8fe3e07980f3b5507/src/triggers.js

Something like:
if (
triggerType === Types.beforeSave ||
triggerType === Types.afterSave ||
triggerType === Types.beforeDelete ||
triggerType === Types.afterDelete ||
// add this
triggerType === Types.afterFind
) {
// Set a copy of the context on the request object.
request.context = Object.assign({}, context);
}

and in function declaration, add the context var
export function maybeRunAfterFindTrigger(
triggerType,
auth,
className,
objects,
config,
query,
context // add this
)

Something like that? Thank's for your reply

[mtrezza]

For sure, I never changed anything on parse server code, but I would love to try.

Great! Welcome to the contributor's league 😉 I suggest you take a look at the contribution guide where you can also find useful infos, for example about how to set up a local development environment for Parse Server.

Something like that?

I suggest you take a look at the changes in #6666. In spec/CloudCode.spec.js you find an example of the beforeFind trigger. You could for example debug the test case and go step-by-step to understand the code path for the beforeFind trigger. If there is already a context object that is passed between beforeFind and afterFind, you may actually have discovered a bug instead. Happy investigating and if you need any guidance, please let us know.

[matheusfrozzi]

ok! Thank you again, I'll do that.