Skip to content

Commit

Permalink
feat: introduce a new permission instead of relying on a role for Glo…
Browse files Browse the repository at this point in the history
…bal sharing

...and introduce some other fixes:
- fix: correct styling for sharing buttons on access modal ui
- fix: cleanup/remove unnecessary parts
- fix: adjust permission name to "artifact:global:share:put" to better reflect its real purpose
- fix: better name for isPermittedGlobalShareArtifact (previous name isPermittedGlobalShareCohort did not reflect the
  fact that it is about all kind of artifacts)
- fix: move isOwner() check and make sure it is only applied when !config.limitedPermissionManagement
- fix: move isOwner() check for conceptset-manager
  • Loading branch information
pieterlukasse committed Nov 25, 2024
1 parent 5bac724 commit 34d7425
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 104 deletions.
5 changes: 2 additions & 3 deletions js/components/security/access/configure-access-modal.html
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,14 @@
<label data-bind="css: classes('new-access-label'), text: ko.i18n('common.configureAccessModal.globalReadStatus', 'Status of global READ access:')"></label>
<div/>
<div class="btn-group" data-toggle="buttons">
<label data-bind="css: { active: !shareFlag()},
<label data-bind="css: { active: shareFlag()},
click: function () { shareFlag(false); grantGlobalReadAccess();},
clickBubble: false,
text: ko.i18n('common.configureAccessModal.globalReadStatusNotGranted', 'Granted')
"
class="btn btn-primary",
/>
<label data-bind="css: {
active: shareFlag()},
<label data-bind="css: { active: !shareFlag()},
click: function () { shareFlag(true); revokeGlobalReadAccess();},
clickBubble: false,
text: ko.i18n('common.configureAccessModal.globalReadStatusIsGranted', 'Not Granted')
Expand Down
2 changes: 1 addition & 1 deletion js/pages/cohort-definitions/cohort-definition-manager.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

