GUACAMOLE-1152: Merge correct handling of client vs. server exceptions.

This commit is contained in:
Virtually Nick
2020-08-20 12:19:25 -04:00
committed by GitHub
3 changed files with 44 additions and 69 deletions

View File

@@ -21,12 +21,12 @@ package org.apache.guacamole.extension;
import java.util.Set;
import java.util.UUID;
import org.apache.guacamole.GuacamoleClientException;
import org.apache.guacamole.GuacamoleException;
import org.apache.guacamole.net.auth.AuthenticatedUser;
import org.apache.guacamole.net.auth.AuthenticationProvider;
import org.apache.guacamole.net.auth.Credentials;
import org.apache.guacamole.net.auth.UserContext;
import org.apache.guacamole.net.auth.credentials.GuacamoleCredentialsException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -190,41 +190,15 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
return authProvider.authenticateUser(credentials);
}
// Pass through credential exceptions untouched, as these are not
// internal failures
catch (GuacamoleCredentialsException e) {
// Pass through client exceptions untouched, including credential
// exceptions, as these are not internal failures
catch (GuacamoleClientException e) {
throw e;
}
// Pass through all other exceptions (aborting authentication entirely)
// only if not configured to ignore such failures
catch (GuacamoleException e) {
// Skip using this authentication provider if configured to ignore
// internal failures during auth
if (isFailureTolerated()) {
warnAuthProviderSkipped(e);
return null;
}
warnAuthAborted();
throw e;
}
catch (RuntimeException e) {
// Skip using this authentication provider if configured to ignore
// internal failures during auth
if (isFailureTolerated()) {
warnAuthProviderSkipped(e);
return null;
}
warnAuthAborted();
throw e;
}
catch (Error e) {
catch (GuacamoleException | RuntimeException | Error e) {
// Skip using this authentication provider if configured to ignore
// internal failures during auth
@@ -274,41 +248,15 @@ public class AuthenticationProviderFacade implements AuthenticationProvider {
return authProvider.getUserContext(authenticatedUser);
}
// Pass through credential exceptions untouched, as these are not
// internal failures
catch (GuacamoleCredentialsException e) {
// Pass through client exceptions untouched, including credential
// exceptions, as these are not internal failures
catch (GuacamoleClientException e) {
throw e;
}
// Pass through all other exceptions (aborting authentication entirely)
// only if not configured to ignore such failures
catch (GuacamoleException e) {
// Skip using this authentication provider if configured to ignore
// internal failures during auth
if (isFailureTolerated()) {
warnAuthProviderSkipped(e);
return null;
}
warnAuthAborted();
throw e;
}
catch (RuntimeException e) {
// Skip using this authentication provider if configured to ignore
// internal failures during auth
if (isFailureTolerated()) {
warnAuthProviderSkipped(e);
return null;
}
warnAuthAborted();
throw e;
}
catch (Error e) {
catch (GuacamoleException | RuntimeException | Error e) {
// Skip using this authentication provider if configured to ignore
// internal failures during auth

View File

@@ -27,6 +27,7 @@ import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;
import org.apache.guacamole.GuacamoleClientException;
import org.apache.guacamole.GuacamoleException;
import org.apache.guacamole.GuacamoleUnauthorizedException;
import org.apache.guacamole.rest.auth.AuthenticationService;
@@ -91,14 +92,25 @@ public class RESTExceptionMapper implements ExceptionMapper<Throwable> {
}
// Translate GuacamoleException subclasses to HTTP error codes
if (t instanceof GuacamoleException)
if (t instanceof GuacamoleException) {
// Always log the human-readable details of GuacacamoleExceptions
// for the benefit of the administrator
if (t instanceof GuacamoleClientException)
logger.debug("Client request rejected: {}", t.getMessage());
else {
logger.error("Request could not be processed: {}", t.getMessage());
logger.debug("Processing of request aborted by extension.", t);
}
return Response
.status(((GuacamoleException) t).getHttpStatusCode())
.entity(new APIError((GuacamoleException) t))
.type(MediaType.APPLICATION_JSON)
.build();
}
// Rethrow unchecked exceptions such that they are properly wrapped
// Wrap unchecked exceptions
String message = t.getMessage();
if (message != null)
logger.error("Unexpected internal error: {}", message);

View File

@@ -71,6 +71,23 @@ angular.module('login').directive('guacLogin', [function guacLogin() {
var authenticationService = $injector.get('authenticationService');
var requestService = $injector.get('requestService');
/**
* The initial value for all login fields. Note that this value must
* not be null. If null, empty fields may not be submitted back to the
* server at all, causing the request to misrepresent true login state.
*
* For example, if a user receives an insufficient credentials error
* due to their password expiring, failing to provide that new password
* should result in the user submitting their username, original
* password, and empty new password. If only the username and original
* password are sent, the invalid password reset request will be
* indistinguishable from a normal login attempt.
*
* @constant
* @type String
*/
var DEFAULT_FIELD_VALUE = '';
/**
* A description of the error that occurred during login, if any.
*
@@ -148,7 +165,7 @@ angular.module('login').directive('guacLogin', [function guacLogin() {
// Set default values for all unset fields
angular.forEach($scope.remainingFields, function setDefault(field) {
if (!$scope.enteredValues[field.name])
$scope.enteredValues[field.name] = '';
$scope.enteredValues[field.name] = DEFAULT_FIELD_VALUE;
});
$scope.relevantField = getRelevantField();
@@ -195,13 +212,11 @@ angular.module('login').directive('guacLogin', [function guacLogin() {
else
$scope.loginError = error.translatableMessage;
// Clear all remaining fields that are not username fields
// Reset all remaining fields to default values, but
// preserve any usernames
angular.forEach($scope.remainingFields, function clearEnteredValueIfPassword(field) {
// If field is not username field, delete it.
if (field.type !== Field.Type.USERNAME && field.name in $scope.enteredValues)
delete $scope.enteredValues[field.name];
$scope.enteredValues[field.name] = DEFAULT_FIELD_VALUE;
});
}