GUACAMOLE-937: Return null on bindAs() failures. Rely on caller to interpret and handle failures.

Throwing GuacamoleInvalidCredentialsException breaks separation of
concerns (bindAs() shouldn't assume that it's being used during login
and that credentials given are the Guacamole user's credentials), and
has unintended side effects (throwing subclasses of
GuacamoleUnauthorizedException causes implicit session invalidation).
This commit is contained in:
Michael Jumper
2020-01-22 23:05:03 -08:00
parent 3805a7527d
commit 2cb0efeda6
2 changed files with 21 additions and 10 deletions

View File

@@ -204,6 +204,10 @@ public class AuthenticationProviderService {
// Attempt bind // Attempt bind
LdapNetworkConnection ldapConnection = ldapService.bindAs(bindDn, password); LdapNetworkConnection ldapConnection = ldapService.bindAs(bindDn, password);
if (ldapConnection == null)
throw new GuacamoleInvalidCredentialsException("Invalid login.",
CredentialsInfo.USERNAME_PASSWORD);
try { try {
// Retrieve group membership of the user that just authenticated // Retrieve group membership of the user that just authenticated
@@ -309,8 +313,16 @@ public class AuthenticationProviderService {
// Bind using credentials associated with AuthenticatedUser // Bind using credentials associated with AuthenticatedUser
Credentials credentials = authenticatedUser.getCredentials(); Credentials credentials = authenticatedUser.getCredentials();
if (authenticatedUser instanceof LDAPAuthenticatedUser) { if (authenticatedUser instanceof LDAPAuthenticatedUser) {
Dn bindDn = ((LDAPAuthenticatedUser) authenticatedUser).getBindDn(); Dn bindDn = ((LDAPAuthenticatedUser) authenticatedUser).getBindDn();
LdapNetworkConnection ldapConnection = ldapService.bindAs(bindDn, credentials.getPassword()); LdapNetworkConnection ldapConnection = ldapService.bindAs(bindDn, credentials.getPassword());
if (ldapConnection == null) {
logger.debug("LDAP bind succeeded for \"{}\" during "
+ "authentication but failed during data retrieval.",
authenticatedUser.getIdentifier());
throw new GuacamoleInvalidCredentialsException("Invalid login.",
CredentialsInfo.USERNAME_PASSWORD);
}
try { try {

View File

@@ -144,13 +144,12 @@ public class LDAPConnectionService {
bindRequest.setDn(userDN); bindRequest.setDn(userDN);
bindRequest.setCredentials(password); bindRequest.setCredentials(password);
BindResponse bindResponse = ldapConnection.bind(bindRequest); BindResponse bindResponse = ldapConnection.bind(bindRequest);
if (bindResponse.getLdapResult().getResultCode() == ResultCodeEnum.SUCCESS)
return ldapConnection; if (bindResponse.getLdapResult().getResultCode() != ResultCodeEnum.SUCCESS) {
ldapConnection.close();
else logger.debug("LDAP bind attempt failed: {}", bindResponse.toString());
throw new GuacamoleInvalidCredentialsException("Error binding" return null;
+ " to server: " + bindResponse.toString(), }
CredentialsInfo.USERNAME_PASSWORD);
} }
@@ -158,11 +157,11 @@ public class LDAPConnectionService {
catch (LdapException e) { catch (LdapException e) {
ldapConnection.close(); ldapConnection.close();
logger.debug("Unable to bind to LDAP server.", e); logger.debug("Unable to bind to LDAP server.", e);
throw new GuacamoleInvalidCredentialsException( return null;
"Unable to bind to the LDAP server.",
CredentialsInfo.USERNAME_PASSWORD);
} }
return ldapConnection;
} }
/** /**