From c87ec1bf5d6545cb8f4ed631257f4ba9667bdceb Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 4 Jun 2017 14:04:56 -0700 Subject: [PATCH] GUACAMOLE-284: Reverse structure of restriction enforcement such that the default action is to deny access. --- .../JDBCAuthenticationProviderService.java | 79 ++++++++++--------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java index a5cc164ea..37ff3bcae 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderService.java @@ -88,52 +88,55 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider // Retrieve user account for already-authenticated user ModeledUser user = userService.retrieveUser(authenticationProvider, authenticatedUser); - if (user == null) { + if (user != null) { - // Do not invalidate the authentication result of users who were - // authenticated via our own connection sharing links - if (authenticatedUser instanceof SharedAuthenticatedUser) - return null; + // User data only exists for purposes of retrieval if the account + // is not disabled + UserModel userModel = user.getModel(); + if (!userModel.isDisabled()) { - // Simply return no data if a database user account is not required - if (!environment.isUserRequired()) - return null; + // Apply account restrictions if this extension authenticated + // the user OR if an account from this extension is explicitly + // required + if (authenticatedUser instanceof ModeledAuthenticatedUser + || environment.isUserRequired()) { - // Otherwise, invalidate the authentication result, as database user - // accounts are absolutely required - throw new GuacamoleInvalidCredentialsException("Invalid login", - CredentialsInfo.USERNAME_PASSWORD); + // Verify user account is still valid as of today + if (!user.isAccountValid()) + throw new GuacamoleClientException("LOGIN.ERROR_NOT_VALID"); + + // Verify user account is allowed to be used at the current time + if (!user.isAccountAccessible()) + throw new GuacamoleClientException("LOGIN.ERROR_NOT_ACCESSIBLE"); + + // Update password if password is expired + if (userModel.isExpired() || passwordPolicyService.isPasswordExpired(user)) + userService.resetExpiredPassword(user, authenticatedUser.getCredentials()); + + } + + // Link to user context + ModeledUserContext context = userContextProvider.get(); + context.init(user.getCurrentUser()); + return context; + + } } - // Apply account restrictions if this extension authenticated the user - // OR if an account from this extension is explicitly required - UserModel userModel = user.getModel(); - if (authenticatedUser instanceof ModeledAuthenticatedUser || environment.isUserRequired()) { + // Do not invalidate the authentication result of users who were + // authenticated via our own connection sharing links + if (authenticatedUser instanceof SharedAuthenticatedUser) + return null; - // If user is disabled, pretend user does not exist - if (userModel.isDisabled()) - throw new GuacamoleInvalidCredentialsException("Invalid login", - CredentialsInfo.USERNAME_PASSWORD); + // Simply return no data if a database user account is not required + if (!environment.isUserRequired()) + return null; - // Verify user account is still valid as of today - if (!user.isAccountValid()) - throw new GuacamoleClientException("LOGIN.ERROR_NOT_VALID"); - - // Verify user account is allowed to be used at the current time - if (!user.isAccountAccessible()) - throw new GuacamoleClientException("LOGIN.ERROR_NOT_ACCESSIBLE"); - - } - - // Update password if password is expired - if (userModel.isExpired() || passwordPolicyService.isPasswordExpired(user)) - userService.resetExpiredPassword(user, authenticatedUser.getCredentials()); - - // Link to user context - ModeledUserContext context = userContextProvider.get(); - context.init(user.getCurrentUser()); - return context; + // Otherwise, invalidate the authentication result, as database user + // accounts are absolutely required + throw new GuacamoleInvalidCredentialsException("Invalid login", + CredentialsInfo.USERNAME_PASSWORD); }