Remove hidden properties from aggregate responses #4351
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4351 +/- ##
==========================================
- Coverage 92.74% 92.73% -0.02%
==========================================
Files 119 119
Lines 8438 8448 +10
==========================================
+ Hits 7826 7834 +8
- Misses 612 614 +2
Continue to review full report at Codecov.
|
spec/ParseQuery.Aggregate.spec.js
Outdated
|
|
||
| expect(result.objectId).not.toBe(undefined); | ||
| expect(result.username).toBe(username); | ||
| expect(result.score).toBe(score); |
There was a problem hiding this comment.
can we check that result.updatedAt and result.createdAt are set, as well as ACL?
There was a problem hiding this comment.
I don't think they are set. Should they be set?
There was a problem hiding this comment.
We should transform transform the results to the proper parse format otherwise the clients ain't gonna like it .
There was a problem hiding this comment.
@flovilmart , all 3 of those are actually returned, but in their direct mongodb forms under their original keys _acl, _created_at and _updated_at. Now that I think of it there should be a transform function somewhere in the mongodb transform or adapter that would move them to the expected keys. I'll look for that and add it into the PR here.
|
@flovilmart changes have been made. Just utilized the standard transforms for results returned in both mongodb's & postgres's adapters. |
|
Looking good! |
* Remove hidden properties from aggregrate responses * transform results from mongo & postgres * Adjust ordering to comply with tests
Noticed that the newly added aggregate endpoint leaks hidden properties. This could be particularly problematic if this endpoint is used with the _User class, where particularly sensitive information is leaked.
This utilizes the same screening function that's used in the UsersRouter (removeHiddenProperties) to remove any hidden properties as the last step before the response is sent. That feels like it should be factored out into a separate utils file, but the priority is in making sure this issue is addressed first.
We'll want this (or an equivalent solution) in place before we create any releases containing the aggregate code.