You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/12/11 02:10:26 UTC
[GitHub] [pulsar] hangc0276 opened a new pull request #8910: fix authParams showing in log with secret string(*****)
hangc0276 opened a new pull request #8910:
URL: https://github.com/apache/pulsar/pull/8910
Fix #8509
### Changes
1. add `Secret` interface and `SecretsSerializer` for fields need to be shown in secret string(*****) when log to json string
2. add `Secret` tag for `authParams` and `authParamMap` in `ClientConfiguration`
3. add a test for the secret feature
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] hangc0276 commented on pull request #8910: fix authParams showing in log with secret string(*****)
Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #8910:
URL: https://github.com/apache/pulsar/pull/8910#issuecomment-742919083
@racorn Please help take a look, thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] jiazhai merged pull request #8910: fix authParams showing in log with secret string(*****)
Posted by GitBox <gi...@apache.org>.
jiazhai merged pull request #8910:
URL: https://github.com/apache/pulsar/pull/8910
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] racorn edited a comment on pull request #8910: fix authParams showing in log with secret string(*****)
Posted by GitBox <gi...@apache.org>.
racorn edited a comment on pull request #8910:
URL: https://github.com/apache/pulsar/pull/8910#issuecomment-743767701
@hangc0276 Thanks for you contribution. Looks good, however I think you also should include the `Views` class in you implementation as I suggested.
The code, as it is now, will always serialize `ClientConfigurationData` with '****' for the secret field. Using the Views class and using the internal view explicitly in `ConfigurationDataUtils.loadData` allows us to serialize `ClientConfigurationData` with secrets for internal use. The case to cater for here is that 1) you set auth params programmatically, and then you load a config without auth params.
Consider your test with the following modification:
``` java
@Test
public void testLoadSecretParams2() {
ClientConfigurationData confData = new ClientConfigurationData();
Map<String, String> authParamMap = new HashMap<>();
authParamMap.put("k1", "v1");
confData.setServiceUrl("pulsar://unknown:6650");
confData.setAuthParams("");
confData.setAuthParamMap(authParamMap);
authParamMap.put("k2", "v2");
Map<String, Object> config = new HashMap<>();
config.put("serviceUrl", "pulsar://localhost:6650");
config.put("authParams", "testAuthParams");
// MODIFICATION: auth param map set on ClientConfigurationData but not on config override map
//config.put("authParamMap", authParamMap);
confData = ConfigurationDataUtils.loadData(config, confData, ClientConfigurationData.class);
assertEquals("pulsar://localhost:6650", confData.getServiceUrl());
assertEquals("testAuthParams", confData.getAuthParams());
assertEquals("v1", confData.getAuthParamMap().get("k1"));
assertEquals("v2", confData.getAuthParamMap().get("k2"));
final String secretStr = "*****";
try {
String confDataJson = new ObjectMapper().writeValueAsString(confData);
Map<String, Object> confDataMap = new ObjectMapper().readValue(confDataJson, Map.class);
assertEquals("pulsar://localhost:6650", confDataMap.get("serviceUrl"));
assertEquals(secretStr, confDataMap.get("authParams"));
assertEquals(secretStr, confDataMap.get("authParamMap"));
} catch (Exception e) {
Assert.fail();
}
}
```
Then the test fails with
`Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `java.util.LinkedHashMap` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('*****')
`
Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] racorn commented on pull request #8910: fix authParams showing in log with secret string(*****)
Posted by GitBox <gi...@apache.org>.
racorn commented on pull request #8910:
URL: https://github.com/apache/pulsar/pull/8910#issuecomment-743767701
@hangc0276 Thanks for you contribution. Looks good, however I think you also should include the `Views` class in you implementation as I suggested.
The code, as it is now, will always serialize `ClientConfigurationData` with '****' for the secret field. Using the Views class and using the internal view explicitly in `ConfigurationDataUtils.loadData` allows us to serialize `ClientConfigurationData` with secrets for internal use. The case to cater for here is that 1) you set auth params programmatically, and then you load a config without auth params.
Consider you test with the following modification:
``` java
@Test
public void testLoadSecretParams2() {
ClientConfigurationData confData = new ClientConfigurationData();
Map<String, String> authParamMap = new HashMap<>();
authParamMap.put("k1", "v1");
confData.setServiceUrl("pulsar://unknown:6650");
confData.setAuthParams("");
confData.setAuthParamMap(authParamMap);
authParamMap.put("k2", "v2");
Map<String, Object> config = new HashMap<>();
config.put("serviceUrl", "pulsar://localhost:6650");
config.put("authParams", "testAuthParams");
// MODIFICATION: auth param map set on ClientConfigurationData but not on config override map
//config.put("authParamMap", authParamMap);
System.out.println(config);
confData = ConfigurationDataUtils.loadData(config, confData, ClientConfigurationData.class);
assertEquals("pulsar://localhost:6650", confData.getServiceUrl());
assertEquals("testAuthParams", confData.getAuthParams());
assertEquals("v1", confData.getAuthParamMap().get("k1"));
assertEquals("v2", confData.getAuthParamMap().get("k2"));
final String secretStr = "*****";
try {
String confDataJson = new ObjectMapper().writeValueAsString(confData);
Map<String, Object> confDataMap = new ObjectMapper().readValue(confDataJson, Map.class);
assertEquals("pulsar://localhost:6650", confDataMap.get("serviceUrl"));
assertEquals(secretStr, confDataMap.get("authParams"));
assertEquals(secretStr, confDataMap.get("authParamMap"));
} catch (Exception e) {
Assert.fail();
}
}
```
Then the test fails with
`Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `java.util.LinkedHashMap` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('*****')
`
Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [pulsar] racorn edited a comment on pull request #8910: fix authParams showing in log with secret string(*****)
Posted by GitBox <gi...@apache.org>.
racorn edited a comment on pull request #8910:
URL: https://github.com/apache/pulsar/pull/8910#issuecomment-743767701
@hangc0276 Thanks for you contribution. Looks good, however I think you also should include the `Views` class in you implementation as I suggested.
The code, as it is now, will always serialize `ClientConfigurationData` with '****' for the secret field. Using the Views class and using the internal view explicitly in `ConfigurationDataUtils.loadData` allows us to serialize `ClientConfigurationData` with secrets for internal use. The case to cater for here is that 1) you set auth params programmatically, and then you load a config without auth params.
Consider your test with the following modification:
``` java
@Test
public void testLoadSecretParams2() {
ClientConfigurationData confData = new ClientConfigurationData();
Map<String, String> authParamMap = new HashMap<>();
authParamMap.put("k1", "v1");
confData.setServiceUrl("pulsar://unknown:6650");
confData.setAuthParams("");
confData.setAuthParamMap(authParamMap);
authParamMap.put("k2", "v2");
Map<String, Object> config = new HashMap<>();
config.put("serviceUrl", "pulsar://localhost:6650");
config.put("authParams", "testAuthParams");
// MODIFICATION: auth param map set on ClientConfigurationData but not on config override map
//config.put("authParamMap", authParamMap);
System.out.println(config);
confData = ConfigurationDataUtils.loadData(config, confData, ClientConfigurationData.class);
assertEquals("pulsar://localhost:6650", confData.getServiceUrl());
assertEquals("testAuthParams", confData.getAuthParams());
assertEquals("v1", confData.getAuthParamMap().get("k1"));
assertEquals("v2", confData.getAuthParamMap().get("k2"));
final String secretStr = "*****";
try {
String confDataJson = new ObjectMapper().writeValueAsString(confData);
Map<String, Object> confDataMap = new ObjectMapper().readValue(confDataJson, Map.class);
assertEquals("pulsar://localhost:6650", confDataMap.get("serviceUrl"));
assertEquals(secretStr, confDataMap.get("authParams"));
assertEquals(secretStr, confDataMap.get("authParamMap"));
} catch (Exception e) {
Assert.fail();
}
}
```
Then the test fails with
`Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot construct instance of `java.util.LinkedHashMap` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('*****')
`
Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org