You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/12/07 02:34:01 UTC

[GitHub] [nifi] exceptionfactory commented on a change in pull request #5574: NIFI-9397 Extending JettyWebServerClient authorization possibilities with custom setup

exceptionfactory commented on a change in pull request #5574:
URL: https://github.com/apache/nifi/pull/5574#discussion_r763566902



##########
File path: nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketClient.java
##########
@@ -145,6 +145,19 @@
             .defaultValue("US-ASCII")
             .build();
 
+    public static final PropertyDescriptor CUSTOM_AUTH = new PropertyDescriptor.Builder()
+            .name("custom-authorization")
+            .displayName("Custom Authorization")
+            .description(
+                    "If set tgether with \"User Name\" and \"User Password\", instead of using Basic" +

Review comment:
       For additional documentation, it would be helpful to include the official standard reference [RFC 7235 Section 4.2](https://datatracker.ietf.org/doc/html/rfc7235#section-4.2).
   
   ```suggestion
                       "Configures a custom HTTP Authorization Header as described in RFC 7235 Section 4.2. Setting a custom Authorization Header excludes configuring the User Name and User Password properties for Basic Authentication." +
   ```

##########
File path: nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketClient.java
##########
@@ -145,6 +145,19 @@
             .defaultValue("US-ASCII")
             .build();
 
+    public static final PropertyDescriptor CUSTOM_AUTH = new PropertyDescriptor.Builder()
+            .name("custom-authorization")
+            .displayName("Custom Authorization")
+            .description(
+                    "If set tgether with \"User Name\" and \"User Password\", instead of using Basic" +
+                    " Authentication the value of the property will be assigned to the \"Authorization\" HTTP header.")

Review comment:
       Instead of describing the behavior this way, what do you think about extending the `customValidate()` method to ensure that setting this property excludes setting `User Name` and `User Password`, and vice versa? Checking the both this property and the other credentials properties and not set would help avoid potential confusion in the component configuration.




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

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