You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/02/11 18:41:20 UTC
[GitHub] [druid] jihoonson opened a new issue #9351: Adding
CredentialProvider, deprecating PasswordProvider
jihoonson opened a new issue #9351: Adding CredentialProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351
### Motivation
We are using `PasswordProvider` to secure user passwords in various places. However, it doesn't secure the user name (or account whatever).
### Proposed changes
This proposal is to add a new class `CredentialProvider`. Similar to `PasswordProvider`, the `CredentialProvider` provides various ways to get the user credential, but both user name and password.
```java
public interface CredentialProvider
{
String getAccount();
String getPassword();
}
```
### Rationale
Since the `PasswordProvider` has one method of `getPassword()`, we are already using two different `PasswordProvider`s to secure both user name and password in some places. `AWSCredentialsConfig` is an example:
```java
public class AWSCredentialsConfig
{
@JsonProperty
private PasswordProvider accessKey = new DefaultPasswordProvider("");
@JsonProperty
private PasswordProvider secretKey = new DefaultPasswordProvider("");
...
```
This is not good for user experience because they generally prefer to secure their account and password in the same way.
### Operational impact
The `PasswordProvider` will be deprecated in favor of the new `CredentialProvider`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
himanshug commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-586348888
> Hmm, should one CredentialsProvider be able to handle multiple secrets? Would you tell me some examples?
For example, https://github.com/apache/druid/blob/master/extensions-core/simple-client-sslcontext/src/main/java/org/apache/druid/https/SSLClientConfig.java has three secrets which could be logically related
Also, now that I remember, even the interface `String getPassword(String key);` would have problems, only way to prevent the problem mentioned in #7400 would be to have an interface like below ( as suggested in https://github.com/apache/druid/issues/6666#issuecomment-486398615 )
```
interface CredentialsProvider {
Map<String, String> getCredentials();
}
```
as you wanna get a snapshot of secrets in single call , if the code needs to make two/multiple calls to `CredentialsProvider` then you would continue to have the race mentioned in #7400
hopefully it made sense or I need to be more descriptive :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson edited a comment on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
jihoonson edited a comment on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585332595
Hi @a2l007, I think the internal usage would be pretty much same. But, the user-facing configurations would change which I expect to achieve in this proposal. For example, let say we changed `AWSCredentialsConfig` to use the `CredentialsProvider` as below.
```java
public class AWSCredentialsConfig
{
@JsonProperty
@Deprecated
private PasswordProvider accessKey = new DefaultPasswordProvider("");
@JsonProperty
@Deprecated
private PasswordProvider secretKey = new DefaultPasswordProvider("");
@JsonProperty
private CredentialsProvider credentials = new DefaultCredentialsProvider();
...
}
```
Then, the s3 credentials configurations would be changed from `druid.s3.accessKey` and `druid.s3.secretKey` to `druid.s3.credentials.accessKey` and `druid.s3.credentials.secretKey`, respectively.
`HttpInputSource` is another example on the ingestion spec side.
```java
@JsonCreator
public HttpInputSource(
@JsonProperty("uris") List<URI> uris,
@JsonProperty("httpAuthenticationUsername") @Deprecated @Nullable String httpAuthenticationUsername,
@JsonProperty("httpAuthenticationPassword") @Deprecated @Nullable PasswordProvider httpAuthenticationPasswordProvider,
@JsonProperty("httpAuthentication") @Nullable CredentialsProvider httpAuthentication
)
```
The new `httpAuthentication` will be a JSON object that has two fields of `userName` and `password`. As a result, the ingestion spec would be changed from
```json
"inputSource": {
"type": "http",
"uris": ["http://example.com/uri1", "http://example2.com/uri2"],
"httpAuthenticationUsername": "username",
"httpAuthenticationPassword": {
"type": "environment",
"variable": "MY_PASSWORD_VAR"
}
}
```
to
```json
"inputSource": {
"type": "http",
"uris": ["http://example.com/uri1", "http://example2.com/uri2"],
"httpAuthentication": {
"type": "environment",
"userNameVariable": "MY_USER_NAME_VAR",
"passwordVariable": "MY_PASSWORD_VAR"
}
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] a2l007 commented on issue #9351: Adding CredentialsProvider,
deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
a2l007 commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585391740
@jihoonson Thanks for the clarification. SGTM
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-586106053
@himanshug thank you for bring up #6666. That's a really good point.
> Also, wouldn't
>
> ```
> public interface CredentialsProvider
> {
> String getPassword(String key);
> }
> ```
>
> be more generic then which can handle more than two logically related secrets.
Hmm, should one `CredentialsProvider` be able to handle multiple secrets? Would you tell me some examples?
> For DB , In all the places I used druid, username was never a secret . Also I see some passwords in https://druid.apache.org/docs/latest/development/extensions-core/druid-basic-security.html which don't necessarily need an "account". I am not saying that we need to keep `PasswordProvider` but just noting down the different use cases.
Good point. I think these are pretty rare cases and still can be handled by the `CredentialProvider`. For example, we can add a `PlainUsernameAndSecuredPasswordProvider` for people who really don't want to secure the username. Similarly, we can add `PasswordOnlyCredentialsProvider` for the use case where the username is not required.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585403918
@a2l007 thank you for taking a look!
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on issue #9351: Adding
CredentialProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9351: Adding CredentialProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-584812032
@maytasm3 thank you for taking a look.
> * Do we need to deprecate PasswordProvider? Can we have both? The reason is that there might be no username and just a password or a single field that needs to be secured. i.e. things that are keys (access key, API keys, etc.)
I think the use cases of `PasswordProvider` and `CredentialProvider` are pretty overlapped. Would you elaborate more on the example applications that require a password but not an account we should consider?
> * What if Druid user wants to secured username and password differently? Is this still possible? For example, a user might pass username as plaintext and password with environment variable
I think this would be pretty rare, but it's still possible with a custom credential provider.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] a2l007 commented on issue #9351: Adding CredentialProvider,
deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
a2l007 commented on issue #9351: Adding CredentialProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585249785
@jihoonson Just to clarify, would this be impacting existing usages of getPassword() in any way? I would expect the only change would be `PasswordProvider.getPassword()` usages changing to `CredentialProvider.getPassword()`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-588383545
@himanshug makes sense. I updated the proposal. Thanks for comments!
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585999965
coming back to peek into Druid PRs after a while and noticed this. Just wanted to let you know of https://github.com/apache/druid/issues/6666 which proposes something related for other reasons and it would be good to have that in mind as well.
Also, wouldn't
```
public interface CredentialsProvider
{
String getPassword(String key);
}
```
be more generic then which can handle more than two logically related secrets.
> Would you elaborate more on the example applications that require a password but not an account we should consider?
For DB , In all the places I used druid, username was never a secret . Also I see some passwords in https://druid.apache.org/docs/latest/development/extensions-core/druid-basic-security.html which don't necessarily need an "account". I am not saying that we need to keep `PasswordProvider` but just noting down the different use cases.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585332595
Hi @a2l007, I think the internal usage would be pretty much same. But, the user-facing configurations would be changed which I expect to achieve in this proposal. For example, let say we changed `AWSCredentialsConfig` to use the `CredentialsProvider` as below.
```java
public class AWSCredentialsConfig
{
@JsonProperty
@Deprecated
private PasswordProvider accessKey = new DefaultPasswordProvider("");
@JsonProperty
@Deprecated
private PasswordProvider secretKey = new DefaultPasswordProvider("");
@JsonProperty
private CredentialsProvider credentials = new DefaultCredentialsProvider();
...
}
```
Then, the s3 credentials configurations would be changed from `druid.s3.accessKey` and `druid.s3.secretKey` to `druid.s3.credentials.accessKey` and `druid.s3.credentials.secretKey`, respectively.
`HttpInputSource` is another example on the ingestion spec side.
```java
@JsonCreator
public HttpInputSource(
@JsonProperty("uris") List<URI> uris,
@JsonProperty("httpAuthenticationUsername") @Deprecated @Nullable String httpAuthenticationUsername,
@JsonProperty("httpAuthenticationPassword") @Deprecated @Nullable PasswordProvider httpAuthenticationPasswordProvider,
@JsonProperty("httpAuthentication") @Nullable CredentialsProvider httpAuthentication
)
```
The new `httpAuthentication` will be a JSON object that has two fields of `userName` and `password`. As a result, the ingestion spec would be changed from
```json
"inputSource": {
"type": "http",
"uris": ["http://example.com/uri1", "http://example2.com/uri2"],
"httpAuthenticationUsername": "username",
"httpAuthenticationPassword": {
"type": "environment",
"variable": "MY_PASSWORD_VAR"
}
}
```
to
```json
"inputSource": {
"type": "http",
"uris": ["http://example.com/uri1", "http://example2.com/uri2"],
"httpAuthentication": {
"type": "environment",
"userNameVariable": "MY_USER_NAME_VAR",
"passwordVariable": "MY_PASSWORD_VAR"
}
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on issue #9351: Adding
CredentialsProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
himanshug commented on issue #9351: Adding CredentialsProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-585999965
coming back to peek into Druid PRs after a while and noticed this. Just wanted to let you know of https://github.com/apache/druid/issues/6666 which proposes something related for other reasons and it would good to have that in mind as well.
Also, wouldn't
```
public interface CredentialsProvider
{
String getPassword(String key);
}
```
be more generic then which can handle more than two logically related secrets.
> Would you elaborate more on the example applications that require a password but not an account we should consider?
For DB , In all the places I used druid, username was never a secret . Also I see some passwords in https://druid.apache.org/docs/latest/development/extensions-core/druid-basic-security.html which don't necessarily need an "account". I am not saying that we need to keep `PasswordProvider` but just noting down the different use cases.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm3 commented on issue #9351: Adding
CredentialProvider, deprecating PasswordProvider
Posted by GitBox <gi...@apache.org>.
maytasm3 commented on issue #9351: Adding CredentialProvider, deprecating PasswordProvider
URL: https://github.com/apache/druid/issues/9351#issuecomment-584806554
- Do we need to deprecate PasswordProvider? Can we have both? The reason is that there might be no username and just a password or a single field that needs to be secured. i.e. things that are keys (access key, API keys, etc.)
- What if Druid user wants to secured username and password differently? Is this still possible? For example, a user might pass username as plaintext and password with environment variable
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org