From 0eef629a9dad12ad6d60a0d045e845236761be88 Mon Sep 17 00:00:00 2001
From: Michael Jumper <mjumper@apache.org>
Date: Sun, 4 Jun 2017 13:42:28 -0700
Subject: [PATCH] GUACAMOLE-284: Move enforcement of account restrictions into
 AuthenticationProviderService.

---
 .../JDBCAuthenticationProviderService.java    | 21 +++++++++++++++----
 .../guacamole/auth/jdbc/user/UserService.java | 19 ++++-------------
 2 files changed, 21 insertions(+), 19 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 2e85e788c..a5cc164ea 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
@@ -21,9 +21,11 @@ package org.apache.guacamole.auth.jdbc;
 
 import com.google.inject.Inject;
 import com.google.inject.Provider;
+import org.apache.guacamole.GuacamoleClientException;
 import org.apache.guacamole.GuacamoleException;
 import org.apache.guacamole.auth.jdbc.security.PasswordPolicyService;
 import org.apache.guacamole.auth.jdbc.sharing.user.SharedAuthenticatedUser;
+import org.apache.guacamole.auth.jdbc.user.ModeledAuthenticatedUser;
 import org.apache.guacamole.auth.jdbc.user.ModeledUser;
 import org.apache.guacamole.auth.jdbc.user.ModeledUserContext;
 import org.apache.guacamole.auth.jdbc.user.UserModel;
@@ -104,13 +106,24 @@ public class JDBCAuthenticationProviderService implements AuthenticationProvider
 
         }
 
-        // Veto authentication result if account is required but unavailable
-        // due to account restrictions
+        // Apply account restrictions if this extension authenticated the user
+        // OR if an account from this extension is explicitly required
         UserModel userModel = user.getModel();
-        if (environment.isUserRequired()
-                && (userModel.isDisabled() || !user.isAccountValid() || !user.isAccountAccessible())) {
+        if (authenticatedUser instanceof ModeledAuthenticatedUser || environment.isUserRequired()) {
+
+            // If user is disabled, pretend user does not exist
+            if (userModel.isDisabled())
                 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
diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java
index 7935f864d..3dc025fcd 100644
--- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java
+++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/UserService.java
@@ -312,9 +312,10 @@ public class UserService extends ModeledDirectoryObjectService<ModeledUser, User
 
     /**
      * Retrieves the user corresponding to the given credentials from the
-     * database. If the user account is expired, and the credentials contain
-     * the necessary additional parameters to reset the user's password, the
-     * password is reset.
+     * database. Note that this function will not enforce any additional
+     * account restrictions, including explicitly disabled accounts,
+     * scheduling, and password expiration. It is the responsibility of the
+     * caller to enforce such restrictions, if desired.
      *
      * @param authenticationProvider
      *     The AuthenticationProvider on behalf of which the user is being
@@ -342,10 +343,6 @@ public class UserService extends ModeledDirectoryObjectService<ModeledUser, User
         if (userModel == null)
             return null;
 
-        // If user is disabled, pretend user does not exist
-        if (userModel.isDisabled())
-            return null;
-
         // Verify provided password is correct
         byte[] hash = encryptionService.createPasswordHash(password, userModel.getPasswordSalt());
         if (!Arrays.equals(hash, userModel.getPasswordHash()))
@@ -355,14 +352,6 @@ public class UserService extends ModeledDirectoryObjectService<ModeledUser, User
         ModeledUser user = getObjectInstance(null, userModel);
         user.setCurrentUser(new ModeledAuthenticatedUser(authenticationProvider, user, credentials));
 
-        // 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");
-
         // Return now-authenticated user
         return user.getCurrentUser();