Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9481

BasicAuthPlugin should support standalone mode

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.5, 7.0
    • Component/s: security
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:

      Description

      The BasicAuthPlugin currently only supports SolrCloud, and reads users and credentials from ZK /security.json

      Add support for standalone mode operation

      1. SOLR-9481.patch
        43 kB
        Jan Høydahl
      2. SOLR-9481.patch
        4 kB
        Jan Høydahl
      3. SOLR-9481-6x.patch
        46 kB
        Jan Høydahl
      4. SOLR-9481-6x.patch
        45 kB
        Jan Høydahl

        Issue Links

          Activity

          Hide
          janhoy Jan Høydahl added a comment -

          A first thought is to allow for security.json to be read locally from SOLR_HOME on every single node, next to solr.xml, and disallow editing API in standalone mode. Doable?

          Show
          janhoy Jan Høydahl added a comment - A first thought is to allow for security.json to be read locally from SOLR_HOME on every single node, next to solr.xml, and disallow editing API in standalone mode. Doable?
          Hide
          noble.paul Noble Paul added a comment -

          yeah, the first version can have a non-editable security.json. It can be loaded from the SOLR_HOME.

          I'm sure we must add support for standalone RulebasedAuthorization plugin as well

          Show
          noble.paul Noble Paul added a comment - yeah, the first version can have a non-editable security.json. It can be loaded from the SOLR_HOME. I'm sure we must add support for standalone RulebasedAuthorization plugin as well
          Hide
          janhoy Jan Høydahl added a comment -

          First patch. Only changed CoreContainer to support loading security.json from SOLR_HOME. This was all it takes to fix all Auth/Autz plugins to work in standalone mode. Tested manually for BasicAuthPlugin and also RuleBasedAuthorizationPlugin, they both work without any modifications. The edit API does not work, will probably throw some exception that ZK is not available...

          No automated tests yet.

          Show
          janhoy Jan Høydahl added a comment - First patch. Only changed CoreContainer to support loading security.json from SOLR_HOME. This was all it takes to fix all Auth/Autz plugins to work in standalone mode. Tested manually for BasicAuthPlugin and also RuleBasedAuthorizationPlugin , they both work without any modifications. The edit API does not work, will probably throw some exception that ZK is not available... No automated tests yet.
          Hide
          noble.paul Noble Paul added a comment -

          +1
          This is a good start. Disable the edit operations if you don't plan to support edit. Just respond with a message saying that edit is not supported in standalone mode

          Show
          noble.paul Noble Paul added a comment - +1 This is a good start. Disable the edit operations if you don't plan to support edit. Just respond with a message saying that edit is not supported in standalone mode
          Hide
          janhoy Jan Høydahl added a comment -

          I tried throwing an error if edit was used when not in ZK mode, but that unfortunately broke some tests which relied on module testing the edit feature in standalone mode
          Will try to find some way around it.

          Show
          janhoy Jan Høydahl added a comment - I tried throwing an error if edit was used when not in ZK mode, but that unfortunately broke some tests which relied on module testing the edit feature in standalone mode Will try to find some way around it.
          Hide
          janhoy Jan Høydahl added a comment -

          Regarding edit, I'm thinking perhaps we could instead retrofit the edit API to work in standalone mode too, by writing $SOLR_HOME/security.json locally. Of course, the admin must then perform the edit call to each and every node, but it would still be more convenient than having to copy and restart in the filesystem. Also we could implement polling of changed security.json on the file system and invoke a reload?

          Show
          janhoy Jan Høydahl added a comment - Regarding edit, I'm thinking perhaps we could instead retrofit the edit API to work in standalone mode too, by writing $SOLR_HOME/security.json locally. Of course, the admin must then perform the edit call to each and every node, but it would still be more convenient than having to copy and restart in the filesystem. Also we could implement polling of changed security.json on the file system and invoke a reload?
          Hide
          noble.paul Noble Paul added a comment -

          It's much better to use the API to edit the security configuration. When it's edited, you can reload the in memory object. Let's not do polling of files

          Show
          noble.paul Noble Paul added a comment - It's much better to use the API to edit the security configuration. When it's edited, you can reload the in memory object. Let's not do polling of files
          Hide
          janhoy Jan Høydahl added a comment -

          Yea, we probably don't need both. Maybe it would be convenient with a http://localhost:8983/solr/admin/authentication?reload=true API to force reload of file if you have changed it through e.g. uploading from local VCS? Alternatively, a http://localhost:8983/solr/admin/security endpoint which can GET/POST the entire security.json file, and which would work in cloud as well?

          Show
          janhoy Jan Høydahl added a comment - Yea, we probably don't need both. Maybe it would be convenient with a http://localhost:8983/solr/admin/authentication?reload=true API to force reload of file if you have changed it through e.g. uploading from local VCS? Alternatively, a http://localhost:8983/solr/admin/security endpoint which can GET/POST the entire security.json file, and which would work in cloud as well?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user janhoy opened a pull request:

          https://github.com/apache/lucene-solr/pull/95

          SOLR-9481: Authentication and Authorization plugins support for non-cloud

          Adds ability to use Auth/Autz plugins in standalone non-cloud mode.

          • Place `security.json` in $SOLR_HOME
          • Solr will initialise plugins from local file
          • Edits through API `/solr/admin/authentication` and `/solr/admin/authorization`supported
          • Each edit will update local copy of `security.json` and reload security config
          • If you have several nodes in master/slave setup, need to perform the edit on each node
          • Refactored `SecurityConfHandler`into a base class independent of ZK. Each sub class overrides methods `getSecurityConfig`, `getConf`, `persistConf` and `securityConfEdited`:
          • `SecurityConfHandlerZk` is instantiated if zkAware
          • `SecurityConfHandlerLocal` is instantiated if in local mode, reads/writes local file in SOLR_HOME
          • In local mode there is no callback when `security.json` changes, so `SecurityConfHandlerLocal` explicitly reloads security configs in its `securityConfEdited()` method
          • `MockSecurityHandler` used in tests persists to in-memory Map
          • New object `SecurityConfig` to hold security config, since `ZkStateReader.ConfigData` is tied to ZK.
          • New test case `BasicAuthStandaloneTest` spins up Jetty, writes local `security.json` through the persistConf API, adds a user through edit API, validates that permission is enforced and that local file contains user name.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/cominvent/lucene-solr solr9481

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/95.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #95


          commit 6c04d63e633f74b4df9f77ad78b2335d9dd87470
          Author: Jan Høydahl <janhoy@apache.org>
          Date: 2016-10-13T08:46:30Z

          SOLR-9481: Authentication and Authorization plugins now work in standalone mode, including edit API


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user janhoy opened a pull request: https://github.com/apache/lucene-solr/pull/95 SOLR-9481 : Authentication and Authorization plugins support for non-cloud Adds ability to use Auth/Autz plugins in standalone non-cloud mode. Place `security.json` in $SOLR_HOME Solr will initialise plugins from local file Edits through API `/solr/admin/authentication` and `/solr/admin/authorization`supported Each edit will update local copy of `security.json` and reload security config If you have several nodes in master/slave setup, need to perform the edit on each node Refactored `SecurityConfHandler`into a base class independent of ZK. Each sub class overrides methods `getSecurityConfig`, `getConf`, `persistConf` and `securityConfEdited`: `SecurityConfHandlerZk` is instantiated if zkAware `SecurityConfHandlerLocal` is instantiated if in local mode, reads/writes local file in SOLR_HOME In local mode there is no callback when `security.json` changes, so `SecurityConfHandlerLocal` explicitly reloads security configs in its `securityConfEdited()` method `MockSecurityHandler` used in tests persists to in-memory Map New object `SecurityConfig` to hold security config, since `ZkStateReader.ConfigData` is tied to ZK. New test case `BasicAuthStandaloneTest` spins up Jetty, writes local `security.json` through the persistConf API, adds a user through edit API, validates that permission is enforced and that local file contains user name. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cominvent/lucene-solr solr9481 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/95.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #95 commit 6c04d63e633f74b4df9f77ad78b2335d9dd87470 Author: Jan Høydahl <janhoy@apache.org> Date: 2016-10-13T08:46:30Z SOLR-9481 : Authentication and Authorization plugins now work in standalone mode, including edit API
          Hide
          janhoy Jan Høydahl added a comment -

          Also attaching path, however feel free to review and comment in the PR https://github.com/apache/lucene-solr/pull/95/files

          Show
          janhoy Jan Høydahl added a comment - Also attaching path, however feel free to review and comment in the PR https://github.com/apache/lucene-solr/pull/95/files
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user janhoy commented on the issue:

          https://github.com/apache/lucene-solr/pull/95

          Fixed an endless loop on edit

          Show
          githubbot ASF GitHub Bot added a comment - Github user janhoy commented on the issue: https://github.com/apache/lucene-solr/pull/95 Fixed an endless loop on edit
          Hide
          janhoy Jan Høydahl added a comment -

          Noble Paul, would you care to take a look? I'm quite excited that we seem to be able to make all Auth and Autz plugins work in standalone mode (at least singlenode) like this, without need for another config format or API.

          Show
          janhoy Jan Høydahl added a comment - Noble Paul , would you care to take a look? I'm quite excited that we seem to be able to make all Auth and Autz plugins work in standalone mode (at least singlenode) like this, without need for another config format or API.
          Hide
          noble.paul Noble Paul added a comment -

          Sure , will do

          Show
          noble.paul Noble Paul added a comment - Sure , will do
          Hide
          noble.paul Noble Paul added a comment -

          +1 LGTM

          Show
          noble.paul Noble Paul added a comment - +1 LGTM
          Hide
          janhoy Jan Høydahl added a comment -

          I was wondering whether the HTTP response from the edit API should return a message

          {"message": "Security configuration updated locally on node <nodename> only"}
          

          ...so that if people use cURL, PostMAN or similar they will get a reminder that the config change is not replicated to the slaves...

          Show
          janhoy Jan Høydahl added a comment - I was wondering whether the HTTP response from the edit API should return a message {"message": "Security configuration updated locally on node <nodename> only"} ...so that if people use cURL, PostMAN or similar they will get a reminder that the config change is not replicated to the slaves...
          Hide
          noble.paul Noble Paul added a comment -

          I don't think it's necessary. The fact that the server is using zk or not is a deployment detail. The API user should have the same experience always

          Show
          noble.paul Noble Paul added a comment - I don't think it's necessary. The fact that the server is using zk or not is a deployment detail. The API user should have the same experience always
          Hide
          janhoy Jan Høydahl added a comment -

          The API user should have the same experience always

          That's why I thought of adding the message. In Cloud, you only need to issue the edit operation once, but in master/slave you need to send the same request to every node in your cluster.

          We could add a param nodelist=node1:8983,node2:8983... to the API which would be required in non-cloud mode and let you get away with one request. But I don't like the added complexity of what to do if one node is down etc. Better let the client visit each node and handle exceptions as needed...

          Show
          janhoy Jan Høydahl added a comment - The API user should have the same experience always That's why I thought of adding the message. In Cloud, you only need to issue the edit operation once, but in master/slave you need to send the same request to every node in your cluster. We could add a param nodelist=node1:8983,node2:8983... to the API which would be required in non-cloud mode and let you get away with one request. But I don't like the added complexity of what to do if one node is down etc. Better let the client visit each node and handle exceptions as needed...
          Hide
          noble.paul Noble Paul added a comment -

          Let's not support the nodelist thing. We really don't support any operation that spans master and slave . So, this doesn't have to be special

          Show
          noble.paul Noble Paul added a comment - Let's not support the nodelist thing. We really don't support any operation that spans master and slave . So, this doesn't have to be special
          Hide
          janhoy Jan Høydahl added a comment -

          Ok, I think we're approaching committable state. Even better if a few more people would test though. If silence I plan to commit on thusday/friday.

          I have not tested with Kerberos, but in my understanding, it should "just work" since it will get served the correct config object no matter where it originates from.. But there could be other properties of Kerberos that breaks in a non-cloud setting?

          I plan to document in the RefGuide in a generic way and make a statement that Security plugins are available in both Cloud and standalone mode. Then go on to make sure security.json and Edit APIs are described without ties to Cloud/ZK in the guide. Then there would be perhaps two very short step-by-step HOWTOs on the main "Auth&Autz" page, for Cloud and Standalone respectively.

          Show
          janhoy Jan Høydahl added a comment - Ok, I think we're approaching committable state. Even better if a few more people would test though. If silence I plan to commit on thusday/friday. I have not tested with Kerberos, but in my understanding, it should "just work" since it will get served the correct config object no matter where it originates from.. But there could be other properties of Kerberos that breaks in a non-cloud setting? I plan to document in the RefGuide in a generic way and make a statement that Security plugins are available in both Cloud and standalone mode. Then go on to make sure security.json and Edit APIs are described without ties to Cloud/ZK in the guide. Then there would be perhaps two very short step-by-step HOWTOs on the main "Auth&Autz" page, for Cloud and Standalone respectively.
          Hide
          janhoy Jan Høydahl added a comment -

          I'm going to commit this to master first and let it bake and receive feedback for some days. It is probably easier for people to take it for a spin when it's on master.

          Show
          janhoy Jan Høydahl added a comment - I'm going to commit this to master first and let it bake and receive feedback for some days. It is probably easier for people to take it for a spin when it's on master.
          Hide
          janhoy Jan Høydahl added a comment -

          Hmm, need to choose another SOLR_HOME for testing write of security.json:

             [junit4] ERROR   0.57s J1 | BasicAuthStandaloneTest.testBasicAuth <<<
             [junit4]    > Throwable #1: org.apache.solr.common.SolrException: Failed persisting security.json to /Users/janhoy/git/lucene-solr/solr/core/src/test-files/solr/security.json
             [junit4]    > 	at __randomizedtesting.SeedInfo.seed([CDB85BECEFA64B6E:71D62DFE4BF5C814]:0)
             [junit4]    > 	at org.apache.solr.handler.admin.SecurityConfHandlerLocal.persistConf(SecurityConfHandlerLocal.java:86)
             [junit4]    > 	at org.apache.solr.handler.admin.SecurityConfHandlerLocalForTest.persistConf(SecurityConfHandlerLocalForTest.java:33)
             [junit4]    > 	at org.apache.solr.security.BasicAuthStandaloneTest.testBasicAuth(BasicAuthStandaloneTest.java:105)
             [junit4]    > 	at java.lang.Thread.run(Thread.java:745)
             [junit4]    > Caused by: java.security.AccessControlException: access denied ("java.io.FilePermission" "/Users/janhoy/git/lucene-solr/solr/core/src/test-files/solr/security.json" "write")
             [junit4]    > 	at java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
           
          Show
          janhoy Jan Høydahl added a comment - Hmm, need to choose another SOLR_HOME for testing write of security.json : [junit4] ERROR 0.57s J1 | BasicAuthStandaloneTest.testBasicAuth <<< [junit4] > Throwable #1: org.apache.solr.common.SolrException: Failed persisting security.json to /Users/janhoy/git/lucene-solr/solr/core/src/test-files/solr/security.json [junit4] > at __randomizedtesting.SeedInfo.seed([CDB85BECEFA64B6E:71D62DFE4BF5C814]:0) [junit4] > at org.apache.solr.handler.admin.SecurityConfHandlerLocal.persistConf(SecurityConfHandlerLocal.java:86) [junit4] > at org.apache.solr.handler.admin.SecurityConfHandlerLocalForTest.persistConf(SecurityConfHandlerLocalForTest.java:33) [junit4] > at org.apache.solr.security.BasicAuthStandaloneTest.testBasicAuth(BasicAuthStandaloneTest.java:105) [junit4] > at java.lang.Thread.run(Thread.java:745) [junit4] > Caused by: java.security.AccessControlException: access denied ("java.io.FilePermission" "/Users/janhoy/git/lucene-solr/solr/core/src/test-files/solr/security.json" "write") [junit4] > at java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit d25a6181612fa00a8e5a1c1e6d889b6d21053486 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d25a618 ]

          SOLR-9481: Authentication and Authorization plugins now work in standalone mode, including edit API

          Show
          jira-bot ASF subversion and git services added a comment - Commit d25a6181612fa00a8e5a1c1e6d889b6d21053486 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d25a618 ] SOLR-9481 : Authentication and Authorization plugins now work in standalone mode, including edit API
          Hide
          janhoy Jan Høydahl added a comment -

          Turned out the problem was that SecurityConfHandlerLocal.SECURITY_JSON_PATH was initialized as static final, but the test changed solr.solr.home prior to running, so the wrong solr home was used in the test.

          Show
          janhoy Jan Høydahl added a comment - Turned out the problem was that SecurityConfHandlerLocal.SECURITY_JSON_PATH was initialized as static final, but the test changed solr.solr.home prior to running, so the wrong solr home was used in the test.
          Hide
          janhoy Jan Høydahl added a comment -

          This is now committed to master. Appreciate if someone takes it for a spin before I backport to 6.x.
          Note however that it will only work for single-node due to SOLR-9640, and it will not work with SSL due to urlScheme being resolved from ZK only by hardcoding.

          Show
          janhoy Jan Høydahl added a comment - This is now committed to master. Appreciate if someone takes it for a spin before I backport to 6.x. Note however that it will only work for single-node due to SOLR-9640 , and it will not work with SSL due to urlScheme being resolved from ZK only by hardcoding.
          Hide
          janhoy Jan Høydahl added a comment -

          Quick copy/paste instructions for testing:

          cd solr
          ant server
          echo '{ "authentication": { "class": "solr.BasicAuthPlugin" }, "authorization": { "class": "solr.RuleBasedAuthorizationPlugin" } }' > server/solr/security.json
          bin/solr start
          bin/solr create -c foo
          # Add user
          curl http://localhost:8983/solr/admin/authentication \
            -H 'Content-type:application/json' \
            -d '{"set-user": {"solr" : "solr"}}'
          # Verify security.json
          cat server/solr/security.json
          # Set permissions
          curl http://localhost:8983/solr/admin/authorization \
            -H 'Content-type:application/json' \
            -d '{ "set-permission": {"name":"all", "role": "admin"}, "set-user-role" : {"solr": ["admin"]}}' 
          # Will return error
          curl http://localhost:8983/solr/admin/info/system
          # Will succeed
          curl -u solr:solr http://localhost:8983/solr/admin/info/system
          
          Show
          janhoy Jan Høydahl added a comment - Quick copy/paste instructions for testing: cd solr ant server echo '{ "authentication" : { "class" : "solr.BasicAuthPlugin" }, "authorization" : { "class" : "solr.RuleBasedAuthorizationPlugin" } }' > server/solr/security.json bin/solr start bin/solr create -c foo # Add user curl http: //localhost:8983/solr/admin/authentication \ -H 'Content-type:application/json' \ -d '{ "set-user" : { "solr" : "solr" }}' # Verify security.json cat server/solr/security.json # Set permissions curl http: //localhost:8983/solr/admin/authorization \ -H 'Content-type:application/json' \ -d '{ "set-permission" : { "name" : "all" , "role" : "admin" }, "set-user-role" : { "solr" : [ "admin" ]}}' # Will return error curl http: //localhost:8983/solr/admin/info/system # Will succeed curl -u solr:solr http: //localhost:8983/solr/admin/info/system
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b69c5d9f27aea722401674ed72b876da4dbdb7f4 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b69c5d9 ]

          SOLR-9481: Add info-level log "Initializing authentication plugin: <classname>"
          Move Sha256AuthProv warning "No users configured yet" to debug level, as this is quite normal

          Show
          jira-bot ASF subversion and git services added a comment - Commit b69c5d9f27aea722401674ed72b876da4dbdb7f4 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b69c5d9 ] SOLR-9481 : Add info-level log "Initializing authentication plugin: <classname>" Move Sha256AuthProv warning "No users configured yet" to debug level, as this is quite normal
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 24446f5085468627136e38ca8f874f383be9d3f3 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=24446f5 ]

          SOLR-9481: Fix test errors by using coreContainer.getSolrHome instead of SolrResourceLoader.locateSolrHome() in SecurityConfHandlerLocal

          Show
          jira-bot ASF subversion and git services added a comment - Commit 24446f5085468627136e38ca8f874f383be9d3f3 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=24446f5 ] SOLR-9481 : Fix test errors by using coreContainer.getSolrHome instead of SolrResourceLoader.locateSolrHome() in SecurityConfHandlerLocal
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 3d21029b334a498d59799b167e5278acc6013636 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3d21029 ]

          SOLR-9481: Fix precommit test, unused import

          Show
          jira-bot ASF subversion and git services added a comment - Commit 3d21029b334a498d59799b167e5278acc6013636 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3d21029 ] SOLR-9481 : Fix precommit test, unused import
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1f06411946237eff51f7d23bc52eb64e76a1c18b in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1f06411 ]

          SOLR-9481: Try to fix flaky test error by removing unnecessary initCore() in @Before method

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1f06411946237eff51f7d23bc52eb64e76a1c18b in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1f06411 ] SOLR-9481 : Try to fix flaky test error by removing unnecessary initCore() in @Before method
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit dbc2bc7ce8f76b30138fc47bc5e0a98b2028d504 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=dbc2bc7 ]

          SOLR-9481: Uppering debug level on the failing test and inserting extra logging. Also throw instead of swallow in case of problems parsing local security.json file

          Show
          jira-bot ASF subversion and git services added a comment - Commit dbc2bc7ce8f76b30138fc47bc5e0a98b2028d504 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=dbc2bc7 ] SOLR-9481 : Uppering debug level on the failing test and inserting extra logging. Also throw instead of swallow in case of problems parsing local security.json file
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 2175f6fde3d475de01e09d09c83d498551b19dfe in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2175f6f ]

          Removing 7.0.0 release section and SOLR-9481 from the CHANGES.txt

          Show
          jira-bot ASF subversion and git services added a comment - Commit 2175f6fde3d475de01e09d09c83d498551b19dfe in lucene-solr's branch refs/heads/branch_6x from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2175f6f ] Removing 7.0.0 release section and SOLR-9481 from the CHANGES.txt
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1fe1a54db32b8c27bfae81887cd4d75242090613 in lucene-solr's branch refs/heads/branch_6_3 from Shalin Shekhar Mangar
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1fe1a54 ]

          Removing 7.0.0 release section and SOLR-9481 from the CHANGES.txt

          (cherry picked from commit 2175f6f)

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1fe1a54db32b8c27bfae81887cd4d75242090613 in lucene-solr's branch refs/heads/branch_6_3 from Shalin Shekhar Mangar [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1fe1a54 ] Removing 7.0.0 release section and SOLR-9481 from the CHANGES.txt (cherry picked from commit 2175f6f)
          Hide
          erickerickson Erick Erickson added a comment -

          Hmmm, can this one be closed now since it's been committed back to 6.3 and 6.x?

          Show
          erickerickson Erick Erickson added a comment - Hmmm, can this one be closed now since it's been committed back to 6.3 and 6.x?
          Hide
          risdenk Kevin Risden added a comment -

          This looks like it was only committed to master. The last commit was from Shalin for the release and only affected CHANGES.txt.

          Show
          risdenk Kevin Risden added a comment - This looks like it was only committed to master. The last commit was from Shalin for the release and only affected CHANGES.txt.
          Hide
          risdenk Kevin Risden added a comment -

          Jan Høydahl - I tested with your steps through Docker and it works.

          Details: https://gist.github.com/risdenk/bd2c48dea8a5c60d2b7746d8b96c7ac2

          Show
          risdenk Kevin Risden added a comment - Jan Høydahl - I tested with your steps through Docker and it works. Details: https://gist.github.com/risdenk/bd2c48dea8a5c60d2b7746d8b96c7ac2
          Hide
          janhoy Jan Høydahl added a comment -

          Yes, this is currently only on master, and has some intermittent test failures I'm trying to track down before backporting. I got no idea how the changes entry made it to 6.x though, sorry for that Shalin Shekhar Mangar.

          Kevin, thanks for testing!

          Show
          janhoy Jan Høydahl added a comment - Yes, this is currently only on master, and has some intermittent test failures I'm trying to track down before backporting. I got no idea how the changes entry made it to 6.x though, sorry for that Shalin Shekhar Mangar . Kevin, thanks for testing!
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          I got no idea how the changes entry made it to 6.x though, sorry for that Shalin Shekhar Mangar.

          I should apologize because I made a wrong merge commit which added this issue in CHANGES.txt on branch_6x. However, I just noticed that this issue is still in the 6.3 section on master. Please move it to 6.4 when you can.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - I got no idea how the changes entry made it to 6.x though, sorry for that Shalin Shekhar Mangar. I should apologize because I made a wrong merge commit which added this issue in CHANGES.txt on branch_6x. However, I just noticed that this issue is still in the 6.3 section on master. Please move it to 6.4 when you can.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 22aa34e017bec1c8e8fd517e2969b1311c545c25 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=22aa34e ]

          SOLR-9481: Move changes entry to 6.4

          Show
          jira-bot ASF subversion and git services added a comment - Commit 22aa34e017bec1c8e8fd517e2969b1311c545c25 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=22aa34e ] SOLR-9481 : Move changes entry to 6.4
          Hide
          janhoy Jan Høydahl added a comment -

          Sure, was just waiting for RM to create version 6.4

          Show
          janhoy Jan Høydahl added a comment - Sure, was just waiting for RM to create version 6.4
          Hide
          romseygeek Alan Woodward added a comment -

          I think the test failures may be due to auth credentials being set on the HttpClientConfigurer in previous tests persisting? You can insert my usual rant about using global state here

          Show
          romseygeek Alan Woodward added a comment - I think the test failures may be due to auth credentials being set on the HttpClientConfigurer in previous tests persisting? You can insert my usual rant about using global state here
          Hide
          janhoy Jan Høydahl added a comment -

          Has to be some global state or thread issues I figure. Are you suggesting that some username/password is entered by another test and then we use wrong credentials for this?

          The extra DEBUG logging revealed something interesting too. In a passing test run we see these lines:

             [junit4]   2> 3904 DEBUG (qtp1698836513-21) [    ] o.a.s.s.SolrDispatchFilter Request to authenticate: Request(GET //127.0.0.1:58765/solr/admin/authentication)@b521fd6, domain: 127.0.0.1, port: 58765
             [junit4]   2> 3908 DEBUG (qtp1698836513-21) [    ] o.a.s.s.SolrDispatchFilter User principal: null
             [junit4]   2> 3908 DEBUG (qtp1698836513-21) [    ] o.a.s.s.HttpSolrCall AuthorizationContext : userPrincipal: [null] type: [ADMIN], collections: [], Path: [/admin/authentication] path : /admin/authentication params :
             [junit4]   2> 3911 DEBUG (qtp1698836513-21) [    ] o.a.s.s.RuleBasedAuthorizationPlugin No permissions configured for the resource /admin/authentication . So allowed to access
             [junit4]   2> 3912 INFO  (qtp1698836513-21) [    ] o.a.s.s.HttpSolrCall [admin] webapp=null path=/admin/authentication params={} status=0 QTime=3
             [junit4]   2> 3912 DEBUG (qtp1698836513-21) [    ] o.a.s.s.HttpSolrCall Closing out SolrRequest: {{params(),defaults(wt=json&indent=true)}}
          [...many more lines...]
             [junit4]   2> 4005 INFO  (TEST-BasicAuthStandaloneTest.testBasicAuth-seed#[3F9AB8AA5B5A65E6]) [    ] o.e.j.s.ServerConnector Stopped ServerConnector@117a0348{HTTP/1.1,[http/1.1]}{127.0.0.1:0}
          

          But in the failing test we see no logs about checking userPrincipal at all:

            [junit4]   2> 2497828 DEBUG (qtp1492928552-46458) [    ] o.a.s.s.SolrDispatchFilter Request to authenticate: Request(GET https://127.0.0.1:64493/solr/admin/authentication)@7a1251ec, domain: 127.0.0.1, port: 64493
            [junit4]   2> 2497830 INFO  (TEST-BasicAuthStandaloneTest.testBasicAuth-seed#[89F7DD5B01C6CD5E]) [    ] o.e.j.s.ServerConnector Stopped ServerConnector@66783857{SSL,[ssl, http/1.1]}{127.0.0.1:0}
          

          I'm not sure why. The log o.a.s.s.SolrDispatchFilter User principal: null is always printed if cores.getAuthenticationPlugin() != null... Too little log statements in this part of the code...

          Show
          janhoy Jan Høydahl added a comment - Has to be some global state or thread issues I figure. Are you suggesting that some username/password is entered by another test and then we use wrong credentials for this? The extra DEBUG logging revealed something interesting too. In a passing test run we see these lines: [junit4] 2> 3904 DEBUG (qtp1698836513-21) [ ] o.a.s.s.SolrDispatchFilter Request to authenticate: Request(GET //127.0.0.1:58765/solr/admin/authentication)@b521fd6, domain: 127.0.0.1, port: 58765 [junit4] 2> 3908 DEBUG (qtp1698836513-21) [ ] o.a.s.s.SolrDispatchFilter User principal: null [junit4] 2> 3908 DEBUG (qtp1698836513-21) [ ] o.a.s.s.HttpSolrCall AuthorizationContext : userPrincipal: [null] type: [ADMIN], collections: [], Path: [/admin/authentication] path : /admin/authentication params : [junit4] 2> 3911 DEBUG (qtp1698836513-21) [ ] o.a.s.s.RuleBasedAuthorizationPlugin No permissions configured for the resource /admin/authentication . So allowed to access [junit4] 2> 3912 INFO (qtp1698836513-21) [ ] o.a.s.s.HttpSolrCall [admin] webapp=null path=/admin/authentication params={} status=0 QTime=3 [junit4] 2> 3912 DEBUG (qtp1698836513-21) [ ] o.a.s.s.HttpSolrCall Closing out SolrRequest: {{params(),defaults(wt=json&indent=true)}} [...many more lines...] [junit4] 2> 4005 INFO (TEST-BasicAuthStandaloneTest.testBasicAuth-seed#[3F9AB8AA5B5A65E6]) [ ] o.e.j.s.ServerConnector Stopped ServerConnector@117a0348{HTTP/1.1,[http/1.1]}{127.0.0.1:0} But in the failing test we see no logs about checking userPrincipal at all: [junit4] 2> 2497828 DEBUG (qtp1492928552-46458) [ ] o.a.s.s.SolrDispatchFilter Request to authenticate: Request(GET https://127.0.0.1:64493/solr/admin/authentication)@7a1251ec, domain: 127.0.0.1, port: 64493 [junit4] 2> 2497830 INFO (TEST-BasicAuthStandaloneTest.testBasicAuth-seed#[89F7DD5B01C6CD5E]) [ ] o.e.j.s.ServerConnector Stopped ServerConnector@66783857{SSL,[ssl, http/1.1]}{127.0.0.1:0} I'm not sure why. The log o.a.s.s.SolrDispatchFilter User principal: null is always printed if cores.getAuthenticationPlugin() != null ... Too little log statements in this part of the code...
          Hide
          romseygeek Alan Woodward added a comment -

          Are you suggesting that some username/password is entered by another test and then we use wrong credentials for this?

          Yes; if you look in BasicAuthPlugin.doAuthenticate(), you'll see that the 'Bad Credentials' error is only thrown if the passed in request has an 'Authorization' header. So a previous test must be adding in an auth header setting to the HttpClientConfigurer, and this is still there when we get to this test.

          Show
          romseygeek Alan Woodward added a comment - Are you suggesting that some username/password is entered by another test and then we use wrong credentials for this? Yes; if you look in BasicAuthPlugin.doAuthenticate(), you'll see that the 'Bad Credentials' error is only thrown if the passed in request has an 'Authorization' header. So a previous test must be adding in an auth header setting to the HttpClientConfigurer, and this is still there when we get to this test.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4383bec84c38464c60e63880ad0ba37128d261a3 in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4383bec ]

          SOLR-9481: Clearing existing global interceptors on HttpClientUtil to avoid user/pass leaks from other tests

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4383bec84c38464c60e63880ad0ba37128d261a3 in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4383bec ] SOLR-9481 : Clearing existing global interceptors on HttpClientUtil to avoid user/pass leaks from other tests
          Hide
          janhoy Jan Høydahl added a comment -

          Ok, I'm attempting a blind fix by clearing interceptors on HttpClientUtil in @Before before creating the client I use for the test. So if it runs without new test failures for a few days I guess it worked... This global state stuff surely looks shaky.

          Show
          janhoy Jan Høydahl added a comment - Ok, I'm attempting a blind fix by clearing interceptors on HttpClientUtil in @Before before creating the client I use for the test. So if it runs without new test failures for a few days I guess it worked... This global state stuff surely looks shaky.
          Hide
          elyograg Shawn Heisey added a comment -

          That's kinda scary.

          Here's what came to mind before looking at the code: Thread-safe code becomes very difficult when there are static class variables that affect client building.

          After looking at the code, it's a similar thought, but the situation is worse than I imagined: Every HttpClient built by our util class in a program will share the same global list of interceptors (whatever those do). Which means that if you set up the interceptors the way you want for one server, create the HttpClient and the SolrClient, then clear the interceptors and set up a second HttpClient/SolrClient, BOTH clients will use the new list.

          Show
          elyograg Shawn Heisey added a comment - That's kinda scary. Here's what came to mind before looking at the code: Thread-safe code becomes very difficult when there are static class variables that affect client building. After looking at the code, it's a similar thought, but the situation is worse than I imagined: Every HttpClient built by our util class in a program will share the same global list of interceptors (whatever those do). Which means that if you set up the interceptors the way you want for one server, create the HttpClient and the SolrClient, then clear the interceptors and set up a second HttpClient/SolrClient, BOTH clients will use the new list.
          Hide
          janhoy Jan Høydahl added a comment -

          Perhaps as a first step we create a new JIRA to fix this for tests, I don't know exactly how but looks like something should be done...

          Show
          janhoy Jan Høydahl added a comment - Perhaps as a first step we create a new JIRA to fix this for tests, I don't know exactly how but looks like something should be done...
          Hide
          janhoy Jan Høydahl added a comment -

          Attaching backport patch for 6x.

          • The simple recipe above tested on macOS
          • The 6.x version of HttpClientUtil did not have the clearRequestInterceptors() method, so I backported that as well.
          • Passes precommit

          Instead of cherry-picking 6 commits and then adding another commit for fixing 6x specific issues, I'll squash all of this into one commit on branch_6x and instead list all related master commit hashes in the log.

          Show
          janhoy Jan Høydahl added a comment - Attaching backport patch for 6x. The simple recipe above tested on macOS The 6.x version of HttpClientUtil did not have the clearRequestInterceptors() method, so I backported that as well. Passes precommit Instead of cherry-picking 6 commits and then adding another commit for fixing 6x specific issues, I'll squash all of this into one commit on branch_6x and instead list all related master commit hashes in the log.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 4383bec84c38464c60e63880ad0ba37128d261a3 in lucene-solr's branch refs/heads/apiv2 from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4383bec ]

          SOLR-9481: Clearing existing global interceptors on HttpClientUtil to avoid user/pass leaks from other tests

          Show
          jira-bot ASF subversion and git services added a comment - Commit 4383bec84c38464c60e63880ad0ba37128d261a3 in lucene-solr's branch refs/heads/apiv2 from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4383bec ] SOLR-9481 : Clearing existing global interceptors on HttpClientUtil to avoid user/pass leaks from other tests
          Hide
          janhoy Jan Høydahl added a comment -

          I'll not work on this for the coming few weeks, so anyone who wants this in 6.4 should feel free to take over the last few steps and committing. The attached patch should be pretty solid already.

          Show
          janhoy Jan Høydahl added a comment - I'll not work on this for the coming few weeks, so anyone who wants this in 6.4 should feel free to take over the last few steps and committing. The attached patch should be pretty solid already.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 7bbb918467b45dd529672b2f861e32e463d11aed in lucene-solr's branch refs/heads/master from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7bbb918 ]

          SOLR-9481: Moving changes entry to 6.5 and targeting that release instead

          Show
          jira-bot ASF subversion and git services added a comment - Commit 7bbb918467b45dd529672b2f861e32e463d11aed in lucene-solr's branch refs/heads/master from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7bbb918 ] SOLR-9481 : Moving changes entry to 6.5 and targeting that release instead
          Hide
          steve_rowe Steve Rowe added a comment - - edited

          SOLR-9481: Moving changes entry to 6.5 and targeting that release instead

          Jan Høydahl, looks like you forgot to commit this to branch_6x?

          Show
          steve_rowe Steve Rowe added a comment - - edited SOLR-9481 : Moving changes entry to 6.5 and targeting that release instead Jan Høydahl , looks like you forgot to commit this to branch_6x?
          Hide
          janhoy Jan Høydahl added a comment -

          Absolutely This patch is pure open source idealism, not paid work.. Will try to get around to it again though.

          Show
          janhoy Jan Høydahl added a comment - Absolutely This patch is pure open source idealism, not paid work.. Will try to get around to it again though.
          Hide
          steve_rowe Steve Rowe added a comment -

          Jan, if you don't have time to do it short term, please move the CHANGES entry back out of the 6.5 section.

          Show
          steve_rowe Steve Rowe added a comment - Jan, if you don't have time to do it short term, please move the CHANGES entry back out of the 6.5 section.
          Hide
          janhoy Jan Høydahl added a comment -

          The CHANGES entry was set to 6.x when master was committed, I'm still targeting 6.5

          Attaching updated 6x backport patch. Tests and precommit passes.

          Show
          janhoy Jan Høydahl added a comment - The CHANGES entry was set to 6.x when master was committed, I'm still targeting 6.5 Attaching updated 6x backport patch. Tests and precommit passes.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b1ac6ddcf2f1027806f04a6af0e5a51f01334113 in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b1ac6dd ]

          SOLR-9481: Authentication and Authorization plugins now support standalone mode

          Show
          jira-bot ASF subversion and git services added a comment - Commit b1ac6ddcf2f1027806f04a6af0e5a51f01334113 in lucene-solr's branch refs/heads/branch_6x from Jan Høydahl [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b1ac6dd ] SOLR-9481 : Authentication and Authorization plugins now support standalone mode
          Hide
          janhoy Jan Høydahl added a comment -

          Ok I went on and pushed this to 6x since the code has baked for so long in master, and I want to give it some time in 6x before someone announces a 6.5 RC. Added a small "experimental" notice to the CHANGES entry since it will still not work fully with SSL

          Show
          janhoy Jan Høydahl added a comment - Ok I went on and pushed this to 6x since the code has baked for so long in master, and I want to give it some time in 6x before someone announces a 6.5 RC. Added a small "experimental" notice to the CHANGES entry since it will still not work fully with SSL
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user janhoy closed the pull request at:

          https://github.com/apache/lucene-solr/pull/95

          Show
          githubbot ASF GitHub Bot added a comment - Github user janhoy closed the pull request at: https://github.com/apache/lucene-solr/pull/95

            People

            • Assignee:
              janhoy Jan Høydahl
              Reporter:
              janhoy Jan Høydahl
            • Votes:
              2 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development