From f6b7d7486ecf72ee155b6a7531256916002c6c22 Mon Sep 17 00:00:00 2001 From: Georges Jamous Date: Fri, 8 Jun 2018 14:57:59 +0300 Subject: [PATCH 1/5] ActiveTests --- spec/ParseRole.spec.js | 36 +++++++++++++++++++++++++++++ spec/support/jasmine.json | 2 +- src/Auth.js | 7 ++++++ src/Controllers/SchemaController.js | 3 ++- src/RestWrite.js | 11 +++++++-- 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index b9d315060a..b114a9fa35 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -527,4 +527,40 @@ describe('Parse Role testing', () => { }); }); }); + + it('should be able to create an active role', (done) => { + const roleACL = new Parse.ACL(); + roleACL.setPublicReadAccess(true); + const role = new Parse.Role('some_active_role', roleACL); + role.set('active', true); + role.save({}, {useMasterKey : true}) + .then((savedRole)=>{ + expect(savedRole.get('active')).toEqual(true); + const query = new Parse.Query('_Role'); + return query.find({ useMasterKey: true }); + }).then((roles) => { + expect(roles.length).toEqual(1); + const role = roles[0]; + expect(role.get('active')).toEqual(true); + done(); + }); + }); + + it('should be able to create an inactive role', (done) => { + const roleACL = new Parse.ACL(); + roleACL.setPublicReadAccess(true); + const role = new Parse.Role('some_active_role', roleACL); + role.set('active', false); + role.save({}, {useMasterKey : true}) + .then((savedRole)=>{ + expect(savedRole.get('active')).toEqual(false); + const query = new Parse.Query('_Role'); + return query.find({ useMasterKey: true }); + }).then((roles) => { + expect(roles.length).toEqual(1); + const role = roles[0]; + expect(role.get('active')).toEqual(false); + done(); + }); + }); }); diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json index 3f247853a2..c72d88372b 100644 --- a/spec/support/jasmine.json +++ b/spec/support/jasmine.json @@ -1,7 +1,7 @@ { "spec_dir": "spec", "spec_files": [ - "*spec.js" + "ParseRole.spec.js" ], "helpers": [ "../node_modules/babel-core/register.js", diff --git a/src/Auth.js b/src/Auth.js index 1cb2e96563..3a8f8b4776 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -131,6 +131,10 @@ Auth.prototype._loadRoles = function() { __type: 'Pointer', className: '_User', objectId: this.user.id + }, + // By using $ne instead of $eq=true we support earlier parse versions where the active field might be undefined + 'active': { + '$ne' : false } }; // First get the role ids this user is directly a member of @@ -191,6 +195,9 @@ Auth.prototype._getAllRolesNamesForRoleIds = function(roleIDs, names = [], queri } else { restWhere = { 'roles': { '$in': ins }} } + // Always make sure roles are active + restWhere.active = { '$ne' : false } + const query = new RestQuery(this.config, master(this.config), '_Role', restWhere, {}); return query.execute().then((response) => { var results = response.results; diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 7c133f269c..353b71a696 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -62,7 +62,8 @@ const defaultColumns: {[string]: SchemaFields} = Object.freeze({ _Role: { "name": {type:'String'}, "users": {type:'Relation', targetClass:'_User'}, - "roles": {type:'Relation', targetClass:'_Role'} + "roles": {type:'Relation', targetClass:'_Role'}, + "active": {type:'Boolean'} }, // The additional default columns for the _Session collection (in addition to DefaultCols) _Session: { diff --git a/src/RestWrite.js b/src/RestWrite.js index fae37a1fc5..6a07cee6cb 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -190,9 +190,16 @@ RestWrite.prototype.setRequiredFieldsIfNeeded = function() { if (!this.query) { this.data.createdAt = this.updatedAt; - // Only assign new objectId if we are creating new object - if (!this.data.objectId) { + const creatingObject = !this.data.objectId; + if (creatingObject) { + // Only assign new objectId if we are creating new object this.data.objectId = cryptoUtils.newObjectId(this.config.objectIdSize); + // On Role, assume is active, and assign 'active' to true + if(this.className === '_Role'){ + if(this.data.active === undefined){ + this.data.active = true; + } + } } } } From 11c0f9f6431559139479f5c764365007f267fdf5 Mon Sep 17 00:00:00 2001 From: Georges Jamous Date: Fri, 8 Jun 2018 16:53:14 +0300 Subject: [PATCH 2/5] writing tests --- spec/ParseRole.spec.js | 126 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 2 deletions(-) diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index b114a9fa35..df8b31e0ee 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -528,7 +528,7 @@ describe('Parse Role testing', () => { }); }); - it('should be able to create an active role', (done) => { + it('should be able to create an active role (#4591)', (done) => { const roleACL = new Parse.ACL(); roleACL.setPublicReadAccess(true); const role = new Parse.Role('some_active_role', roleACL); @@ -546,7 +546,7 @@ describe('Parse Role testing', () => { }); }); - it('should be able to create an inactive role', (done) => { + it('should be able to create an inactive role (#4591)', (done) => { const roleACL = new Parse.ACL(); roleACL.setPublicReadAccess(true); const role = new Parse.Role('some_active_role', roleACL); @@ -563,4 +563,126 @@ describe('Parse Role testing', () => { done(); }); }); + + it('should properly handle active/inactive role state permissions across multiple role levels (#4591)', (done) => { + // Owners inherit from Collaborators + // Collaborators inherit from members + // Members doe not inherit from any role + const owner = new Parse.User(); + const collaborator = new Parse.User(); + const member = new Parse.User(); + let ownerRole, collaboratorRole, memberRole; + let objectOnlyForOwners; // Acl access by owners only + let objectOnlyForCollaborators; // Acl access by collaborators only + let objectOnlyForMembers; // Acl access by members only + let ownerACL, collaboratorACL, memberACL; + + return owner.save({ username: 'owner', password: 'pass' }) + .then(() => { + return collaborator.save({ username: 'collaborator', password: 'pass' }); + }).then(() => { + return member.save({ username: 'member', password: 'pass' }); + }).then(() => { + ownerACL = new Parse.ACL(); + ownerACL.setRoleReadAccess("ownerRole", true); + ownerACL.setRoleWriteAccess("ownerRole", true); + ownerRole = new Parse.Role('ownerRole', ownerACL); + ownerRole.getUsers().add(owner); + return ownerRole.save({}, { useMasterKey: true }); + }).then(() => { + collaboratorACL = new Parse.ACL(); + collaboratorACL.setRoleReadAccess('collaboratorRole', true); + collaboratorACL.setRoleWriteAccess('collaboratorRole', true); + collaboratorRole = new Parse.Role('collaboratorRole', collaboratorACL); + collaboratorRole.getUsers().add(collaborator); + // owners inherit from collaborators + collaboratorRole.getRoles().add(ownerRole); + return collaboratorRole.save({}, { useMasterKey: true }); + }).then(() => { + memberACL = new Parse.ACL(); + memberACL.setRoleReadAccess('memberRole', true); + memberRole = new Parse.Role('memberRole', memberACL); + memberRole.set('active', false); + memberRole.getUsers().add(member); + // collaborators inherit from members + memberRole.getRoles().add(collaboratorRole); + return memberRole.save({}, { useMasterKey: true }); + }).then(() => { + // routine check + const query = new Parse.Query('_Role'); + return query.find({ useMasterKey: true }); + }).then((x) => { + expect(x.length).toEqual(3); + + const acl = new Parse.ACL(); + acl.setRoleReadAccess("memberRole", true); + acl.setRoleWriteAccess("memberRole", true); + objectOnlyForMembers = new Parse.Object('TestObjectRoles'); + objectOnlyForMembers.setACL(acl); + return objectOnlyForMembers.save(null, { useMasterKey: true }); + }).then(() => { + const acl = new Parse.ACL(); + acl.setRoleReadAccess("collaboratorRole", true); + acl.setRoleWriteAccess("collaboratorRole", true); + objectOnlyForCollaborators = new Parse.Object('TestObjectRoles'); + objectOnlyForCollaborators.setACL(acl); + return objectOnlyForCollaborators.save(null, { useMasterKey: true }); + }).then(() => { + const acl = new Parse.ACL(); + acl.setRoleReadAccess("ownerRole", true); + acl.setRoleWriteAccess("ownerRole", true); + objectOnlyForOwners = new Parse.Object('TestObjectRoles'); + objectOnlyForOwners.setACL(acl); + return objectOnlyForOwners.save(null, { useMasterKey: true }); + }) + + .then(() => { + // First level role - members should not be able to edit object when role is inactive + objectOnlyForMembers.set('hello', 'hello'); + return objectOnlyForMembers.save(null, { sessionToken: member.getSessionToken() }); + }).then(() => { + fail('Operation should not have succeeded (Level-0)'); + done() + }, (error) => { + expect(error.code).toEqual(101); + return Promise.resolve() + }) + + .then(() => { + // Second level role - collaborators should not be able to edit object when role is inactive + objectOnlyForMembers.set('hello', 'hello'); + return objectOnlyForMembers.save(null, { sessionToken: collaborator.getSessionToken() }); + }).then(() => { + fail('Operation should not have succeeded (Level-1)'); + done() + }, (error) => { + expect(error.code).toEqual(101); + return Promise.resolve() + }) + + .then(() => { + // Third level role - admins should not be able to edit object when role is inactive + objectOnlyForMembers.set('hello', 'hello'); + return objectOnlyForMembers.save(null, { sessionToken: owner.getSessionToken() }); + }).then(() => { + fail('Operation should not have succeeded (Level-2)'); + done() + }, (error) => { + expect(error.code).toEqual(101); + return Promise.resolve() + }) + + .then(() => { + // Set members to active and collaborators to inactive + // Members should be abel to edit. Collaborators and Owners should not. + objectOnlyForMembers.set('hello', 'hello'); + return objectOnlyForMembers.save(null, { sessionToken: owner.getSessionToken() }); + }).then(() => { + fail('Operation should not have succeeded (Level-2)'); + done() + }, (error) => { + expect(error.code).toEqual(101); + return Promise.resolve() + }) + }); }); From 814f9abafff6749868d2f0db5ffe14cff04a9eb6 Mon Sep 17 00:00:00 2001 From: Confidential Date: Fri, 8 Jun 2018 21:00:06 +0300 Subject: [PATCH 3/5] Changed active field name to enabled --- spec/ParseRole.spec.js | 104 ++++++++++++++++++++++------ spec/support/jasmine.json | 2 +- src/Auth.js | 9 +-- src/Controllers/SchemaController.js | 2 +- src/RestWrite.js | 6 +- 5 files changed, 93 insertions(+), 30 deletions(-) diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index df8b31e0ee..366b32219e 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -528,46 +528,48 @@ describe('Parse Role testing', () => { }); }); - it('should be able to create an active role (#4591)', (done) => { + it('should be able to create an enabled role (#4591)', (done) => { const roleACL = new Parse.ACL(); roleACL.setPublicReadAccess(true); const role = new Parse.Role('some_active_role', roleACL); - role.set('active', true); + role.set('enabled', true); role.save({}, {useMasterKey : true}) .then((savedRole)=>{ - expect(savedRole.get('active')).toEqual(true); + expect(savedRole.get('enabled')).toEqual(true); const query = new Parse.Query('_Role'); return query.find({ useMasterKey: true }); }).then((roles) => { expect(roles.length).toEqual(1); const role = roles[0]; - expect(role.get('active')).toEqual(true); + expect(role.get('enabled')).toEqual(true); done(); }); }); - it('should be able to create an inactive role (#4591)', (done) => { + it('should be able to create a disabled role (#4591)', (done) => { const roleACL = new Parse.ACL(); roleACL.setPublicReadAccess(true); const role = new Parse.Role('some_active_role', roleACL); - role.set('active', false); + role.set('enabled', false); role.save({}, {useMasterKey : true}) .then((savedRole)=>{ - expect(savedRole.get('active')).toEqual(false); + expect(savedRole.get('enabled')).toEqual(false); const query = new Parse.Query('_Role'); return query.find({ useMasterKey: true }); }).then((roles) => { expect(roles.length).toEqual(1); const role = roles[0]; - expect(role.get('active')).toEqual(false); + expect(role.get('enabled')).toEqual(false); done(); }); }); - it('should properly handle active/inactive role state permissions across multiple role levels (#4591)', (done) => { + it('should properly handle enabled/disabled role states permissions across multiple role levels properly (#4591)', (done) => { // Owners inherit from Collaborators // Collaborators inherit from members - // Members doe not inherit from any role + // Members does not inherit from any role + // Owner -> Collaborator -> member -> [protected objects] + // If any role is disabled, the link is broken. const owner = new Parse.User(); const collaborator = new Parse.User(); const member = new Parse.User(); @@ -578,11 +580,9 @@ describe('Parse Role testing', () => { let ownerACL, collaboratorACL, memberACL; return owner.save({ username: 'owner', password: 'pass' }) + .then(() => collaborator.save({ username: 'collaborator', password: 'pass' })) + .then(() => member.save({ username: 'member', password: 'pass' })) .then(() => { - return collaborator.save({ username: 'collaborator', password: 'pass' }); - }).then(() => { - return member.save({ username: 'member', password: 'pass' }); - }).then(() => { ownerACL = new Parse.ACL(); ownerACL.setRoleReadAccess("ownerRole", true); ownerACL.setRoleWriteAccess("ownerRole", true); @@ -602,7 +602,7 @@ describe('Parse Role testing', () => { memberACL = new Parse.ACL(); memberACL.setRoleReadAccess('memberRole', true); memberRole = new Parse.Role('memberRole', memberACL); - memberRole.set('active', false); + memberRole.set('enabled', false); // Disabled!! memberRole.getUsers().add(member); // collaborators inherit from members memberRole.getRoles().add(collaboratorRole); @@ -613,6 +613,11 @@ describe('Parse Role testing', () => { return query.find({ useMasterKey: true }); }).then((x) => { expect(x.length).toEqual(3); + x.forEach(role => { + if(role.name === "ownerRole") expect(role.get('enabled').toBeEqual(true)); + if(role.name === "collaboratorRole") expect(role.get('enabled').toBeEqual(true)); + if(role.name === "memberRole") expect(role.get('enabled').toBeEqual(false)); + }); const acl = new Parse.ACL(); acl.setRoleReadAccess("memberRole", true); @@ -637,7 +642,7 @@ describe('Parse Role testing', () => { }) .then(() => { - // First level role - members should not be able to edit object when role is inactive + // First level role - members should not be able to edit object when their role is disabled objectOnlyForMembers.set('hello', 'hello'); return objectOnlyForMembers.save(null, { sessionToken: member.getSessionToken() }); }).then(() => { @@ -649,7 +654,7 @@ describe('Parse Role testing', () => { }) .then(() => { - // Second level role - collaborators should not be able to edit object when role is inactive + // Second level role - collaborators should not be able to edit object when member role is disabled objectOnlyForMembers.set('hello', 'hello'); return objectOnlyForMembers.save(null, { sessionToken: collaborator.getSessionToken() }); }).then(() => { @@ -661,8 +666,7 @@ describe('Parse Role testing', () => { }) .then(() => { - // Third level role - admins should not be able to edit object when role is inactive - objectOnlyForMembers.set('hello', 'hello'); + // Third level role - admins should not be able to edit object when member role is disabled return objectOnlyForMembers.save(null, { sessionToken: owner.getSessionToken() }); }).then(() => { fail('Operation should not have succeeded (Level-2)'); @@ -672,17 +676,75 @@ describe('Parse Role testing', () => { return Promise.resolve() }) + .then(() => { + // Owners should be able to inherit form collaborator role and edit object + objectOnlyForCollaborators.set('hello', 'hello'); + return objectOnlyForCollaborators.save(null, { sessionToken: owner.getSessionToken() }); + }).then(() => { + return Promise.resolve() + }, () => { + fail('Owners should be able to save collaborator object normally'); + done() + }) + .then(() => { // Set members to active and collaborators to inactive - // Members should be abel to edit. Collaborators and Owners should not. + // Members should be able to edit. Collaborators and Owners should not. + memberRole.set('enabled', true); + collaboratorRole.set('enabled', false); + return memberRole.save({}, {useMasterKey: true}).then(() => collaboratorRole.save({}, {useMasterKey: true})); + }).then(() => { + // this should succeed objectOnlyForMembers.set('hello', 'hello'); + return objectOnlyForMembers.save(null, { sessionToken: member.getSessionToken() }); + }, () => { + fail('Users in an enabled role should be able to access role secured objects. Enabled roles should grant permissions'); + done() + }) + .then(() => { + expect(objectOnlyForMembers.get('hello')).toEqual('hello'); + // this should fail, collaborator should not be able to edit, since their role is disabled + objectOnlyForMembers.unset('hello'); + return objectOnlyForMembers.save(null, { sessionToken: collaborator.getSessionToken() }); + }) + .then(() => { + fail('Operation should not have succeeded. A disabled role should not grant permissions'); + done(); + }, (error) => { + expect(error.code).toEqual(101); + return Promise.resolve() + }) + .then(() => { + // this should fail return objectOnlyForMembers.save(null, { sessionToken: owner.getSessionToken() }); }).then(() => { - fail('Operation should not have succeeded (Level-2)'); + fail('Operation should not have succeeded. Owners should not have access to object if collaborator role is disabled'); done() }, (error) => { expect(error.code).toEqual(101); return Promise.resolve() }) + + // Extra uneeded check + .then(() => { + // Check that role tree operate normally in enabled/disabled state. + // Collaborators should not be able to edit admin role protected objects. + collaboratorRole.set('enabled', true); + ownerRole.set('enabled', false); + return ownerRole.save({}, {useMasterKey: true}).then(() => collaboratorRole.save({}, {useMasterKey: true})); + }).then(() => { + objectOnlyForOwners.unset('hello'); + return objectOnlyForOwners.save(null, { sessionToken: collaborator.getSessionToken() }); + }).then(() => { + fail('This test should fail, roles do not work this way. Child inherits from parent, not the otehr way around'); + done() + }, (error) => { + expect(error.code).toEqual(101); + return Promise.resolve() + }) + + .then(() => { + done(); + }); }); }); diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json index c72d88372b..05f29048a1 100644 --- a/spec/support/jasmine.json +++ b/spec/support/jasmine.json @@ -1,7 +1,7 @@ { "spec_dir": "spec", "spec_files": [ - "ParseRole.spec.js" + "*.spec.js" ], "helpers": [ "../node_modules/babel-core/register.js", diff --git a/src/Auth.js b/src/Auth.js index 3a8f8b4776..65c157a764 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -132,8 +132,9 @@ Auth.prototype._loadRoles = function() { className: '_User', objectId: this.user.id }, - // By using $ne instead of $eq=true we support earlier parse versions where the active field might be undefined - 'active': { + // By using $ne instead of $eq=true we support earlier versions of parse where the 'enabled' field might be undefined + // $ne should not affect performance since Roles are often fetched from cache + 'enabled': { '$ne' : false } }; @@ -195,8 +196,8 @@ Auth.prototype._getAllRolesNamesForRoleIds = function(roleIDs, names = [], queri } else { restWhere = { 'roles': { '$in': ins }} } - // Always make sure roles are active - restWhere.active = { '$ne' : false } + // Always make sure roles are enabled + restWhere.enabled = { '$ne' : false } const query = new RestQuery(this.config, master(this.config), '_Role', restWhere, {}); return query.execute().then((response) => { diff --git a/src/Controllers/SchemaController.js b/src/Controllers/SchemaController.js index 353b71a696..29073a9733 100644 --- a/src/Controllers/SchemaController.js +++ b/src/Controllers/SchemaController.js @@ -63,7 +63,7 @@ const defaultColumns: {[string]: SchemaFields} = Object.freeze({ "name": {type:'String'}, "users": {type:'Relation', targetClass:'_User'}, "roles": {type:'Relation', targetClass:'_Role'}, - "active": {type:'Boolean'} + "enabled": {type:'Boolean'} }, // The additional default columns for the _Session collection (in addition to DefaultCols) _Session: { diff --git a/src/RestWrite.js b/src/RestWrite.js index 6a07cee6cb..7a10a20cb7 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -194,10 +194,10 @@ RestWrite.prototype.setRequiredFieldsIfNeeded = function() { if (creatingObject) { // Only assign new objectId if we are creating new object this.data.objectId = cryptoUtils.newObjectId(this.config.objectIdSize); - // On Role, assume is active, and assign 'active' to true + // On Role, assume is not disabled, and assign 'enabled' to true if(this.className === '_Role'){ - if(this.data.active === undefined){ - this.data.active = true; + if(this.data.enabled === undefined){ + this.data.enabled = true; } } } From 4cd5d08106ffb13c2b0d70c4bea0e954f340fe75 Mon Sep 17 00:00:00 2001 From: Confidential Date: Fri, 8 Jun 2018 22:18:47 +0300 Subject: [PATCH 4/5] Fixed issue when role is saved without enabled key --- spec/ParseRole.spec.js | 122 ++++++++++++++++++++++++++++++++++++----- src/RestWrite.js | 6 ++ 2 files changed, 115 insertions(+), 13 deletions(-) diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index 366b32219e..c7af4ec6c8 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -549,11 +549,12 @@ describe('Parse Role testing', () => { it('should be able to create a disabled role (#4591)', (done) => { const roleACL = new Parse.ACL(); roleACL.setPublicReadAccess(true); - const role = new Parse.Role('some_active_role', roleACL); + const role = new Parse.Role('some_disabled_role', roleACL); role.set('enabled', false); role.save({}, {useMasterKey : true}) - .then((savedRole)=>{ - expect(savedRole.get('enabled')).toEqual(false); + .then(() => { + expect(role.get('enabled')).toEqual(false); + const query = new Parse.Query('_Role'); return query.find({ useMasterKey: true }); }).then((roles) => { @@ -564,12 +565,30 @@ describe('Parse Role testing', () => { }); }); + it('should create an enabled role by default (#4591)', (done) => { + const roleACL = new Parse.ACL(); + roleACL.setPublicReadAccess(true); + const role = new Parse.Role('some_active_role', roleACL); + role.save({}, {useMasterKey : true}) + .then(() => { + expect(role.get('enabled')).toEqual(true); + + const query = new Parse.Query('_Role'); + return query.find({ useMasterKey: true }); + }).then((roles) => { + expect(roles.length).toEqual(1); + const role = roles[0]; + expect(role.get('enabled')).toEqual(true); + done(); + }); + }); + it('should properly handle enabled/disabled role states permissions across multiple role levels properly (#4591)', (done) => { // Owners inherit from Collaborators // Collaborators inherit from members // Members does not inherit from any role // Owner -> Collaborator -> member -> [protected objects] - // If any role is disabled, the link is broken. + // If any role is disabled, the remaining role link tree is broken. const owner = new Parse.User(); const collaborator = new Parse.User(); const member = new Parse.User(); @@ -646,7 +665,7 @@ describe('Parse Role testing', () => { objectOnlyForMembers.set('hello', 'hello'); return objectOnlyForMembers.save(null, { sessionToken: member.getSessionToken() }); }).then(() => { - fail('Operation should not have succeeded (Level-0)'); + fail('A disabled role cannot grant permission to its users. (Level-0)'); done() }, (error) => { expect(error.code).toEqual(101); @@ -658,7 +677,7 @@ describe('Parse Role testing', () => { objectOnlyForMembers.set('hello', 'hello'); return objectOnlyForMembers.save(null, { sessionToken: collaborator.getSessionToken() }); }).then(() => { - fail('Operation should not have succeeded (Level-1)'); + fail('A disabled role cannot grant permission to its child roles. (Level-1)'); done() }, (error) => { expect(error.code).toEqual(101); @@ -669,7 +688,7 @@ describe('Parse Role testing', () => { // Third level role - admins should not be able to edit object when member role is disabled return objectOnlyForMembers.save(null, { sessionToken: owner.getSessionToken() }); }).then(() => { - fail('Operation should not have succeeded (Level-2)'); + fail('A disabled role cannot grant permission to its child roles. (Level-2)'); done() }, (error) => { expect(error.code).toEqual(101); @@ -683,12 +702,12 @@ describe('Parse Role testing', () => { }).then(() => { return Promise.resolve() }, () => { - fail('Owners should be able to save collaborator object normally'); + fail('Enabled roles should grant permissions to child roles normally.'); done() }) .then(() => { - // Set members to active and collaborators to inactive + // Set members enabled and collaborators to disabled // Members should be able to edit. Collaborators and Owners should not. memberRole.set('enabled', true); collaboratorRole.set('enabled', false); @@ -698,7 +717,7 @@ describe('Parse Role testing', () => { objectOnlyForMembers.set('hello', 'hello'); return objectOnlyForMembers.save(null, { sessionToken: member.getSessionToken() }); }, () => { - fail('Users in an enabled role should be able to access role secured objects. Enabled roles should grant permissions'); + fail('Enabled roles should grant permissions to its users.'); done() }) .then(() => { @@ -708,7 +727,7 @@ describe('Parse Role testing', () => { return objectOnlyForMembers.save(null, { sessionToken: collaborator.getSessionToken() }); }) .then(() => { - fail('Operation should not have succeeded. A disabled role should not grant permissions'); + fail('Disabled roles cannot not grant permission ot its users'); done(); }, (error) => { expect(error.code).toEqual(101); @@ -718,7 +737,7 @@ describe('Parse Role testing', () => { // this should fail return objectOnlyForMembers.save(null, { sessionToken: owner.getSessionToken() }); }).then(() => { - fail('Operation should not have succeeded. Owners should not have access to object if collaborator role is disabled'); + fail('Disabled roles cannot not grant permission to its children roles'); done() }, (error) => { expect(error.code).toEqual(101); @@ -736,7 +755,7 @@ describe('Parse Role testing', () => { objectOnlyForOwners.unset('hello'); return objectOnlyForOwners.save(null, { sessionToken: collaborator.getSessionToken() }); }).then(() => { - fail('This test should fail, roles do not work this way. Child inherits from parent, not the otehr way around'); + fail('Roles do not work this way. Child inherits from parent, not the other way around'); done() }, (error) => { expect(error.code).toEqual(101); @@ -747,4 +766,81 @@ describe('Parse Role testing', () => { done(); }); }); + + it('parent role should still be able to edit roles that it has disabled and have R/W access to (#4591)', (done) => { + const admin = new Parse.User(); + const member = new Parse.User(); + let adminRole, membersRole; + let adminACL, memberACL; + + return admin.save({ username: 'admin', password: 'pass' }) + .then(() => member.save({ username: 'member', password: 'pass' })) + .then(() => { + adminACL = new Parse.ACL(); + adminACL.setRoleReadAccess("ownerRole", true); + adminACL.setRoleWriteAccess("ownerRole", true); + adminRole = new Parse.Role('ownerRole', adminACL); + adminRole.getUsers().add(admin); + return adminRole.save({}, { useMasterKey: true }); + }).then(() => { + memberACL = new Parse.ACL(); + memberACL.setRoleReadAccess('collaboratorRole', true); + // admin can write on this role + memberACL.setRoleWriteAccess('ownerRole', true); + membersRole = new Parse.Role('collaboratorRole', memberACL); + membersRole.getUsers().add(member); + // admins inherit from members + membersRole.getRoles().add(adminRole); + return membersRole.save({}, { useMasterKey: true }); + }).then(() => { + // admins should be able to edit members when members are enabled + membersRole.set('enabled', false) + return membersRole.save(null, { sessionToken: admin.getSessionToken() }); + }).then(() => { + return Promise.resolve() + }, () => { + fail('parent role should be able to edit child roles when enabled child roles are enabled'); + return Promise.resolve() + }) + .then(() => { + // admins should be able to edit members even when members role is disabled + membersRole.set('enabled', true) + return membersRole.save(null, { sessionToken: admin.getSessionToken() }); + }).then(() => { + return Promise.resolve() + }, () => { + fail('parent role should be able to edit child roles when enabled child roles are disabled'); + return Promise.resolve() + }) + .then(() => { + done(); + }); + }); + + it('disabled roles cannot edit themselves even with R/W access (#4591)', (done) => { + const member = new Parse.User(); + let role; + let roleACL; + + return member.save({ username: 'member', password: 'pass' }) + .then(() => { + roleACL = new Parse.ACL(); + roleACL.setRoleReadAccess("ownerRole", true); + roleACL.setRoleWriteAccess("ownerRole", true); + role = new Parse.Role('ownerRole', roleACL); + role.getUsers().add(member); + role.set('enabled', false); + return role.save({}, { useMasterKey: true }); + }).then(() => { + role.set('enabled', true) + return role.save(null, { sessionToken: member.getSessionToken() }); + }).then(() => { + fail('disabled role should not grand permission to its users, even for itself'); + done(); + }, (error) => { + expect(error.code).toEqual(101); + done() + }) + }); + }); diff --git a/src/RestWrite.js b/src/RestWrite.js index 7a10a20cb7..aa36526376 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1093,6 +1093,12 @@ RestWrite.prototype.runDatabaseOperation = function() { response.objectId = this.data.objectId; response.createdAt = this.data.createdAt; + // (#4591) Role "enabled" key is generated with on create, and SDKs might not support this change yet. + // removing this line will brake RoleTest ("should create an enabled role by default") + if(this.className === "_Role" && !this.query){ + response.enabled = this.data.enabled; + } + if (this.responseShouldHaveUsername) { response.username = this.data.username; } From 412f881cc9c950a9d3d3d2ec53c53673fe6e9a5a Mon Sep 17 00:00:00 2001 From: Confidential Date: Fri, 8 Jun 2018 22:30:38 +0300 Subject: [PATCH 5/5] Reverted jasmine --- spec/support/jasmine.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json index 05f29048a1..3f247853a2 100644 --- a/spec/support/jasmine.json +++ b/spec/support/jasmine.json @@ -1,7 +1,7 @@ { "spec_dir": "spec", "spec_files": [ - "*.spec.js" + "*spec.js" ], "helpers": [ "../node_modules/babel-core/register.js",