mike-jumper commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r273619494
##########
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java
##########
@@ -129,6 +133,18 @@ private RadiusAuthenticator
setupRadiusAuthenticator(RadiusClient radiusClient)
if (radAuth == null)
throw new GuacamoleException("Could not get a valid
RadiusAuthenticator for specified protocol: " +
confService.getRadiusAuthProtocol());
+ // For MSCHAPv1/2, we need MD4 support
+ if (radAuth instanceof MSCHAPv1Authenticator
+ || radAuth instanceof MSCHAPv2Authenticator) {
Review comment:
Rather than checking `instanceof` everywhere, and thus relying on the
internals of JRadius, it would be better if `getRadiusAuthProtocol()` leveraged
an `enum` that could be checked independently of whatever JRadius decides to
return. Such an `enum` could provide a method for obtaining a corresponding
instance of `RadiusAuthenticator` given a `RadiusClient`.
Rechecking `getRadiusAuthProtocol()`, it is somewhat scary that the raw
string value is passed straight to JRadius. I think there should be some
validation on the value on the Guacamole side, and a decoupling from the
underlying RADIUS library.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
