[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318767322
##
File path:
solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java
##
@@ -96,35 +96,35 @@ public void testBasicAuth() throws Exception {
String baseUrl = buildUrl(jetty.getLocalPort(), "/solr");
httpSolrClient = getHttpSolrClient(baseUrl);
- verifySecurityStatus(cl, baseUrl + authcPrefix, "/errorMessages", null,
20);
Review comment:
My latest simplifications reduces the patch from 1446 to 391 lines
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318763377
##
File path:
solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java
##
@@ -96,35 +96,35 @@ public void testBasicAuth() throws Exception {
String baseUrl = buildUrl(jetty.getLocalPort(), "/solr");
httpSolrClient = getHttpSolrClient(baseUrl);
- verifySecurityStatus(cl, baseUrl + authcPrefix, "/errorMessages", null,
20);
Review comment:
So my point was that there is no need to change the test behavior for this
change, we can instead add the blockUnknown=false to these tests (which is
exactly what users would do to keep existing behavior) and that's it.
I opened a PR https://github.com/MarcusSorealheis/lucene-solr/pull/1 with my
proposal which removes a bunch of unnecessary and complicating changes. You
should be able to merge it into this PR :)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318452959
##
File path:
solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java
##
@@ -96,35 +96,35 @@ public void testBasicAuth() throws Exception {
String baseUrl = buildUrl(jetty.getLocalPort(), "/solr");
httpSolrClient = getHttpSolrClient(baseUrl);
- verifySecurityStatus(cl, baseUrl + authcPrefix, "/errorMessages", null,
20);
Review comment:
> you cannot anymore, which makes sense
With an explicit `blockUnknown=false` sure you can do anonymous requests if
those are not covered by any of the permissions set up for authentication. So
also this test class could be reverted to initial state and just explicitly set
blockUnknown=false -- as long as blockUnknown=true is tested elsewhere.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318451600
##
File path:
solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java
##
@@ -114,7 +114,7 @@ public void testBasicAuth() throws Exception {
String baseUrl = randomJetty.getBaseUrl().toString();
verifySecurityStatus(cl, baseUrl + authcPrefix, "/errorMessages", null,
20);
zkClient().setData("/security.json", STD_CONF.replaceAll("'",
"\"").getBytes(UTF_8), true);
- verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication/class",
"solr.BasicAuthPlugin", 20);
+ verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication/class",
"solr.BasicAuthPlugin", 20, "solr", "SolrRocks");
Review comment:
The `protected static final String STD_CONF` in that test class actually
*does* define an initial user...
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318417151
##
File path:
solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java
##
@@ -96,35 +96,35 @@ public void testBasicAuth() throws Exception {
String baseUrl = buildUrl(jetty.getLocalPort(), "/solr");
httpSolrClient = getHttpSolrClient(baseUrl);
- verifySecurityStatus(cl, baseUrl + authcPrefix, "/errorMessages", null,
20);
Review comment:
Same comment here as for BasicAuthIntegrationTest - this test tries to prove
that even with auth enabled you can do certain actions without creds.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318414048
##
File path:
solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java
##
@@ -275,7 +275,7 @@ private void assertThreeAdminEvents() throws Exception {
private static String AUTH_SECTION = ",\n" +
" \"authentication\":{\n" +
- "\"blockUnknown\":\"false\",\n" +
+ "\"blockUnknown\":\"true\",\n" +
Review comment:
Why does this explicit setting need to change? I cannot see that it changes
anything elsewhere in the test?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the
default behavior of the basic authentication plugin.
URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318416829
##
File path:
solr/core/src/test/org/apache/solr/security/BasicAuthIntegrationTest.java
##
@@ -114,7 +114,7 @@ public void testBasicAuth() throws Exception {
String baseUrl = randomJetty.getBaseUrl().toString();
verifySecurityStatus(cl, baseUrl + authcPrefix, "/errorMessages", null,
20);
zkClient().setData("/security.json", STD_CONF.replaceAll("'",
"\"").getBytes(UTF_8), true);
- verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication/class",
"solr.BasicAuthPlugin", 20);
+ verifySecurityStatus(cl, baseUrl + authcPrefix, "authentication/class",
"solr.BasicAuthPlugin", 20, "solr", "SolrRocks");
Review comment:
A general comment for this test class: Why not instead add
blockUnknown=false to `STD_CONF`? We do not necessarily need to test that the
default is true in this test. With the current changes, the nature of the test
has changed and does no longer test that certain actions can be done without
credentials while other need those.
As long as we have proper testing of the behavior of the new default we
should not need to change all other tests?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin. URL: https://github.com/apache/lucene-solr/pull/805#discussion_r318417661 ## File path: solr/solr-ref-guide/src/basic-authentication-plugin.adoc ## @@ -60,7 +60,7 @@ There are several things defined in this file: Save your settings to a file called `security.json` locally. If you are using Solr in standalone mode, you should put this file in `$SOLR_HOME`. -If `blockUnknown` does not appear in the `security.json` file, it will default to `false`. This has the effect of not requiring authentication at all. In some cases, you may want this; for example, if you want to have `security.json` in place but aren't ready to enable authentication. However, you will want to ensure that this parameter is set to `true` in order for authentication to be truly enabled in your system. +If `blockUnknown` does not appear in the `security.json` file, it will default to `true`. This has the effect of requiring authentication for HTTP access to Solr. In some cases, you may not want authentication after enabling the plugin; for example, if you want to have `security.json` in place but aren't ready to enable authentication. However, you will want to ensure that `blockUnknown` is set to `true` or omitted entirely in order for authentication to be truly enabled in your system. Review comment: >in order for authentication to be truly enabled in your system. Change to "...in order for authentication to be enforced for all requests to your system" 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin. URL: https://github.com/apache/lucene-solr/pull/805#discussion_r309926547 ## File path: solr/CHANGES.txt ## @@ -57,6 +57,8 @@ Upgrade Notes * SOLR-13596: Deprecated GroupingSpecification methods are removed. (Munendra S N) +* SOLR-13649: When Basic Authentication is enabled, users will be required to enter credentials to access the Admin UI and associated operations by default. The blockUnknown parameter can still be set to false to disable the need to authenticate. (marcussorealheis) Review comment: I'd simply write **BasicAuth property 'blockUknown' now defaults to 'true'** here. Then add a section under "Upgrade Notes" where you describe how this is backward incompatible, i.e. if you need `blockUnknown=false` behavior it must now be explicitly stated in `security.json`. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin.
janhoy commented on a change in pull request #805: SOLR-13649 change the default behavior of the basic authentication plugin. URL: https://github.com/apache/lucene-solr/pull/805#discussion_r309926975 ## File path: solr/solr-ref-guide/src/basic-authentication-plugin.adoc ## @@ -60,7 +60,7 @@ There are several things defined in this file: Save your settings to a file called `security.json` locally. If you are using Solr in standalone mode, you should put this file in `$SOLR_HOME`. -If `blockUnknown` does not appear in the `security.json` file, it will default to `false`. This has the effect of not requiring authentication at all. In some cases, you may want this; for example, if you want to have `security.json` in place but aren't ready to enable authentication. However, you will want to ensure that this parameter is set to `true` in order for authentication to be truly enabled in your system. +If `blockUnknown` does not appear in the `security.json` file, it will default to `true`. This has the effect of requiring authentication upon navigating to the Admin UI. In some cases, you may not want authentication after enabling the plugin; for example, if you want to have `security.json` in place but aren't ready to enable authentication. However, you will want to ensure that `blockUnknown` is set to `true` or omitted entirely in order for authentication to be truly enabled in your system. Review comment: Again, this is not really about Admin UI but about all HTTP access to Solr, including Admin UI. I'd re-phrase this. Please also add a section similar to the one in Upgrade Notes to a new `Major Changes in Solr 9` refGuide page. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