<!-- ko if: (enablePermissionManagement() === true && userCanShare() === true) -->
<button class="btn btn-primary"
data-bind="title: ko.i18n('common.configureAccess', 'Configure access'), visible: isOwner() && !previewVersion(), click: () => isAccessModalShown(!isAccessModalShown())">
data-bind="title: ko.i18n('common.configureAccess', 'Configure access'), visible: !previewVersion(), click: () => isAccessModalShown(!isAccessModalShown())">
<i class="fa fa-lock"></i>
</button>
<!-- /ko -->
Expand Down
50 changes: 22 additions & 28 deletions js/pages/cohort-definitions/cohort-definition-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ define(['jquery', 'knockout', 'text!./cohort-definition-manager.html',
'appConfig',
'components/cohortbuilder/CohortDefinition',
'services/CohortDefinition',
'services/ShareRoleCheck',
'services/MomentAPI',
'services/ConceptSet',
'services/Permission',
Expand Down Expand Up @@ -59,15 +58,14 @@ define(['jquery', 'knockout', 'text!./cohort-definition-manager.html',
'utilities/sql',
'components/conceptset/conceptset-list',
'components/name-validation',
'components/versions/versions',
'components/versions/versions'
], function (
$,
ko,
view,
config,
CohortDefinition,
cohortDefinitionService,
shareRoleCheck,
cohortDefinitionService,
momentApi,
conceptSetService,
PermissionService,
Expand Down Expand Up @@ -200,28 +198,30 @@ define(['jquery', 'knockout', 'text!./cohort-definition-manager.html',
this.pollTimeoutId = null;
this.authApi = authApi;
this.config = config;

this.enablePermissionManagement = ko.observable(false);
this.enablePermissionManagement(config.enablePermissionManagement);

this.userCanShare = ko.observable(false);
if (config.permissionManagementRoleId === "") {
this.userCanShare(true);
} else {
shareRoleCheck.checkIfRoleCanShare(authApi.subject(), config.permissionManagementRoleId)
.then(res=>{
this.userCanShare(res);
})
.catch(error => {
console.error(error);
alert(ko.i18n('cohortDefinitions.cohortDefinitionManager.shareRoleCheck', 'Error when determining if user can share cohorts')());
});
}

this.relatedSourcecodesOptions = globalConstants.relatedSourcecodesOptions;
this.commonUtils = commonUtils;
this.isLoading = ko.observable(false);
this.currentCohortDefinition = sharedState.CohortDefinition.current;

PermissionService.decorateComponent(this, {
entityTypeGetter: () => entityType.COHORT_DEFINITION,
entityIdGetter: () => this.currentCohortDefinition().id(),
createdByUsernameGetter: () => this.currentCohortDefinition() && this.currentCohortDefinition().createdBy()
&& this.currentCohortDefinition().createdBy().login
});

this.enablePermissionManagement = ko.observable(config.enablePermissionManagement);
if (config.enablePermissionManagement) {
this.userCanShare = ko.observable(
(config.limitedPermissionManagement &&
authApi.isPermittedGlobalShareArtifact()) ||
(!config.limitedPermissionManagement &&
this.isOwner())
);
} else {
this.userCanShare = ko.observable(false);
}

this.currentCohortDefinitionMode = sharedState.CohortDefinition.mode;
this.currentCohortDefinitionInfo = sharedState.CohortDefinition.info;
this.cohortDefinitionSourceInfo = sharedState.CohortDefinition.sourceInfo;
Expand Down Expand Up @@ -868,12 +868,6 @@ define(['jquery', 'knockout', 'text!./cohort-definition-manager.html',

}

PermissionService.decorateComponent(this, {
entityTypeGetter: () => entityType.COHORT_DEFINITION,
entityIdGetter: () => this.currentCohortDefinition().id(),
createdByUsernameGetter: () => this.currentCohortDefinition() && this.currentCohortDefinition().createdBy()
&& this.currentCohortDefinition().createdBy().login
});

TagsService.decorateComponent(this, {
assetTypeGetter: () => TagsService.ASSET_TYPE.COHORT_DEFINITION,
Expand Down
2 changes: 1 addition & 1 deletion js/pages/concept-sets/conceptset-manager.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<button type="button" class="btn btn-primary" data-bind="visible: !previewVersion(), click: optimize, css: { disabled: !canOptimize() || isProcessing() }, text: ko.i18n('cs.manager.optimize', 'Optimize')"></button>

<!-- ko if: (enablePermissionManagement() === true && userCanShare() === true) -->
<button class="btn btn-primary" data-bind="visible: !previewVersion() && isOwner(), click: () => isAccessModalShown(!isAccessModalShown()), title: ko.i18n('common.configureAccess', 'Configure access')">
<button class="btn btn-primary" data-bind="visible: !previewVersion(), click: () => isAccessModalShown(!isAccessModalShown()), title: ko.i18n('common.configureAccess', 'Configure access')">
<i class="fa fa-lock"></i>
</button>
<!-- /ko -->
Expand Down
48 changes: 20 additions & 28 deletions js/pages/concept-sets/conceptset-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ define([
'./const',
'const',
'components/conceptset/utils',
'services/Vocabulary',
'services/ShareRoleCheck',
'services/Vocabulary',
'services/Permission',
'services/Tags',
'components/security/access/const',
Expand Down Expand Up @@ -56,8 +55,7 @@ define([
constants,
globalConstants,
utils,
vocabularyAPI,
shareRoleCheck,
vocabularyAPI,
GlobalPermissionService,
TagsService,
{ entityType },
Expand Down Expand Up @@ -177,24 +175,25 @@ define([
return this.currentConceptSet() && this.currentConceptSet().id > 0;
});

this.enablePermissionManagement = ko.observable(false);
this.enablePermissionManagement(config.enablePermissionManagement);

this.userCanShare = ko.observable(false);
if (config.permissionManagementRoleId === "") {
this.userCanShare(true);
} else {
shareRoleCheck.checkIfRoleCanShare(authApi.subject(), config.permissionManagementRoleId)
.then(res=>{
this.userCanShare(res);
})
.catch(error => {
console.error(error);
alert(ko.i18n('conceptSets.conceptSetManager.shareRoleCheck', 'Error when determining if user can share concept sets')());
});
}
GlobalPermissionService.decorateComponent(this, {
entityTypeGetter: () => entityType.CONCEPT_SET,
entityIdGetter: () => this.currentConceptSet() && this.currentConceptSet().id,
createdByUsernameGetter: () => this.currentConceptSet() && this.currentConceptSet().createdBy
&& this.currentConceptSet().createdBy.login
});

this.enablePermissionManagement = ko.observable(config.enablePermissionManagement);
if (config.enablePermissionManagement) {
this.userCanShare = ko.observable(
(config.limitedPermissionManagement &&
authApi.isPermittedGlobalShareArtifact()) ||
(!config.limitedPermissionManagement &&
this.isOwner())
);
} else {
this.userCanShare = ko.observable(false);
}


this.isSaving = ko.observable(false);
this.isDeleting = ko.observable(false);
this.isOptimizing = ko.observable(false);
Expand Down Expand Up @@ -352,13 +351,6 @@ define([

this.activeUtility = ko.observable("");

GlobalPermissionService.decorateComponent(this, {
entityTypeGetter: () => entityType.CONCEPT_SET,
entityIdGetter: () => this.currentConceptSet() && this.currentConceptSet().id,
createdByUsernameGetter: () => this.currentConceptSet() && this.currentConceptSet().createdBy
&& this.currentConceptSet().createdBy.login
});

this.tags = ko.observableArray(this.currentConceptSet() && this.currentConceptSet().tags);
TagsService.decorateComponent(this, {
assetTypeGetter: () => TagsService.ASSET_TYPE.CONCEPT_SET,
Expand Down
29 changes: 18 additions & 11 deletions js/services/AuthAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,12 @@ define(function(require, exports) {
return isPermitted('cohortdefinition:' + id + ':copy:get');
}

var isPermittedGlobalShareArtifact = function() {
// special * permission (intended for admins) that allows the
// user to share any artifact with a "global reader role":
return isPermitted('artifact:global:share:put');
}

var isPermittedUpdateCohort = function(id) {
var permission = 'cohortdefinition:' + id + ':put';
return isPermitted(permission);
Expand All @@ -407,17 +413,17 @@ define(function(require, exports) {
}

var isPermittedGenerateCohort = function(cohortId, sourceKey) {
var v = isPermitted('cohortdefinition:' + cohortId + ':generate:' + sourceKey + ':get') &&
isPermitted('cohortdefinition:' + cohortId + ':info:get');

// By default, everyone can generate any artifact they have
// permission to read. If a permissionManagementRoleId has
// been assigned, (non- empty string assignment), the default
// generate functionality is not desired. Rather, users will have to
// have a role that allows them to update the specific cohort definition.
if (config.permissionManagementRoleId !== ""){
v = v && isPermitted('cohortdefinition:' + cohortId + ':put')
}
var v = isPermitted('cohortdefinition:' + cohortId + ':generate:' + sourceKey + ':get') &&
isPermitted('cohortdefinition:' + cohortId + ':info:get');

// By default, everyone can generate any artifact they have
// permission to read. If limitedPermissionManagement has
// been set to true, the default
// generate functionality is not desired. Rather, users will have to
// have a permission that allows them to update the specific cohort definition.
if (config.limitedPermissionManagement){
v = v && isPermitted('cohortdefinition:' + cohortId + ':put')
}
return v
}

Expand Down Expand Up @@ -586,6 +592,7 @@ define(function(require, exports) {
isPermittedReadCohort: isPermittedReadCohort,
isPermittedCreateCohort: isPermittedCreateCohort,
isPermittedCopyCohort: isPermittedCopyCohort,
isPermittedGlobalShareArtifact: isPermittedGlobalShareArtifact,
isPermittedUpdateCohort: isPermittedUpdateCohort,
isPermittedDeleteCohort: isPermittedDeleteCohort,
isPermittedGenerateCohort: isPermittedGenerateCohort,
Expand Down
32 changes: 0 additions & 32 deletions js/services/ShareRoleCheck.js

This file was deleted.

0 comments on commit 34d7425

Please sign in to comment.