Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion spec/ParseRole.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Parse Role testing', () => {
}).then((x) => {
x.set('foo', 'baz');
// This should fail:
return x.save();
return x.save({},{sessionToken: ""});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I change this because, by default the sessionToken is passed. The result of this never fail.

}).then((x) => {
fail('Should not have been able to save.');
}, (e) => {
Expand Down
2 changes: 1 addition & 1 deletion src/Auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Auth.prototype.getUserRoles = function() {
return Promise.resolve(this.userRoles);
}
if (this.rolePromise) {
return rolePromise;
return this.rolePromise;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}
this.rolePromise = this._loadRoles();
return this.rolePromise;
Expand Down
29 changes: 19 additions & 10 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ function RestWrite(config, auth, className, query, data, originalData) {
this.auth = auth;
this.className = className;
this.storage = {};
this.runOptions = {
acl:['*']
Copy link
Contributor

Choose a reason for hiding this comment

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

is it safe to mark as '*' by default?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, the acl options shouldn't be set by default. A master key query shouldn't have an acl key set, for example.. See ExportAdapter.update

};

if (!query && data.objectId) {
throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, 'objectId ' +
Expand Down Expand Up @@ -66,6 +69,8 @@ function RestWrite(config, auth, className, query, data, originalData) {
// status and location are optional.
RestWrite.prototype.execute = function() {
return Promise.resolve().then(() => {
return this.getUserAndRoleACL();
}).then(() => {
return this.validateSchema();
}).then(() => {
return this.handleInstallation();
Expand All @@ -88,6 +93,18 @@ RestWrite.prototype.execute = function() {
});
};

// Uses the Auth object to get the list of roles, adds the user id
RestWrite.prototype.getUserAndRoleACL = function() {
if (this.auth.isMaster || !this.auth.user) {
return Promise.resolve();
}
return this.auth.getUserRoles().then((roles) => {
roles.push(this.auth.user.id);
this.runOptions.acl = this.runOptions.acl.concat(roles);
return Promise.resolve();
});
};

// Validates this operation against the schema.
RestWrite.prototype.validateSchema = function() {
return this.config.database.validateObject(this.className, this.data);
Expand Down Expand Up @@ -644,24 +661,16 @@ RestWrite.prototype.runDatabaseOperation = function() {
throw new Parse.Error(Parse.Error.INVALID_ACL, 'Invalid ACL.');
}

var options = {};
if (!this.auth.isMaster) {
options.acl = ['*'];
if (this.auth.user) {
options.acl.push(this.auth.user.id);
}
}

if (this.query) {
// Run an update
return this.config.database.update(
this.className, this.query, this.data, options).then((resp) => {
this.className, this.query, this.data, this.runOptions).then((resp) => {
this.response = resp;
this.response.updatedAt = this.updatedAt;
});
} else {
// Run a create
return this.config.database.create(this.className, this.data, options)
return this.config.database.create(this.className, this.data, this.runOptions)
.then(() => {
var resp = {
objectId: this.data.objectId,
Expand Down
7 changes: 7 additions & 0 deletions src/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,19 @@ function del(config, auth, className, objectId) {
});
}
return Promise.resolve({});
}).then(() => {
if (!auth.isMaster) {
return auth.getUserRoles();
Copy link
Contributor

Choose a reason for hiding this comment

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

we could add an auth.resolveACL() that would return a proper ACL query object (into a promise result) given the current state of the auth.
That would remove the duplication of concatenating with userRoles at both places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flovilmart do you mean the auth.resolveACL will ask the server to get the new state of the user ?
What does not do getUserRoles by using a cache if already fetched.

Sorry but my English is not perfect :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean a method on auth like:

function queryACLConstraint() {
// do something
return {}; // the ACL Constraint for the query
}

This inspired by the way we build the ACL options around the app, this way we centralize that logic.

}else{
return Promise.resolve();
}
}).then(() => {
var options = {};
if (!auth.isMaster) {
options.acl = ['*'];
if (auth.user) {
options.acl.push(auth.user.id);
options.acl = options.acl.concat(auth.userRoles);
}
}

Expand Down