epugh commented on code in PR #4916:
URL: https://github.com/apache/solr/pull/4916#discussion_r4029246192


##########
solr/webapp/web/js/angular/controllers/security.js:
##########
@@ -1201,90 +1229,90 @@ solrAdminApp.controller('SecurityController', function 
($scope, $timeout, $cooki
       perms = $scope.upsertRole.grantedPerms;
     }
 
-    // go get the latest role mappings ...
-    Security.get({path: "authorization"}, function (data) {
-      var authz = $scope.findEditableAuthz(data);
-      if (!authz) {
-        $scope.validationError = "User roles not editable via the UI!";
-        return;
-      }
+    // Assigns `name` to one user, replacing their role list.
+    function assignRoleToUser(user, done) {
+      AuthorizationV2.getUserRoles(BASIC_SCHEME, user, function (error, data, 
response) {
+        if (error) { ApiErrorHandler.handle(response); done(); return; }
+        var roles = data.roles.includes(name) ? data.roles : 
data.roles.concat([name]);
+        AuthorizationV2.setUserRoles(BASIC_SCHEME, user, {roles: roles}, 
function (error2, data2, response2) {
+          if (error2) { ApiErrorHandler.handle(response2); }
+          done();
+        });
+      });
+    }
 
-      var userRoles = authz["user-role"];
-      var setUserRoles = {};
-      for (u in usersForRole) {
-        var user = usersForRole[u];
-        var currentRoles = user in userRoles ? asList(userRoles[user]) : [];
-        // add the new role for this user if needed
-        if (!currentRoles.includes(name)) {
-          currentRoles.push(name);
-        }
-        setUserRoles[user] = currentRoles;
-      }
+    // Grants `name` to one permission - updating it if it already exists, 
creating it (only if
+    // predefined) otherwise.
+    function grantPermissionToRole(permName, existingPerms, done) {
+      var existingPerm = existingPerms.find(p => p.name === permName);
 
-      var cmdJson = $scope.wrapSchemeCmd("set-user-role", setUserRoles);
-      Security.post({path: "authorization"}, cmdJson, function (data2) {
+      function afterGrant(error, response) {
+        if (error) { ApiErrorHandler.handle(response); }
+        done();
+      }
 
-        var errorCause = checkError(data2);
-        if (errorCause != null) {
-          $scope.securityAPIError = "set-user-role for role "+name+" failed 
due to: "+errorCause;
-          $scope.securityAPIErrorDetails = JSON.stringify(data2);
-          return;
+      if (existingPerm) {
+        var roles = asList(existingPerm.role);
+        if (!roles.includes(name)) {
+          roles = roles.concat([name]);
         }
+        AuthorizationV2.updatePermission(existingPerm.index, {role: roles}, 
function (error, data, response) {
+          afterGrant(error, response);
+        });
+      } else if ($scope.predefinedPermissions.includes(permName)) {
+        AuthorizationV2.createPermission({name: permName, role: [name]}, 
function (error, data, response) {
+          afterGrant(error, response);
+        });
+      } else {
+        done(); // custom permission that doesn't exist yet - nothing to grant
+      }
+    }
 
-        function roleReflected(data3) {
-          var authz3 = $scope.findEditableAuthz(data3);
-          if (!authz3) return true;
-          return usersForRole.every(u => 
asList(authz3["user-role"][u]).includes(name));
+    function runTasks(tasks, done) {
+      var remaining = tasks.length;
+      if (remaining === 0) {
+        done();
+        return;
+      }
+      tasks.forEach(task => task(function () {
+        if (--remaining === 0) {
+          done();
         }
+      }));
+    }
 
-        if (perms.length === 0) {
-          // close dialog and refresh the tables ...
-          $scope.toggleRoleDialog();
-          whenReflected("authorization", roleReflected, 
$scope.refreshSecurityPanel);
-          return;
-        }
+    // Once every write above has returned, this is the same single 
whenReflected("authorization",
+    // ...) poll the legacy command-batch code used - just checking the 
users/perms this dialog
+    // actually touched, rather than re-inventing per-resource polling against 
the new v2 GETs.
+    function finishUp(attemptedPerms) {
+      $scope.toggleRoleDialog();
+      whenReflected("authorization", function (data) {
+        var authz = $scope.findEditableAuthz(data);
+        if (!authz) return true;
+        var rolesOk = usersForRole.every(u => 
asList(authz["user-role"][u]).includes(name));
+        var permsOk = attemptedPerms.every(p => {
+          var have = permissionRoles(data, p);
+          return have != null && have.includes(name);
+        });
+        return rolesOk && permsOk;
+      }, $scope.refreshSecurityPanel);
+    }
 
-        var currentPerms = data.authorization["permissions"];
-        for (i in perms) {
-          let permName = perms[i];
-          var existingPerm = currentPerms.find(p => p.name === permName);
-
-          if (existingPerm) {
-            var roleList = [];
-            if (existingPerm.role) {
-              if (Array.isArray(existingPerm.role)) {
-                roleList = existingPerm.role;
-              } else {
-                roleList.push(existingPerm.role);
-              }
-            }
-            if (!roleList.includes(name)) {
-              roleList.push(name);
-            }
-            existingPerm.role = roleList;
-            Security.post({path: "authorization"}, { "update-permission": 
existingPerm }, function (data3) {
-              whenReflected("authorization", function (data4) {
-                var have = permissionRoles(data4, permName);
-                return roleReflected(data4) && have != null && 
have.includes(name);
-              }, $scope.refreshSecurityPanel);
-            });
-          } else {
-            // new perm ... must be a predefined ...
-            if ($scope.predefinedPermissions.includes(permName)) {
-              var setPermission = {name: permName, role:[name]};
-              Security.post({path: "authorization"}, { "set-permission": 
setPermission }, function (data3) {
-                whenReflected("authorization", function (data4) {
-                  var have = permissionRoles(data4, permName);
-                  return roleReflected(data4) && have != null && 
have.includes(name);
-                }, $scope.refreshSecurityPanel);
-              });
-            } // else ignore it
-          }
-        }
-        $scope.toggleRoleDialog();
+    var userTasks = usersForRole.map(u => cb => assignRoleToUser(u, cb));
+    if (perms.length === 0) {
+      runTasks(userTasks, () => finishUp([]));

Review Comment:
   we may not need finishUp after we tweaked how we interact with Zookeeper to 
not ever use a cached copy of the security.json file!



##########
solr/webapp/web/js/angular/controllers/security.js:
##########
@@ -1201,90 +1229,90 @@ solrAdminApp.controller('SecurityController', function 
($scope, $timeout, $cooki
       perms = $scope.upsertRole.grantedPerms;
     }
 
-    // go get the latest role mappings ...
-    Security.get({path: "authorization"}, function (data) {
-      var authz = $scope.findEditableAuthz(data);
-      if (!authz) {
-        $scope.validationError = "User roles not editable via the UI!";
-        return;
-      }
+    // Assigns `name` to one user, replacing their role list.
+    function assignRoleToUser(user, done) {
+      AuthorizationV2.getUserRoles(BASIC_SCHEME, user, function (error, data, 
response) {
+        if (error) { ApiErrorHandler.handle(response); done(); return; }
+        var roles = data.roles.includes(name) ? data.roles : 
data.roles.concat([name]);
+        AuthorizationV2.setUserRoles(BASIC_SCHEME, user, {roles: roles}, 
function (error2, data2, response2) {
+          if (error2) { ApiErrorHandler.handle(response2); }
+          done();
+        });
+      });
+    }
 
-      var userRoles = authz["user-role"];
-      var setUserRoles = {};
-      for (u in usersForRole) {
-        var user = usersForRole[u];
-        var currentRoles = user in userRoles ? asList(userRoles[user]) : [];
-        // add the new role for this user if needed
-        if (!currentRoles.includes(name)) {
-          currentRoles.push(name);
-        }
-        setUserRoles[user] = currentRoles;
-      }
+    // Grants `name` to one permission - updating it if it already exists, 
creating it (only if
+    // predefined) otherwise.
+    function grantPermissionToRole(permName, existingPerms, done) {
+      var existingPerm = existingPerms.find(p => p.name === permName);
 
-      var cmdJson = $scope.wrapSchemeCmd("set-user-role", setUserRoles);
-      Security.post({path: "authorization"}, cmdJson, function (data2) {
+      function afterGrant(error, response) {
+        if (error) { ApiErrorHandler.handle(response); }
+        done();
+      }
 
-        var errorCause = checkError(data2);
-        if (errorCause != null) {
-          $scope.securityAPIError = "set-user-role for role "+name+" failed 
due to: "+errorCause;
-          $scope.securityAPIErrorDetails = JSON.stringify(data2);
-          return;
+      if (existingPerm) {
+        var roles = asList(existingPerm.role);
+        if (!roles.includes(name)) {
+          roles = roles.concat([name]);
         }
+        AuthorizationV2.updatePermission(existingPerm.index, {role: roles}, 
function (error, data, response) {
+          afterGrant(error, response);
+        });
+      } else if ($scope.predefinedPermissions.includes(permName)) {
+        AuthorizationV2.createPermission({name: permName, role: [name]}, 
function (error, data, response) {
+          afterGrant(error, response);
+        });
+      } else {
+        done(); // custom permission that doesn't exist yet - nothing to grant
+      }
+    }
 
-        function roleReflected(data3) {
-          var authz3 = $scope.findEditableAuthz(data3);
-          if (!authz3) return true;
-          return usersForRole.every(u => 
asList(authz3["user-role"][u]).includes(name));
+    function runTasks(tasks, done) {
+      var remaining = tasks.length;
+      if (remaining === 0) {
+        done();
+        return;
+      }
+      tasks.forEach(task => task(function () {
+        if (--remaining === 0) {
+          done();
         }
+      }));
+    }
 
-        if (perms.length === 0) {
-          // close dialog and refresh the tables ...
-          $scope.toggleRoleDialog();
-          whenReflected("authorization", roleReflected, 
$scope.refreshSecurityPanel);
-          return;
-        }
+    // Once every write above has returned, this is the same single 
whenReflected("authorization",

Review Comment:
   We may not need this now that our API calls don't ever use a cached 
security.json file.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to