[GitHub] guacamole-client pull request #242: GUACAMOLE-197: Convert state to Hex stri...

[necouchman]

Github user necouchman commented on a diff in the pull request:

https://github.com/apache/guacamole-client/pull/242#discussion_r165853678
  
--- Diff: 
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java
 ---
@@ -155,9 +158,21 @@ public AuthenticatedUser authenticateUser(Credentials 
credentials)
 // This is a response to a previous challenge, authenticate with 
that.
 else {
 try {
+String stateString = 
request.getParameter(RadiusStateField.PARAMETER_NAME);
+if (stateString == null) {
+logger.warn("Expected state parameter was not present 
in challenge/response.");
+throw new 
GuacamoleInvalidCredentialsException("Authentication error.", 
CredentialsInfo.USERNAME_PASSWORD);
+}
+
+byte[] stateBytes = 
DatatypeConverter.parseHexBinary(stateString);
 radPack = 
radiusService.sendChallengeResponse(credentials.getUsername(),
- challengeResponse,
- 
request.getParameter(RadiusStateField.PARAMETER_NAME));
+  
challengeResponse,
+  stateBytes);
+}
+catch (IllegalArgumentException e) {
+logger.warn("Illegal hexadecimal value while parsing 
RADIUS state string.", e.getMessage());
+logger.debug("Encountered exception while attepmting to 
perse the hexidecimanl state value.", e);
--- End diff --

Whoa, not my best job of speling.


---

[GitHub] guacamole-client pull request #242: GUACAMOLE-197: Convert state to Hex stri...

[mike-jumper]

Github user mike-jumper commented on a diff in the pull request:

https://github.com/apache/guacamole-client/pull/242#discussion_r165853668
  
--- Diff: 
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/AuthenticationProviderService.java
 ---
@@ -155,9 +158,21 @@ public AuthenticatedUser authenticateUser(Credentials 
credentials)
 // This is a response to a previous challenge, authenticate with 
that.
 else {
 try {
+String stateString = 
request.getParameter(RadiusStateField.PARAMETER_NAME);
+if (stateString == null) {
+logger.warn("Expected state parameter was not present 
in challenge/response.");
+throw new 
GuacamoleInvalidCredentialsException("Authentication error.", 
CredentialsInfo.USERNAME_PASSWORD);
+}
+
+byte[] stateBytes = 
DatatypeConverter.parseHexBinary(stateString);
 radPack = 
radiusService.sendChallengeResponse(credentials.getUsername(),
- challengeResponse,
- 
request.getParameter(RadiusStateField.PARAMETER_NAME));
+  
challengeResponse,
+  stateBytes);
+}
+catch (IllegalArgumentException e) {
+logger.warn("Illegal hexadecimal value while parsing 
RADIUS state string.", e.getMessage());
--- End diff --

If you are going to include `e.getMessage()` within the log message, you 
will need to use the `{}` placeholder (analogous to `%s` of `printf`). See:


https://github.com/apache/guacamole-client/blob/9b5483edc24d88a05075343c76377057de09f9e8/extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java#L127


---