You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by th...@apache.org on 2021/12/01 22:51:19 UTC
[lucene-solr] branch branch_8_11 updated: SOLR-13900: Reset index values on authorization rules after deleting by index (#434) (#2617)
This is an automated email from the ASF dual-hosted git repository.
thelabdude pushed a commit to branch branch_8_11
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8_11 by this push:
new fd6a057 SOLR-13900: Reset index values on authorization rules after deleting by index (#434) (#2617)
fd6a057 is described below
commit fd6a057b2fe84fd39dae638c092af1f50385709f
Author: Timothy Potter <th...@gmail.com>
AuthorDate: Wed Dec 1 15:50:59 2021 -0700
SOLR-13900: Reset index values on authorization rules after deleting by index (#434) (#2617)
---
solr/CHANGES.txt | 2 +
.../solr/security/AutorizationEditOperation.java | 105 ++++++++++++---------
.../BaseTestRuleBasedAuthorizationPlugin.java | 31 ++++++
3 files changed, 96 insertions(+), 42 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 69bdb5f..f45812d 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -21,6 +21,8 @@ Bug Fixes
* SOLR-15774: Avoid weird off-by-one errors with Angular's 'chosen' select box directive for the security and schema-designer screens in Admin UI (Timothy Potter)
+* SOLR-13900: Reset index values on authorization rules after deleting by index (Timothy Potter)
+
* SOLR-15794: Switching a PRS collection from true -> false -> true results in INACTIVE replicas (noble)
* SOLR-15813: Schema designer not handling `update.autoCreateFields` stored as a string (vs. boolean) in the config overlay (Timothy Potter)
diff --git a/solr/core/src/java/org/apache/solr/security/AutorizationEditOperation.java b/solr/core/src/java/org/apache/solr/security/AutorizationEditOperation.java
index a080d72..9049d96 100644
--- a/solr/core/src/java/org/apache/solr/security/AutorizationEditOperation.java
+++ b/solr/core/src/java/org/apache/solr/security/AutorizationEditOperation.java
@@ -56,12 +56,12 @@ enum AutorizationEditOperation {
@Override
@SuppressWarnings({"unchecked"})
public Map<String, Object> edit(Map<String, Object> latestConf, CommandOperation op) {
- Integer index = op.getInt("index", null);
- Integer beforeIdx = op.getInt("before",null);
+ Integer index = op.getInt(INDEX, null);
+ Integer beforeIdx = op.getInt(BEFORE, null);
Map<String, Object> dataMap = op.getDataMap();
if (op.hasError()) return null;
dataMap = getDeepCopy(dataMap, 3);
- dataMap.remove("before");
+ dataMap.remove(BEFORE);
if (beforeIdx != null && index != null) {
op.addError("Cannot use 'index' and 'before together ");
return null;
@@ -76,18 +76,14 @@ enum AutorizationEditOperation {
op.addError(e.getMessage());
return null;
}
- if(op.hasError()) return null;
- @SuppressWarnings({"rawtypes"})
- List<Map> permissions = getListValue(latestConf, "permissions");
- setIndex(permissions);
- @SuppressWarnings({"rawtypes"})
- List<Map> permissionsCopy = new ArrayList<>();
+ if (op.hasError()) return null;
+
+ List<Map<String, Object>> permissions = AutorizationEditOperation.ensureIndexOnPermissions(latestConf);
+ List<Map<String, Object>> permissionsCopy = new ArrayList<>();
boolean beforeSatisfied = beforeIdx == null;
boolean indexSatisfied = index == null;
- for (int i = 0; i < permissions.size(); i++) {
- @SuppressWarnings({"rawtypes"})
- Map perm = permissions.get(i);
- Integer thisIdx = (Integer) perm.get("index");
+ for (Map<String, Object> perm : permissions) {
+ Integer thisIdx = getIndex(perm);
if (thisIdx.equals(beforeIdx)) {
beforeSatisfied = true;
permissionsCopy.add(dataMap);
@@ -110,26 +106,31 @@ enum AutorizationEditOperation {
return null;
}
- if (!permissionsCopy.contains(dataMap)) permissionsCopy.add(dataMap);
- latestConf.put("permissions", permissionsCopy);
+ // just add the new permission at the end if not already added via index / before
+ if (!permissionsCopy.contains(dataMap)) {
+ permissionsCopy.add(dataMap);
+ }
+
setIndex(permissionsCopy);
+
+ latestConf.put(PERMISSIONS, permissionsCopy);
return latestConf;
}
},
UPDATE_PERMISSION("update-permission") {
@Override
- @SuppressWarnings({"unchecked", "rawtypes"})
public Map<String, Object> edit(Map<String, Object> latestConf, CommandOperation op) {
- Integer index = op.getInt("index");
+ Integer index = op.getInt(INDEX);
if (op.hasError()) return null;
- List<Map> permissions = (List<Map>) getListValue(latestConf, "permissions");
- setIndex(permissions);
- for (Map permission : permissions) {
- if (index.equals(permission.get("index"))) {
- LinkedHashMap copy = new LinkedHashMap<>(permission);
+
+ List<Map<String, Object>> permissions = AutorizationEditOperation.ensureIndexOnPermissions(latestConf);
+ for (Map<String, Object> permission : permissions) {
+ if (index.equals(getIndex(permission))) {
+ LinkedHashMap<String, Object> copy = new LinkedHashMap<>(permission);
copy.putAll(op.getDataMap());
op.setCommandData(copy);
+ latestConf.put(PERMISSIONS, permissions);
return SET_PERMISSION.edit(latestConf, op);
}
}
@@ -139,32 +140,31 @@ enum AutorizationEditOperation {
},
DELETE_PERMISSION("delete-permission") {
@Override
- @SuppressWarnings({"unchecked"})
public Map<String, Object> edit(Map<String, Object> latestConf, CommandOperation op) {
Integer id = op.getInt("");
- if(op.hasError()) return null;
- @SuppressWarnings({"rawtypes"})
- List<Map> p = getListValue(latestConf, "permissions");
- setIndex(p);
- @SuppressWarnings({"rawtypes"})
- List<Map> c = p.stream().filter(map -> !id.equals(map.get("index"))).collect(Collectors.toList());
- if(c.size() == p.size()){
- op.addError("No such index :"+ id);
+ if (op.hasError()) return null;
+
+ // find the permission to delete using the "index"
+ List<Map<String, Object>> p = AutorizationEditOperation.ensureIndexOnPermissions(latestConf);
+ if (p.stream().anyMatch(map -> id.equals(getIndex(map)))) {
+ // filter out the deleted index, then re-order the remaining indexes
+ List<Map<String, Object>> c = p.stream().filter(map -> !id.equals(getIndex(map))).collect(Collectors.toList());
+ setIndex(c);
+ latestConf.put(PERMISSIONS, c);
+ return latestConf;
+ } else {
+ op.addError("No such index: " + id);
return null;
}
- latestConf.put("permissions", c);
- return latestConf;
}
};
- public abstract Map<String, Object> edit(Map<String, Object> latestConf, CommandOperation op);
+ private static final String PERMISSIONS = "permissions";
+ private static final String INDEX = "index";
+ private static final String BEFORE = "before";
public final String name;
- public String getOperationName() {
- return name;
- }
-
AutorizationEditOperation(String s) {
this.name = s;
}
@@ -174,9 +174,30 @@ enum AutorizationEditOperation {
return null;
}
- @SuppressWarnings({"unchecked", "rawtypes"})
- static void setIndex(List<Map> permissionsCopy) {
- AtomicInteger counter = new AtomicInteger(0);
- permissionsCopy.stream().forEach(map -> map.put("index", counter.incrementAndGet()));
+ static void setIndex(List<Map<String, Object>> permissionsCopy) {
+ final AtomicInteger counter = new AtomicInteger(0);
+ permissionsCopy.forEach(map -> map.put(INDEX, counter.incrementAndGet()));
+ }
+
+ static Integer getIndex(final Map<String, Object> permission) {
+ final Object idx = permission.get(INDEX);
+ return (idx instanceof Number) ? ((Number) idx).intValue() : -1;
+ }
+
+ @SuppressWarnings("unchecked")
+ private static List<Map<String, Object>> ensureIndexOnPermissions(Map<String, Object> latestConf) {
+ List<Map<String, Object>> permissions = (List<Map<String, Object>>) getListValue(latestConf, PERMISSIONS);
+ // see if any permissions have "index" set ...
+ if (permissions.stream().noneMatch(map -> map.containsKey(INDEX))) {
+ // "index" not set on the list of permissions, add it now ...
+ setIndex(permissions);
+ }
+ return permissions;
+ }
+
+ public abstract Map<String, Object> edit(Map<String, Object> latestConf, CommandOperation op);
+
+ public String getOperationName() {
+ return name;
}
}
diff --git a/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java b/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java
index e52b09a..61ca758 100644
--- a/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java
+++ b/solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java
@@ -476,13 +476,44 @@ public class BaseTestRuleBasedAuthorizationPlugin extends SolrTestCaseJ4 {
assertEquals("/a/b", perms.getVal("permissions[1]/path"));
perms.runCmd("{update-permission : {index : 2, method : POST }}", true);
assertEquals("POST", perms.getVal("permissions[1]/method"));
+ assertEquals("/a/b", perms.getVal("permissions[1]/path"));
+
perms.runCmd("{set-permission : {name : read, collection : y, role:[guest, dev] , before :2}}", true);
assertNotNull(perms.getVal("permissions[2]"));
assertEquals("y", perms.getVal("permissions[1]/collection"));
assertEquals("read", perms.getVal("permissions[1]/name"));
+ assertEquals("POST", perms.getVal("permissions[2]/method"));
+ assertEquals("/a/b", perms.getVal("permissions[2]/path"));
+
perms.runCmd("{delete-permission : 3}", true);
assertTrue(captureErrors(perms.parsedCommands).isEmpty());
assertEquals("y", perms.getVal("permissions[1]/collection"));
+
+ List<Map<String,Object>> permList = (List<Map<String,Object>>)perms.getVal("permissions");
+ assertEquals(2, permList.size());
+ assertEquals("config-edit", perms.getVal("permissions[0]/name"));
+ assertEquals(1, perms.getVal("permissions[0]/index"));
+ assertEquals("read", perms.getVal("permissions[1]/name"));
+ assertEquals(2, perms.getVal("permissions[1]/index"));
+
+ // delete a non-existent permission
+ perms.runCmd("{delete-permission : 3}", false);
+ assertEquals(1, perms.parsedCommands.size());
+ assertEquals(1, perms.parsedCommands.get(0).getErrors().size());
+ assertEquals("No such index: 3", perms.parsedCommands.get(0).getErrors().get(0));
+
+ perms.runCmd("{delete-permission : 1}", true);
+ assertTrue(captureErrors(perms.parsedCommands).isEmpty());
+ permList = (List<Map<String,Object>>)perms.getVal("permissions");
+ assertEquals(1, permList.size());
+ // indexes should have been re-ordered after the delete, so now "read" has index==1
+ assertEquals("read", perms.getVal("permissions[0]/name"));
+ assertEquals(1, perms.getVal("permissions[0]/index"));
+ // delete last remaining
+ perms.runCmd("{delete-permission : 1}", true);
+ assertTrue(captureErrors(perms.parsedCommands).isEmpty());
+ permList = (List<Map<String,Object>>)perms.getVal("permissions");
+ assertEquals(0, permList.size());
}
static class Perms {