Merge pull request #34499 from dimagi/ad/warn-location-restricted-rol…

…es-assigning-unrestricted Warn users about unrestricted roles being assignable by a restricted role
dimagi · May 8, 2024 · eaedf42 · eaedf42
2 parents f32611e + 9361080
commit eaedf42
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 9 deletions.
diff --git a/corehq/apps/users/static/users/js/roles.js b/corehq/apps/users/static/users/js/roles.js
@@ -1,3 +1,5 @@
+'use strict';
+
 hqDefine('users/js/roles',[
     'jquery',
     'underscore',
@@ -77,7 +79,6 @@ hqDefine('users/js/roles',[
     };
 
     var RolesViewModel = function (o) {
-        'use strict';
         var self, root;
         self = root = {};
 
@@ -142,14 +143,14 @@ hqDefine('users/js/roles',[
                         };
                     }),
                 };
-
                 data.manageRoleAssignments = {
                     all: data.is_non_admin_editable,
                     specific: ko.utils.arrayMap(o.nonAdminRoles, function (role) {
                         return {
                             path: role._id,
                             name: role.name,
-                            value: data.assignable_by.indexOf(role._id) !== -1,
+                            value: ko.observable(data.assignable_by.indexOf(role._id) !== -1),
+                            access_all_locations: role.permissions.access_all_locations,
                         };
                     }),
                 };
@@ -191,6 +192,19 @@ hqDefine('users/js/roles',[
                 };
                 self.preventRoleDelete = data.preventRoleDelete;
                 self.hasUnpermittedLocationRestriction = data.has_unpermitted_location_restriction || false;
+
+                self.restrictRoleChecked = ko.computed(function () {
+                    return data.manageRoleAssignments.specific.some(role => role.value() && !role.access_all_locations);
+                });
+                self.showRestrictedLocationRoleAssignmentWarning = ko.computed(function () {
+                    return self.permissions.access_all_locations() && self.restrictRoleChecked();
+                });
+                self.cantAccessAllLocations = ko.computed(function () {
+                    return !self.hasUnpermittedLocationRestriction && !self.permissions.access_all_locations();
+                });
+                self.unrestrictedButRestrictedRoleCanAssign = ko.computed(function () {
+                    return self.permissions.access_all_locations() && self.restrictRoleChecked();
+                });
                 if (self.hasUnpermittedLocationRestriction) {
                     self.permissions.access_all_locations(true);
                 }

diff --git a/corehq/apps/users/templates/users/partials/edit_role_modal.html b/corehq/apps/users/templates/users/partials/edit_role_modal.html
@@ -1,7 +1,6 @@
 {% load i18n %}
 {% load hq_shared_tags %}
 
-
 <div data-bind="modal: roleBeingEdited" tabindex="-1" role="dialog" class="modal fade">
   <div data-bind="with: roleBeingEdited" class="modal-dialog">
     <form data-bind="submit: $root.submitNewRole">
@@ -299,7 +298,8 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
               </div>
             </div>
             <div class="form-group"
-                 data-bind="css: { 'has-error': hasUnpermittedLocationRestriction },
+                 data-bind="css: { 'has-error': hasUnpermittedLocationRestriction || unrestrictedButRestrictedRoleCanAssign,
+                                   'has-warning': cantAccessAllLocations },
                             visible: $root.canRestrictAccessByLocation || hasUnpermittedLocationRestriction">
               <label class="col-sm-4 control-label">
                 {% trans "Full Organization Access" %}
@@ -314,8 +314,8 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     {% trans "Allow role to access data from all locations." %}
                   </label>
                 </div>
-                <div class="help-block alert alert-warning"
-                     data-bind="visible: !hasUnpermittedLocationRestriction && !permissions.access_all_locations()">
+                <div class="help-block"
+                     data-bind="visible: cantAccessAllLocations">
                   {% blocktrans %}
                     Make sure any users assigned this role also have a location assigned to them.
                     Users without assigned locations will not be permitted to log in.
@@ -328,6 +328,12 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     full organization access for the assigned users.
                   {% endblocktrans %}
                 </div>
+                <div class="help-block" data-bind="visible: unrestrictedButRestrictedRoleCanAssign">
+                  {% blocktrans %}
+                    With the current configuration, this role will have full organization access and roles that can assign it will not.
+                    This is not recommended.
+                  {% endblocktrans %}
+                </div>
               </div>
             </div>
             <div class="form-group">
@@ -457,14 +463,14 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
               <label class="col-sm-4 control-label">
                 {% trans "Manage Role Assignment" %}
               </label>
-              <div class="col-sm-8 controls">
+              <div class="col-sm-8 controls", data-bind="css: { 'has-error': showRestrictedLocationRoleAssignmentWarning }">
                 <div class="panel panel-default">
                   <div class="panel-heading">
                     {% trans "Select which other roles can assign this role:" %}
                   </div>
                   <div class="panel-body"
                        data-bind="foreach: manageRoleAssignments.specific, slideVisible: !manageRoleAssignments.all()">
-                    <div class="form-check">
+                    <div class="form-check" data-bind="css: {'bg-danger': !access_all_locations() && $parent.permissions.access_all_locations()}">
                       <input type="checkbox"
                              data-bind="checked: value, attr: { 'id': 'role-' + path() + '-checkbox' }" />
                       <label data-bind="attr: { 'for': 'role-' + path() + '-checkbox' }">
@@ -473,6 +479,12 @@ <h4 class="modal-title" data-bind="text: modalTitle"></h4>
                     </div>
                   </div>
                 </div>
+                <div class="help-block" data-bind="visible: showRestrictedLocationRoleAssignmentWarning">
+                  {% blocktrans %}
+                    If the highlighted roles are selected, this role will have full organization access and be assignable
+                    by a role that does not. This is not recommended configuration.
+                  {% endblocktrans %}
+                </div>
               </div>
             </div>
             {% if request|request_has_privilege:"CUSTOM_DOMAIN_ALERTS" %}