You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/01/04 14:09:27 UTC

[GitHub] [guacamole-client] necouchman opened a new pull request #586: GUACAMOLE-1252: Allow RADIUS NAS IP to be manually configured.

necouchman opened a new pull request #586:
URL: https://github.com/apache/guacamole-client/pull/586


   This change creates the `radius-nas-ip` property, allowing administrators to configure that manually. If the property is not set, it falls back to the functionality present today, which uses `InetAddress.getLocalHost()` to attempt to determine the IP address automatically.


----------------------------------------------------------------
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] [guacamole-client] necouchman commented on a change in pull request #586: GUACAMOLE-1252: Allow RADIUS NAS IP to be manually configured.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #586:
URL: https://github.com/apache/guacamole-client/pull/586#discussion_r553470630



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/ConfigurationService.java
##########
@@ -327,5 +329,38 @@ public RadiusAuthenticationProtocol getRadiusEAPTTLSInnerProtocol()
         return authProtocol;
         
     }
+    
+    /**
+     * Returns the InetAddress containing the NAS IP address that should be
+     * used to identify this RADIUS client when communicating with the RADIUS
+     * server. If no explicit configuration of this property is defined
+     * in guacamole.properties, it falls back to attempting to determine the
+     * IP address using Java's built-in mechanisms for querying local addresses.
+     * 
+     * @return
+     *     The InetAddress corresponding to the NAS IP address specified in
+     *     guacamole.properties, or the IP determined by querying the address
+     *     of the server on which Guacamole is running.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed, or if the InetAddress
+     *     for the NAS IP cannot be read or determined from the system.
+     */
+    public InetAddress getRadiusNasIp() throws GuacamoleException {
+        try {
+            String nasIpStr = environment.getProperty(RadiusGuacamoleProperties.RADIUS_NAS_IP);
+            
+            // If property is specified and non-empty, attempt to return converted address.
+            if (!(nasIpStr == null == nasIpStr.isEmpty()))

Review comment:
       It's a lot less of a mind-bender if you start with the assumption that I made a typo and `==` should have actually been `&&` :rofl: 




----------------------------------------------------------------
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] [guacamole-client] necouchman commented on a change in pull request #586: GUACAMOLE-1252: Allow RADIUS NAS IP to be manually configured.

Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #586:
URL: https://github.com/apache/guacamole-client/pull/586#discussion_r553470630



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/ConfigurationService.java
##########
@@ -327,5 +329,38 @@ public RadiusAuthenticationProtocol getRadiusEAPTTLSInnerProtocol()
         return authProtocol;
         
     }
+    
+    /**
+     * Returns the InetAddress containing the NAS IP address that should be
+     * used to identify this RADIUS client when communicating with the RADIUS
+     * server. If no explicit configuration of this property is defined
+     * in guacamole.properties, it falls back to attempting to determine the
+     * IP address using Java's built-in mechanisms for querying local addresses.
+     * 
+     * @return
+     *     The InetAddress corresponding to the NAS IP address specified in
+     *     guacamole.properties, or the IP determined by querying the address
+     *     of the server on which Guacamole is running.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed, or if the InetAddress
+     *     for the NAS IP cannot be read or determined from the system.
+     */
+    public InetAddress getRadiusNasIp() throws GuacamoleException {
+        try {
+            String nasIpStr = environment.getProperty(RadiusGuacamoleProperties.RADIUS_NAS_IP);
+            
+            // If property is specified and non-empty, attempt to return converted address.
+            if (!(nasIpStr == null == nasIpStr.isEmpty()))

Review comment:
       It's a lot less of a mind-bender if you start with the assumption that I made a type and `==` should have actually been `&&` :rofl: 




----------------------------------------------------------------
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] [guacamole-client] mike-jumper commented on a change in pull request #586: GUACAMOLE-1252: Allow RADIUS NAS IP to be manually configured.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #586:
URL: https://github.com/apache/guacamole-client/pull/586#discussion_r553142030



##########
File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/ConfigurationService.java
##########
@@ -327,5 +329,38 @@ public RadiusAuthenticationProtocol getRadiusEAPTTLSInnerProtocol()
         return authProtocol;
         
     }
+    
+    /**
+     * Returns the InetAddress containing the NAS IP address that should be
+     * used to identify this RADIUS client when communicating with the RADIUS
+     * server. If no explicit configuration of this property is defined
+     * in guacamole.properties, it falls back to attempting to determine the
+     * IP address using Java's built-in mechanisms for querying local addresses.
+     * 
+     * @return
+     *     The InetAddress corresponding to the NAS IP address specified in
+     *     guacamole.properties, or the IP determined by querying the address
+     *     of the server on which Guacamole is running.
+     * 
+     * @throws GuacamoleException 
+     *     If guacamole.properties cannot be parsed, or if the InetAddress
+     *     for the NAS IP cannot be read or determined from the system.
+     */
+    public InetAddress getRadiusNasIp() throws GuacamoleException {
+        try {
+            String nasIpStr = environment.getProperty(RadiusGuacamoleProperties.RADIUS_NAS_IP);
+            
+            // If property is specified and non-empty, attempt to return converted address.
+            if (!(nasIpStr == null == nasIpStr.isEmpty()))

Review comment:
       ```java
   nasIpStr == null == nasIpStr.isEmpty()
   ```
   
   This _really_ bent my mind for a bit, until I realized that the last `== nasIpStr.isEmpty()` bit is comparing against the boolean result of `nasIpStr == null`.
   
   Won't this throw a `NullPointerException` when evaluating `nasIpStr.isEmpty()` if `nasIpStr` is null? Why not just good ol' `nasIpStr != null && !nasIpStr.isEmpty()`?




----------------------------------------------------------------
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] [guacamole-client] mike-jumper merged pull request #586: GUACAMOLE-1252: Allow RADIUS NAS IP to be manually configured.

Posted by GitBox <gi...@apache.org>.
mike-jumper merged pull request #586:
URL: https://github.com/apache/guacamole-client/pull/586


   


-- 
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