Skip to content

Allow read-access to protectedFields based on user for custom classes #5887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions spec/ParseACL.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -931,4 +931,143 @@ describe('Parse.ACL', () => {

rest.create(config, auth.nobody(config), '_User', anonUser);
});

it('acl an object with read access by one user allows reading protectedFields by the same user', async done => {
// Add a protected field to the TestObject class
await reconfigureServer({
protectedFields: {
TestObject: {
'*': ['protectedInfo'],
},
},
});

// Create an object readable by Alice.
const user = new Parse.User();
user.set('username', 'alice');
user.set('password', 'wonderland');
await user.signUp();

const object = new TestObject();
object.set({
protectedInfo: 'IAmASecret',
});
const acl = new Parse.ACL();
acl.setReadAccess(user.id, true);
object.setACL(acl);
await object.save();

// Check if the logged in user can read the protected fields
const query = new Parse.Query(TestObject);
query.equalTo('objectId', object.id);
const objectRefetched = await query.find();
equal(objectRefetched.length, 1);
equal(objectRefetched[0].get('protectedInfo'), 'IAmASecret');
done();
});

it('acl an object with read access by one user disallows reading protectedFields by another user', async done => {
// Add a protected field to the TestObject class
await reconfigureServer({
protectedFields: {
TestObject: {
'*': ['protectedInfo'],
},
},
});

// Create an object readable by Alice.
let user = new Parse.User();
user.set('username', 'alice');
user.set('password', 'wonderland');
await user.signUp();

const object = new TestObject();
object.set({
protectedInfo: 'IAmASecret',
});
const acl = new Parse.ACL();
acl.setPublicReadAccess(true);
acl.setReadAccess(user.id, true);
object.setACL(acl);
await object.save();
await Parse.User.logOut();

// Signin as an user without ACL read access
user = new Parse.User();
user.set('username', 'alice2');
user.set('password', 'wonderland2');
await user.signUp();

// Check if the logged in user can read the protected fields
const query = new Parse.Query(TestObject);
query.equalTo('objectId', object.id);
const objectRefetched = await query.find();
equal(objectRefetched.length, 1);
equal(objectRefetched[0].get('protectedInfo'), undefined);
done();
});

it('acl an object with public access and an singedIn user does not allow reading protectedFields', async done => {
// Add a protected field to the TestObject class
await reconfigureServer({
protectedFields: {
TestObject: {
'*': ['protectedInfo'],
},
},
});

// Create a dummy user
const user = new Parse.User();
user.set('username', 'alice');
user.set('password', 'wonderland');
await user.signUp();

const object = new TestObject();
object.set({
protectedInfo: 'IAmASecret',
});
const acl = new Parse.ACL();
acl.setPublicReadAccess(true);
object.setACL(acl);
await object.save();

// Check if the dummy user can read the protected fields
const query = new Parse.Query(TestObject);
query.equalTo('objectId', object.id);
const objectRefetched = await query.find();
equal(objectRefetched.length, 1);
equal(objectRefetched[0].get('protectedInfo'), undefined);
done();
});

it('acl an object with public access does not allow reading protectedFields', async done => {
// Add a protected field to the TestObject class
await reconfigureServer({
protectedFields: {
TestObject: {
'*': ['protectedInfo'],
},
},
});

// Create an object with public read access
const object = new TestObject();
object.set({
protectedInfo: 'IAmASecret',
});
const acl = new Parse.ACL();
acl.setPublicReadAccess(true);
object.setACL(acl);
await object.save();

// Check if the oublic fields can be read publicly
const query = new Parse.Query(TestObject);
query.equalTo('objectId', object.id);
const objectRefetched = await query.find();
equal(objectRefetched.length, 1);
equal(objectRefetched[0].get('protectedInfo'), undefined);
done();
});
});
24 changes: 21 additions & 3 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,24 @@ const filterSensitiveData = (
protectedFields,
object
) => {
protectedFields && protectedFields.forEach(k => delete object[k]);
// Get all users with read access
if (protectedFields && protectedFields.length > 0) {
const userACLs = aclGroup.filter(acl => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you just go through the object ACLs and check if there is a rule with the user id and read true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand the aclGroup contains all roles associated with the current user and the user id. aclGroup is not the object's ACL. So I first use the aclGroup to retrieve the current user's id. Then I use the actual object to check if the user id has read permission using getReadAccess.

If there is a better way to retrieve the current user's id (instead of using the acl group) I'll gladly use it.

The aclGroup approach is used in the addPointerPermissions method too:

const userACL = aclGroup.filter(acl => {
return acl.indexOf('role:') != 0 && acl != '*';
});
// the ACL should have exactly 1 user
if (perms && perms[field] && perms[field].length > 0) {
// No user set return undefined
// If the length is > 1, that means we didn't de-dupe users correctly
if (userACL.length != 1) {
return;
}
const userId = userACL[0];
const userPointer = {
__type: 'Pointer',
className: '_User',
objectId: userId,
};

Line 202 avoids the whole retrieval of the user id if there are no protected fields to remove.

return acl.indexOf('role:') != 0 && acl != '*';
});

var objectCopyWithClassName = {};
Object.assign(objectCopyWithClassName, object);
objectCopyWithClassName.className = className;

if (
userACLs.length !== 1 ||
!Parse.Object.fromJSON(objectCopyWithClassName)
.getACL()
.getReadAccess(userACLs[0])
)
protectedFields.forEach(k => delete object[k]);
}

if (className !== '_User') {
return object;
Expand Down Expand Up @@ -1315,8 +1332,9 @@ class DatabaseController {
query,
aclGroup
);
// ProtectedFields is generated before executing the query so we
// can optimize the query using Mongo Projection at a later stage.
// Don't use a Mongo Projection for removing the protectedFields from the query.
// The retrieval of the data could be allowed to the user through whitelisting by
// a read ACL. This is determined after querying.
protectedFields = this.addProtectedFields(
schemaController,
className,
Expand Down