mark-weghorst commented on a change in pull request #4508:
URL: https://github.com/apache/nifi/pull/4508#discussion_r482575747
##########
File path:
nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -68,14 +69,21 @@
private static final String SCHEMA_REGISTRY_CONTENT_TYPE =
"application/vnd.schemaregistry.v1+json";
- public RestSchemaRegistryClient(final List<String> baseUrls, final int
timeoutMillis, final SSLContext sslContext, final ComponentLog logger) {
+ public RestSchemaRegistryClient(final List<String> baseUrls, final String
authType, final String authUser,
+ final String authPass, final int
timeoutMillis, final SSLContext sslContext, final ComponentLog logger) {
this.baseUrls = new ArrayList<>(baseUrls);
final ClientConfig clientConfig = new ClientConfig();
clientConfig.property(ClientProperties.CONNECT_TIMEOUT, timeoutMillis);
clientConfig.property(ClientProperties.READ_TIMEOUT, timeoutMillis);
client = WebUtils.createClient(clientConfig, sslContext);
+ if (!authUser.isEmpty() && !authType.isEmpty()) {
+ if (authType.equals("BASIC")) {
+ client.register(HttpAuthenticationFeature.basic(authUser,
authPass));
Review comment:
You raise an interesting question, AFAIK basic HTTP authentication
requires a username, but will work with a password that is an empty string.
A quick search found bugs in other projects where they are failing to
support null passwords. **_From my point of view the question is really
whether we should allow empty passwords or not._**
In your review of the original PR in April, you suggested that
customValidate for the controller be enhanced to throw an error if auth type
was basic and either username or password were unset.
I've done as you requested in that orignal PR, and the UX will display a
validation error with a meaningful message that both username and password are
required when auth type is basic.
Bottom line here is I'll bend to your judgement on this, let me know what
you think we should do
----------------------------------------------------------------
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:
[email protected]
