dsmiley commented on a change in pull request #372:
URL: https://github.com/apache/solr/pull/372#discussion_r737735230
##########
File path:
solr/core/src/java/org/apache/solr/handler/admin/SegmentsInfoRequestHandler.java
##########
@@ -434,4 +435,9 @@ public String getDescription() {
public Category getCategory() {
return Category.ADMIN;
}
+
+ @Override
+ public Name getPermissionName(AuthorizationContext request) {
+ return Name.CONFIG_READ_PERM;
Review comment:
Is it really "config" (configuration)? I think it's metrics.
##########
File path: solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java
##########
@@ -126,4 +127,9 @@ public void init(NamedList<?> args) {
if (nl!=null) subpaths = nl.getAll("subpath");
}
}
+
+ @Override
+ public Name getPermissionName(AuthorizationContext request) {
+ return Name.CONFIG_READ_PERM;
Review comment:
Only with "remote streaming" would this be bad but that's disabled by
default and should perhaps only be thought of as a local dev/POC scenario only.
##########
File path: solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java
##########
@@ -168,4 +169,8 @@ public Category getCategory() {
return Category.ADMIN;
}
+ @Override
+ public Name getPermissionName(AuthorizationContext request) {
+ return Name.CONFIG_READ_PERM;
Review comment:
Well it can be WRITE. Can we vary this based on the HTTP verb? Yet
sadly we permit mutations on GET...
##########
File path: solr/core/src/java/org/apache/solr/handler/DumpRequestHandler.java
##########
@@ -126,4 +127,9 @@ public void init(NamedList<?> args) {
if (nl!=null) subpaths = nl.getAll("subpath");
}
}
+
+ @Override
+ public Name getPermissionName(AuthorizationContext request) {
+ return Name.CONFIG_READ_PERM;
Review comment:
It just dumps the input back to output; right? Shouldn't the completely
open?
##########
File path:
solr/core/src/java/org/apache/solr/handler/admin/ThreadDumpHandler.java
##########
@@ -172,4 +173,9 @@ public String getDescription() {
public Category getCategory() {
return Category.ADMIN;
}
+
+ @Override
+ public Name getPermissionName(AuthorizationContext request) {
+ return Name.CONFIG_READ_PERM;
Review comment:
Is it really "config" (configuration)? I think it's metrics.
##########
File path: solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
##########
@@ -51,9 +52,11 @@
import static org.apache.solr.core.RequestParams.USEPARAM;
/**
- *
+ * Base class for all request handlers.
+ * Since v9.0 all sub classes must implement {@link PermissionNameProvider} to
decide on required security permissions
Review comment:
I don't think we should add a statement like that. APIs evolve over
time and I'm dubious we need a chronology of that evolution in the javadocs.
##########
File path:
solr/core/src/java/org/apache/solr/handler/admin/PluginInfoHandler.java
##########
@@ -82,4 +83,9 @@ public String getDescription() {
public Category getCategory() {
return Category.ADMIN;
}
+
+ @Override
+ public Name getPermissionName(AuthorizationContext request) {
+ return Name.METRICS_READ_PERM;
Review comment:
Why? This isn't obvious to me but may be right.
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
