You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/07/15 15:18:14 UTC

[GitHub] [solr] thelabdude opened a new pull request #222: SOLR-15525: Read ZK credentials from a file specified using a system property instead of exposing plain-text credentials

thelabdude opened a new pull request #222:
URL: https://github.com/apache/solr/pull/222


   
   https://issues.apache.org/jira/browse/SOLR-15525
   
   # Description
   
   Added new Java system property `zkDigestCredentialsFile` to allow loading of zk creds from a file instead of from system properties.
   
   # Tests
   
   Added to existing unit test `VMParamsZkACLAndCredentialsProvidersTest` to use the new `zkDigestCredentialsFile` feature.
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [ ] I have created a Jira issue and added the issue ID to my pull request title.
   - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [ ] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] thelabdude commented on pull request #222: SOLR-15525: Read ZK credentials from a file specified using a system property instead of exposing plain-text credentials

Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #222:
URL: https://github.com/apache/solr/pull/222#issuecomment-881474761


   @madrob ~ forbidden check didn't like your suggested change:
   ```
   Forbidden method invocation: java.util.Properties#store(java.io.OutputStream,java.lang.String) [Properties files should be read/written with Reader/Writer, using UTF-8 charset. This allows reading older files with unicode escapes, too.]
     in org.apache.solr.cloud.VMParamsZkACLAndCredentialsProvidersTest (VMParamsZkACLAndCredentialsProvidersTest.java:302)
   > Task :solr:core:forbiddenApisTest
   ```
   It looks like it wants a FileWriter like I had originally.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] thelabdude commented on a change in pull request #222: SOLR-15525: Read ZK credentials from a file specified using a system property instead of exposing plain-text credentials

Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #222:
URL: https://github.com/apache/solr/pull/222#discussion_r670825201



##########
File path: solr/core/src/test/org/apache/solr/cloud/VMParamsZkACLAndCredentialsProvidersTest.java
##########
@@ -250,6 +276,33 @@ private void useReadonlyCredentials() {
     System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_USERNAME_VM_PARAM_NAME, "readonlyACLUsername");
     System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_PASSWORD_VM_PARAM_NAME, "readonlyACLPassword");
   }
+
+  private void useReadonlyCredentialsFromFile() throws IOException {
+    useCredentialsFromFile("readonlyACLUsername", "readonlyACLPassword");
+  }
+
+  private void useAllCredentialsFromFile() throws IOException {
+    useCredentialsFromFile("connectAndAllACLUsername", "connectAndAllACLPassword");
+  }
+
+  private void useCredentialsFromFile(String username, String password) throws IOException {
+    clearSecuritySystemProperties();
+
+    System.setProperty(SolrZkClient.ZK_CRED_PROVIDER_CLASS_NAME_VM_PARAM_NAME, VMParamsSingleSetCredentialsDigestZkCredentialsProvider.class.getName());
+
+    Properties props = new Properties();
+    props.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_USERNAME_VM_PARAM_NAME, username);
+    props.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_PASSWORD_VM_PARAM_NAME, password);
+    saveCredentialsFile(props);
+  }
+
+  private void saveCredentialsFile(Properties props) throws IOException {
+    File credentialsFile = createTempFile("zk-creds", "properties").toFile();
+    try (FileWriter writer = new FileWriter(credentialsFile, StandardCharsets.UTF_8)) {
+      props.store(writer, "test");
+    }
+    System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_FILE_VM_PARAM_NAME, credentialsFile.getAbsolutePath());

Review comment:
       @madrob I think you have perms to just push that change to the PR branch, no?




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on pull request #222: SOLR-15525: Read ZK credentials from a file specified using a system property instead of exposing plain-text credentials

Posted by GitBox <gi...@apache.org>.
madrob commented on pull request #222:
URL: https://github.com/apache/solr/pull/222#issuecomment-881035513


   Yea but I didn’t test it 😅


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] thelabdude merged pull request #222: SOLR-15525: Read ZK credentials from a file specified using a system property instead of exposing plain-text credentials

Posted by GitBox <gi...@apache.org>.
thelabdude merged pull request #222:
URL: https://github.com/apache/solr/pull/222


   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on a change in pull request #222: SOLR-15525: Read ZK credentials from a file specified using a system property instead of exposing plain-text credentials

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #222:
URL: https://github.com/apache/solr/pull/222#discussion_r670775643



##########
File path: solr/core/src/test/org/apache/solr/cloud/VMParamsZkACLAndCredentialsProvidersTest.java
##########
@@ -250,6 +276,33 @@ private void useReadonlyCredentials() {
     System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_USERNAME_VM_PARAM_NAME, "readonlyACLUsername");
     System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_PASSWORD_VM_PARAM_NAME, "readonlyACLPassword");
   }
+
+  private void useReadonlyCredentialsFromFile() throws IOException {
+    useCredentialsFromFile("readonlyACLUsername", "readonlyACLPassword");
+  }
+
+  private void useAllCredentialsFromFile() throws IOException {
+    useCredentialsFromFile("connectAndAllACLUsername", "connectAndAllACLPassword");
+  }
+
+  private void useCredentialsFromFile(String username, String password) throws IOException {
+    clearSecuritySystemProperties();
+
+    System.setProperty(SolrZkClient.ZK_CRED_PROVIDER_CLASS_NAME_VM_PARAM_NAME, VMParamsSingleSetCredentialsDigestZkCredentialsProvider.class.getName());
+
+    Properties props = new Properties();
+    props.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_USERNAME_VM_PARAM_NAME, username);
+    props.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_PASSWORD_VM_PARAM_NAME, password);
+    saveCredentialsFile(props);
+  }
+
+  private void saveCredentialsFile(Properties props) throws IOException {
+    File credentialsFile = createTempFile("zk-creds", "properties").toFile();
+    try (FileWriter writer = new FileWriter(credentialsFile, StandardCharsets.UTF_8)) {
+      props.store(writer, "test");
+    }
+    System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_FILE_VM_PARAM_NAME, credentialsFile.getAbsolutePath());

Review comment:
       ```suggestion
       Path tmp = createTempFile("zk-creds", "properties");
       try (OutputStream os = Files.newOutputStream(tmp)) {
         props.store(os, "test");
       }
       System.setProperty(VMParamsSingleSetCredentialsDigestZkCredentialsProvider.DEFAULT_DIGEST_FILE_VM_PARAM_NAME, tmp.toAbsolutePath().toString());
   ```




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org