Skip to content

Fix : User Roles not added to create, update or delete calls #374

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

Merged
merged 13 commits into from
Feb 19, 2016
Merged
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
34 changes: 24 additions & 10 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function RestWrite(config, auth, className, query, data, originalData) {
this.auth = auth;
this.className = className;
this.storage = {};
this.runOptions = {};

if (!query && data.objectId) {
throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, 'objectId ' +
Expand Down Expand Up @@ -66,6 +67,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 +91,25 @@ 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) {
return Promise.resolve();
}

this.runOptions.acl = ['*'];

if( this.auth.user ){
return this.auth.getUserRoles().then((roles) => {
roles.push(this.auth.user.id);
this.runOptions.acl = this.runOptions.acl.concat(roles);
return Promise.resolve();
});
}else{
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 @@ -690,18 +712,10 @@ 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;
});
Expand All @@ -714,7 +728,7 @@ RestWrite.prototype.runDatabaseOperation = function() {
this.data.ACL = ACL;
}
// 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