From 1ef3131ed7801ef7694d717aeefc13cbe8563a76 Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Thu, 10 Nov 2016 14:14:43 +0000 Subject: [PATCH 1/3] HSC-1235 - Update user permissions to use space/org permission --- .../assign-users-workflow/assign-users.service.js | 7 +++---- .../actions/manage-user/manage-user.service.js | 6 +++--- .../roles-tables/roles-tables.directive.js | 9 ++++----- .../cluster/actions/roles-tables/roles.service.js | 15 ++++++--------- .../detail/actions/cluster-actions.directive.js | 4 ++-- .../detail/users/cluster-detail.users.module.js | 2 +- .../users/organization-detail.users.module.js | 7 +++---- .../detail/users/space-detail.users.module.js | 6 +++--- .../cloud-foundry/model/auth/auth.model.js | 2 +- 9 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js b/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js index 9b745ec50d..ef53fe0cde 100644 --- a/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js +++ b/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js @@ -101,7 +101,7 @@ // Create a collection to support the organization drop down var organizations = _.omitBy(that.organizationModel.organizations[that.data.clusterGuid], function (org) { return !that.authModel.isOrgOrSpaceActionableByResource(that.data.clusterGuid, org, - that.authModel.resources.user, that.authModel.actions.update); + that.authModel.resources.organization, that.authModel.actions.update); }); that.data.organizations = _.chain(organizations) @@ -231,9 +231,8 @@ return false; } return that.authModel.isAllowed(context.clusterGuid, - that.authModel.resources.user, - that.authModel.actions.update, null, - org.details.guid); + that.authModel.resources.organization, + that.authModel.actions.update, org.details.guid); } }, table: { diff --git a/src/app/view/endpoints/clusters/cluster/actions/manage-user/manage-user.service.js b/src/app/view/endpoints/clusters/cluster/actions/manage-user/manage-user.service.js index a828a97787..3d7f1a734f 100644 --- a/src/app/view/endpoints/clusters/cluster/actions/manage-user/manage-user.service.js +++ b/src/app/view/endpoints/clusters/cluster/actions/manage-user/manage-user.service.js @@ -54,7 +54,7 @@ if (organizationGuid) { return organizationGuid !== orgGuid; } - return !authModel.isOrgOrSpaceActionableByResource(clusterGuid, org, authModel.resources.user, + return !authModel.isOrgOrSpaceActionableByResource(clusterGuid, org, authModel.resources.organization, authModel.actions.update); }); @@ -65,9 +65,9 @@ _.forEach(organizations, function (organization) { selectedRoles[organization.details.org.metadata.guid] = {}; disableClearAll = disableClearAll || !authModel.isAllowed(clusterGuid, - authModel.resources.user, + authModel.resources.organization, authModel.actions.update, - null, organization.details.org.metadata.guid); + organization.details.org.metadata.guid); }); // Async refresh roles diff --git a/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles-tables.directive.js b/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles-tables.directive.js index 8e80a0ab99..df96596b60 100644 --- a/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles-tables.directive.js +++ b/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles-tables.directive.js @@ -73,18 +73,17 @@ this.disableAssignSpaceRoles = function (spaceKey) { var space = that.organization.spaces[spaceKey]; return !that.authModel.isAllowed(that.config.clusterGuid, - that.authModel.resources.user, + that.authModel.resources.space, that.authModel.actions.update, - space.metadata.guid, space.entity.organization_guid, - true); + space.metadata.guid, space.entity.organization_guid); }; // Helper to enable/disable org role checkbox inputs this.disableAssignOrgRoles = function (org) { return !that.authModel.isAllowed(that.config.clusterGuid, - that.authModel.resources.user, + that.authModel.resources.organization, that.authModel.actions.update, - null, org.metadata.guid); + org.metadata.guid); }; function refresh() { diff --git a/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles.service.js b/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles.service.js index 382d14ce71..29510504b7 100644 --- a/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles.service.js +++ b/src/app/view/endpoints/clusters/cluster/actions/roles-tables/roles.service.js @@ -104,9 +104,8 @@ this.canRemoveOrgRole = function (role, clusterGuid, orgGuid, userGuid) { var isAllowed = authModel.isAllowed(clusterGuid, - authModel.resources.user, - authModel.actions.update, null, - orgGuid); + authModel.resources.organization, + authModel.actions.update, orgGuid); if (!isAllowed) { return false; @@ -609,10 +608,8 @@ // Calculate org role delta only for organizations for which user is allowed to var isUserAllowed = authModel.isAllowed(clusterGuid, - authModel.resources.user, - authModel.actions.update, - null, - orgGuid); + authModel.resources.organization, + authModel.actions.update, orgGuid); // For each organization role _.forEach(orgRolesPerUser.organization, function (selected, roleKey) { @@ -629,8 +626,8 @@ _.forEach(orgRolesPerUser.spaces, function (spaceRoles, spaceGuid) { // calculate space role delta only for spaces for which user is allowed - var isAllowed = authModel.isAllowed(clusterGuid, authModel.resources.user, - authModel.actions.update, spaceGuid, orgGuid, true); + var isAllowed = authModel.isAllowed(clusterGuid, authModel.resources.space, + authModel.actions.update, spaceGuid, orgGuid); // For each space role _.forEach(spaceRoles, function (selected, roleKey) { diff --git a/src/app/view/endpoints/clusters/cluster/detail/actions/cluster-actions.directive.js b/src/app/view/endpoints/clusters/cluster/detail/actions/cluster-actions.directive.js index 6e893032a8..a17104b1ff 100644 --- a/src/app/view/endpoints/clusters/cluster/detail/actions/cluster-actions.directive.js +++ b/src/app/view/endpoints/clusters/cluster/detail/actions/cluster-actions.directive.js @@ -251,11 +251,11 @@ if (that.spaceGuid) { // We're at least the 'space' depth of a cluster. Check permissions against it. canAssignUsers = - authModel.isAllowed(that.clusterGuid, authModel.resources.user, authModel.actions.update, that.spaceGuid, that.organizationGuid, true); + authModel.isAllowed(that.clusterGuid, authModel.resources.space, authModel.actions.update, that.spaceGuid, that.organizationGuid); } else { // We're at the organization depth, check if user has any space manager roles within it canAssignUsers = - authModel.isAllowed(that.clusterGuid, authModel.resources.user, authModel.actions.update, null, that.organizationGuid) || + authModel.isAllowed(that.clusterGuid, authModel.resources.organization, authModel.actions.update, that.organizationGuid) || _.find(authModel.principal[that.clusterGuid].userSummary.spaces.managed, { entity: { organization_guid: that.organizationGuid}}); } } else { diff --git a/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.js b/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.js index 3f30cce7bb..6971bcfe55 100644 --- a/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.js +++ b/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.js @@ -84,7 +84,7 @@ roleLabel: that.organizationModel.organizationRoleToString(role) }); unEditableOrg = unEditableOrg || - !that.authModel.isAllowed(that.guid, that.authModel.resources.user, that.authModel.actions.update, null, orgGuid); + !that.authModel.isAllowed(that.guid, that.authModel.resources.organization, that.authModel.actions.update, orgGuid); }); }); diff --git a/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.js b/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.js index ae663a5b42..707878033a 100644 --- a/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.js +++ b/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.js @@ -108,8 +108,7 @@ }; this.canUserRemoveFromOrg = function () { - return that.authModel.isAllowed(that.guid, that.authModel.resources.user, that.authModel.actions.update, null, - that.organizationGuid); + return that.authModel.isAllowed(that.guid, that.authModel.resources.organization, that.authModel.actions.update, that.organizationGuid); }; this.disableManageRoles = function () { @@ -200,8 +199,8 @@ }; this.canRemoveSpaceRole = function (spaceGuid) { - return that.authModel.isAllowed(that.guid, that.authModel.resources.user, that.authModel.actions.update, - spaceGuid, that.organizationGuid, true); + return that.authModel.isAllowed(that.guid, that.authModel.resources.space, that.authModel.actions.update, + spaceGuid, that.organizationGuid); }; this.removeSpaceRole = function (user, spaceRole) { diff --git a/src/app/view/endpoints/clusters/cluster/organization/space/detail/users/space-detail.users.module.js b/src/app/view/endpoints/clusters/cluster/organization/space/detail/users/space-detail.users.module.js index 852a7db67b..2d6d8ce9d5 100644 --- a/src/app/view/endpoints/clusters/cluster/organization/space/detail/users/space-detail.users.module.js +++ b/src/app/view/endpoints/clusters/cluster/organization/space/detail/users/space-detail.users.module.js @@ -98,13 +98,13 @@ this.canUserManageRoles = function () { // User can assign org roles - return that.authModel.isAllowed(that.guid, that.authModel.resources.user, that.authModel.actions.update, that.organizationGuid) || + return that.authModel.isAllowed(that.guid, that.authModel.resources.organization, that.authModel.actions.update, that.organizationGuid) || // User can assign space roles - that.authModel.isAllowed(that.guid, that.authModel.resources.user, that.authModel.actions.update, that.spaceGuid, that.organizationGuid, true); + that.authModel.isAllowed(that.guid, that.authModel.resources.space, that.authModel.actions.update, that.spaceGuid, that.organizationGuid); }; this.canUserRemoveFromOrg = function () { - return that.authModel.isAllowed(that.guid, that.authModel.resources.user, that.authModel.actions.update, null, that.organizationGuid); + return that.authModel.isAllowed(that.guid, that.authModel.resources.organization, that.authModel.actions.update, that.organizationGuid); }; this.canUserRemoveFromSpace = function () { diff --git a/src/plugins/cloud-foundry/model/auth/auth.model.js b/src/plugins/cloud-foundry/model/auth/auth.model.js index 6eb3c911c0..f443c9dc0f 100644 --- a/src/plugins/cloud-foundry/model/auth/auth.model.js +++ b/src/plugins/cloud-foundry/model/auth/auth.model.js @@ -255,7 +255,7 @@ var that = this; var orgGuid = org.details.org.metadata.guid; // Is the organization valid? - if (this.isAllowed(cnsiGuid, resourceType, action, null, orgGuid)) { + if (this.isAllowed(cnsiGuid, resourceType, action, orgGuid)) { return true; } else { // Is any of the organization's spaces valid? From 56e14a9bf6c7a2fbd3b5486ddc4c8534262ca3ea Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Thu, 10 Nov 2016 14:29:27 +0000 Subject: [PATCH 2/3] Fixed unit tests --- .../cluster/detail/users/cluster-detail.users.module.spec.js | 2 +- .../detail/users/organization-detail.users.module.spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.spec.js b/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.spec.js index 2d3a335503..89a1d69998 100644 --- a/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.spec.js +++ b/src/app/view/endpoints/clusters/cluster/detail/users/cluster-detail.users.module.spec.js @@ -99,7 +99,7 @@ } ]); _.set(authModel, 'principal.' + clusterGuid + '.userSummary.spaces.managed', []); - spyOn(authModel, 'isAllowed').and.callFake(function (cnsiGuid, resource, action, something, orgGuid) { + spyOn(authModel, 'isAllowed').and.callFake(function (cnsiGuid, resource, action, orgGuid) { return orgGuid !== org1.details.org.metadata.guid; }); diff --git a/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.spec.js b/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.spec.js index b0ed14c342..9c15a31818 100644 --- a/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.spec.js +++ b/src/app/view/endpoints/clusters/cluster/organization/detail/users/organization-detail.users.module.spec.js @@ -250,7 +250,7 @@ {} ]); _.set(authModel, 'principal.' + clusterGuid + '.userSummary.spaces.managed', []); - spyOn(authModel, 'isAllowed').and.callFake(function (cnsiGuid, resource, action, something, orgGuid) { + spyOn(authModel, 'isAllowed').and.callFake(function (cnsiGuid, resource, action, orgGuid) { expect(cnsiGuid).toEqual(clusterGuid); expect(orgGuid).toEqual(organizationGuid); return true; From 6465d8b743aa8c328c76863022ebbe08c70f137f Mon Sep 17 00:00:00 2001 From: Irfan Habib Date: Thu, 10 Nov 2016 15:52:02 +0000 Subject: [PATCH 3/3] fixed a bug affecting roles service --- .../actions/assign-users-workflow/assign-users.service.js | 3 +-- src/plugins/cloud-foundry/model/auth/auth.model.js | 7 +++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js b/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js index ef53fe0cde..721777a552 100644 --- a/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js +++ b/src/app/view/endpoints/clusters/cluster/actions/assign-users-workflow/assign-users.service.js @@ -100,8 +100,7 @@ // Omit any org that we don't have permissions to either edit org or at least one child space // Create a collection to support the organization drop down var organizations = _.omitBy(that.organizationModel.organizations[that.data.clusterGuid], function (org) { - return !that.authModel.isOrgOrSpaceActionableByResource(that.data.clusterGuid, org, - that.authModel.resources.organization, that.authModel.actions.update); + return !that.authModel.isOrgOrSpaceActionableByResource(that.data.clusterGuid, org, that.authModel.actions.update); }); that.data.organizations = _.chain(organizations) diff --git a/src/plugins/cloud-foundry/model/auth/auth.model.js b/src/plugins/cloud-foundry/model/auth/auth.model.js index f443c9dc0f..f0cf3040aa 100644 --- a/src/plugins/cloud-foundry/model/auth/auth.model.js +++ b/src/plugins/cloud-foundry/model/auth/auth.model.js @@ -246,23 +246,22 @@ * in the organization or any of the organization's spaces * @param {string} cnsiGuid - Cluster GUID * @param {object} org - console organization object - * @param {string} resourceType - Type of resource * (organization, space, user, service_managed_instances, routes, applications) * @param {string} action - action (create, delete, update..) * @returns {boolean} */ - isOrgOrSpaceActionableByResource: function (cnsiGuid, org, resourceType, action) { + isOrgOrSpaceActionableByResource: function (cnsiGuid, org, action) { var that = this; var orgGuid = org.details.org.metadata.guid; // Is the organization valid? - if (this.isAllowed(cnsiGuid, resourceType, action, orgGuid)) { + if (this.isAllowed(cnsiGuid, this.resources.organization, action, orgGuid)) { return true; } else { // Is any of the organization's spaces valid? for (var spaceGuid in org.spaces) { if (!org.spaces.hasOwnProperty(spaceGuid)) { continue; } var space = org.spaces[spaceGuid]; - if (that.isAllowed(cnsiGuid, resourceType, action, space.metadata.guid, orgGuid, true)) { + if (that.isAllowed(cnsiGuid, this.resources.space, action, space.metadata.guid, orgGuid)) { return true; } }