Move "limit unsuccessful logins" settings to core (#7645)

#### Rationale
Some IT security teams think this is a very important setting to have.
LabKey/internal-issues#907

#### Related Pull Request
- LabKey/premiumModules#546
- LabKey/testAutomation#2979

---------

Co-authored-by: labkey-tchad <[email protected]>

Author: labkey-adam

api/src/org/labkey/api/security/AuthenticationManager.java

@@ -45,6 +45,8 @@
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.data.CoreSchema;
+import org.labkey.api.data.DbScope;
+import org.labkey.api.data.DbScope.Transaction;
 import org.labkey.api.data.Project;
 import org.labkey.api.data.PropertyManager;
 import org.labkey.api.data.PropertyManager.PropertyMap;
@@ -263,6 +265,41 @@ public static boolean isAutoCreateAccountsEnabled()
 
     public static boolean isSelfServiceEmailChangesEnabled() { return getAuthSetting(SELF_SERVICE_EMAIL_CHANGES_KEY, false);}
 
+    public static boolean isLoginAttemptControlEnabled()
+    {
+        return getAuthSetting(LOGIN_ATTEMPT_ENABLED_KEY, false);
+    }
+
+    public static int getLoginAttemptLimit()
+    {
+        return getAuthenticationProperty(LOGIN_ATTEMPT_LIMIT_KEY, 3);
+    }
+
+    public static int getLoginAttemptPeriod()
+    {
+        return getAuthenticationProperty(LOGIN_ATTEMPT_PERIOD_KEY, 30);
+    }
+
+    public static int getLoginAttemptResetTime()
+    {
+        return getAuthenticationProperty(LOGIN_ATTEMPT_RESET_TIME_KEY, 5);
+    }
+
+    // Convenience method that returns the default value on missing or bad value
+    private static int getAuthenticationProperty(@NotNull String key, int defaultValue)
+    {
+        Map<String, String> props = PropertyManager.getProperties(AUTHENTICATION_CATEGORY);
+        String value = props.get(key);
+        try
+        {
+            return value == null ? defaultValue : Integer.parseInt(value);
+        }
+        catch (NumberFormatException e)
+        {
+            return defaultValue;
+        }
+    }
+
     public static @NotNull String getDefaultDomain()
     {
         Map<String, String> props = PropertyManager.getProperties(AUTHENTICATION_CATEGORY);
@@ -291,7 +328,7 @@ public static void saveAuthSetting(User user, String key, boolean value)
         saveAuthSetting(user, key, Boolean.toString(value), value ? "enabled" : "disabled");
     }
 
-    private static void saveAuthSetting(User user, String key, String value, String action)
+    public static void saveAuthSetting(User user, String key, String value, String action)
     {
         WritablePropertyMap props = PropertyManager.getWritableProperties(AUTHENTICATION_CATEGORY, true);
         props.put(key, value);
@@ -308,6 +345,41 @@ public static void saveAuthSettings(User user, Map<String, Boolean> map)
             .forEach(e->saveAuthSetting(user, e.getKey(), e.getValue()));
     }
 
+    // Returns true if any setting changed
+    public static boolean saveLoginAttemptSettings(User user, boolean enabled, int limit, int period, int resetTime)
+    {
+        if (limit < 1 ||  period < 1 || resetTime < 1)
+            throw new IllegalArgumentException("limit, period, and resetTime values must be positive!");
+
+        // Use standard saveAuthSetting() methods to ensure audit logging
+        boolean changed = false;
+        try (Transaction t = DbScope.getLabKeyScope().beginTransaction())
+        {
+            if (enabled != isLoginAttemptControlEnabled())
+            {
+                saveAuthSetting(user, LOGIN_ATTEMPT_ENABLED_KEY, enabled);
+                changed = true;
+            }
+            if (limit != getLoginAttemptLimit())
+            {
+                saveAuthSetting(user, LOGIN_ATTEMPT_LIMIT_KEY, String.valueOf(limit), "set to " + limit);
+                changed = true;
+            }
+            if (period != getLoginAttemptPeriod())
+            {
+                saveAuthSetting(user, LOGIN_ATTEMPT_PERIOD_KEY, String.valueOf(period), "set to " + period);
+                changed = true;
+            }
+            if (resetTime != getLoginAttemptResetTime())
+            {
+                saveAuthSetting(user, LOGIN_ATTEMPT_RESET_TIME_KEY, String.valueOf(resetTime), "set to " + resetTime);
+                changed = true;
+            }
+            t.commit();
+        }
+        return changed;
+    }
+
     public static void reorderConfigurations(User user, String name, int[] rowIds)
     {
         if (null != rowIds && rowIds.length != 0)
@@ -588,7 +660,8 @@ private static void addAuthSettingAuditEvent(User user, String name, String acti
         return AuthenticationProviderCache.getProvider(ResetPasswordProvider.class, name);
     }
 
-    public static @Nullable DisableLoginProvider getEnabledDisableLoginProviderForUser(String id)
+    // Return a DisableLoginProvider if it's enabled and applicable to this user
+    public static @Nullable DisableLoginProvider getDisableLoginProviderForUser(String id)
     {
         for (DisableLoginProvider provider : AuthenticationProviderCache.getProviders(DisableLoginProvider.class))
             if (provider.isEnabledForUser(id))
@@ -631,19 +704,27 @@ public static boolean isAcceptOnlyFicamProviders()
 
     public static void setAcceptOnlyFicamProviders(User user, boolean enable)
     {
-        saveAuthSetting(user, ACCEPT_ONLY_FICAM_PROVIDERS_KEY, enable);
-        AuthenticationConfigurationCache.clear();
+        if (isAcceptOnlyFicamProviders() != enable)
+        {
+            saveAuthSetting(user, ACCEPT_ONLY_FICAM_PROVIDERS_KEY, enable);
+            AuthenticationConfigurationCache.clear();
+        }
     }
 
-    // Used by start-up properties
-    private static final String AUTHENTICATION_CATEGORY = "Authentication";
+    // Used by start-up properties and upgrade code
+    public static final String AUTHENTICATION_CATEGORY = "Authentication";
 
     public static final String SELF_REGISTRATION_KEY = "SelfRegistration";
     public static final String AUTO_CREATE_ACCOUNTS_KEY = "AutoCreateAccounts";
     public static final String DEFAULT_DOMAIN = "DefaultDomain";
     public static final String SELF_SERVICE_EMAIL_CHANGES_KEY = "SelfServiceEmailChanges";
     public static final String ACCEPT_ONLY_FICAM_PROVIDERS_KEY = "AcceptOnlyFicamProviders";
 
+    public static final String LOGIN_ATTEMPT_ENABLED_KEY = "LoginAttemptEnabled";
+    public static final String LOGIN_ATTEMPT_LIMIT_KEY = "LoginAttemptLimit";
+    public static final String LOGIN_ATTEMPT_PERIOD_KEY = "LoginAttemptPeriod";
+    public static final String LOGIN_ATTEMPT_RESET_TIME_KEY = "LoginAttemptResetTime";
+
     public enum AuthenticationSettings implements StartupProperty
     {
         SelfRegistration("Allow self sign up"),
@@ -1165,17 +1246,23 @@ public static PrimaryAuthenticationResult finalizePrimaryAuthentication(HttpServ
 
     // limit one bad login per second averaged out over 60sec
     private static final Cache<Integer, RateLimiter> addrLimiter = CacheManager.getCache(1001, TimeUnit.MINUTES.toMillis(5), "Login limiter");
-    private static final Cache<Integer, RateLimiter> userLimiter = CacheManager.getCache(1001, TimeUnit.MINUTES.toMillis(5), "User limiter");
     private static final Cache<Integer, RateLimiter> pwdLimiter = CacheManager.getCache(1001, TimeUnit.MINUTES.toMillis(5), "Password limiter");
-    private static final CacheLoader<Integer, RateLimiter> addrLoader = (key, request) -> new RateLimiter("Addr limiter: " + key, new Rate(60, TimeUnit.MINUTES));
-    private static final CacheLoader<Integer, RateLimiter> pwdLoader = (key, request) -> new RateLimiter("Pwd limiter: " + key, new Rate(20, TimeUnit.MINUTES));
-    private static final CacheLoader<Integer, RateLimiter> userLoader = (key, request) -> new RateLimiter("User limiter: " + key, new Rate(20, TimeUnit.MINUTES));
+    private static final CacheLoader<Integer, RateLimiter> addrLoader = (key, _) -> new RateLimiter("Addr limiter: " + key, new Rate(60, TimeUnit.MINUTES));
+    private static final CacheLoader<Integer, RateLimiter> pwdLoader = (key, _) -> new RateLimiter("Pwd limiter: " + key, new Rate(20, TimeUnit.MINUTES));
+
+    private static final Cache<String, RateLimiter> userLimiter = CacheManager.getCache(10000, TimeUnit.MINUTES.toMillis(5), "User limiter");
+    private static final CacheLoader<String, RateLimiter> userLoader = (key, _) -> new RateLimiter("User limiter: " + key, new Rate(20, TimeUnit.MINUTES));
 
-    private static Integer _toKey(String s)
+    private static Integer getIntCacheKey(String s)
     {
         return null==s ? 0 : s.toLowerCase().hashCode() % 1000;
     }
 
+    public static String getEmailCacheKey(String s)
+    {
+        return StringUtils.trimToEmpty(s).toLowerCase();
+    }
+
     private static PrimaryAuthenticationResult _beforeAuthenticate(HttpServletRequest request, String id, String pwd)
     {
         if (null == id || null == pwd)
@@ -1184,10 +1271,10 @@ private static PrimaryAuthenticationResult _beforeAuthenticate(HttpServletReques
         long delay = 0;
 
         // slow down login attempts when we detect more than 20/minute bad attempts per user, password, or ip address
-        rl = addrLimiter.get(_toKey(request == null ? null : request.getRemoteAddr()));
+        rl = addrLimiter.get(getIntCacheKey(request == null ? null : request.getRemoteAddr()));
         if (null != rl)
             delay = Math.max(delay,rl.add(0, false));
-         rl = pwdLimiter.get(_toKey(pwd));
+         rl = pwdLimiter.get(getIntCacheKey(pwd));
         if (null != rl)
             delay = Math.max(delay, rl.add(0, false));
 
@@ -1209,15 +1296,15 @@ private static PrimaryAuthenticationResult _beforeAuthenticate(HttpServletReques
 
     private static long getUserLoginDelay(String id) throws LoginDisabledException
     {
-        DisableLoginProvider provider = AuthenticationManager.getEnabledDisableLoginProviderForUser(id);
+        DisableLoginProvider provider = AuthenticationManager.getDisableLoginProviderForUser(id);
         if (provider != null)
             return provider.getUserDelay(id);
         return getDefaultUserLoginDelay(id);
     }
 
     private static long getDefaultUserLoginDelay(String id)
     {
-        RateLimiter rl = userLimiter.get(_toKey(id));
+        RateLimiter rl = userLimiter.get(getEmailCacheKey(id));
         if (null != rl)
             return rl.add(0, false);
         return 0;
@@ -1230,9 +1317,9 @@ private static void _afterAuthenticate(HttpServletRequest request, String id, St
         if (result.getStatus() ==  AuthenticationStatus.BadCredentials || result.getStatus() == AuthenticationStatus.InactiveUser)
         {
             RateLimiter rl;
-            rl = addrLimiter.get(_toKey(request.getRemoteAddr()),request, addrLoader);
+            rl = addrLimiter.get(getIntCacheKey(request.getRemoteAddr()),request, addrLoader);
             rl.add(1, false);
-            rl = pwdLimiter.get(_toKey(pwd),request, pwdLoader);
+            rl = pwdLimiter.get(getIntCacheKey(pwd),request, pwdLoader);
             rl.add(1, false);
 
             addUserLoginDelay(request, id);
@@ -1245,14 +1332,14 @@ else if (result.getStatus() ==  AuthenticationStatus.Success)
 
     private static void resetModuleUserLoginDelay(String id)
     {
-        DisableLoginProvider provider = AuthenticationManager.getEnabledDisableLoginProviderForUser(id);
+        DisableLoginProvider provider = AuthenticationManager.getDisableLoginProviderForUser(id);
         if (provider != null)
             provider.resetUserDelay(id);
     }
 
     private static void addUserLoginDelay(HttpServletRequest request, String id)
     {
-        DisableLoginProvider provider = AuthenticationManager.getEnabledDisableLoginProviderForUser(id);
+        DisableLoginProvider provider = AuthenticationManager.getDisableLoginProviderForUser(id);
         if (provider != null)
             provider.addUserDelay(request, id, 1);
         else
@@ -1261,7 +1348,7 @@ private static void addUserLoginDelay(HttpServletRequest request, String id)
 
     private static void addDefaultUserLoginDelay(HttpServletRequest request, String id)
     {
-        RateLimiter rl = userLimiter.get(_toKey(id),request, userLoader);
+        RateLimiter rl = userLimiter.get(getEmailCacheKey(id),request, userLoader);
         rl.add(1, false);
     }
 

core/module.properties

@@ -1,6 +1,6 @@
 Name: Core
 ModuleClass: org.labkey.core.CoreModule
-SchemaVersion: 26.004
+SchemaVersion: 26.005
 Label: Administration and Essential Services
 Description: The Core module provides central services such as login, \
     security, administration, folder management, user management, \

core/resources/schemas/dbscripts/postgresql/core-26.004-26.005.sql

@@ -0,0 +1,2 @@
+-- Migrate login attempt settings from the compliance module's property store to core authentication settings.
+SELECT core.executeJavaUpgradeCode('migrateLoginAttemptSettings');

core/resources/schemas/dbscripts/sqlserver/core-26.004-26.005.sql

@@ -0,0 +1,2 @@
+-- Migrate login attempt settings from the compliance module's property store to core authentication settings.
+EXEC core.executeJavaUpgradeCode 'migrateLoginAttemptSettings';

core/src/client/AuthenticationConfiguration/authenticationConfiguration.scss

@@ -27,6 +27,15 @@
   margin-bottom: 14px;
 }
 
+.global-settings__text-row-section {
+  margin-left: 17px;
+  margin-top: 8px;
+}
+
+.global-settings__text-row-section.disabled {
+  opacity: 0.5;
+}
+
 .global-settings__text {
   margin-left: 14px;
 }

core/src/client/components/GlobalSettings.test.tsx

@@ -6,19 +6,12 @@ import { GLOBAL_SETTINGS } from '../../../test/data';
 
 import { GlobalSettings } from './GlobalSettings';
 
-describe('<GlobalSettings/>', () => {
+describe('GlobalSettings', () => {
     test('Clicking a checkbox toggles the checkbox', async () => {
         const checkGlobalAuthBox = jest.fn();
-        render(
-            <GlobalSettings
-                globalSettings={GLOBAL_SETTINGS}
-                canEdit={true}
-                authCount={3}
-                onChange={checkGlobalAuthBox}
-            />
-        );
+        render(<GlobalSettings authCount={3} canEdit globalSettings={GLOBAL_SETTINGS} onChange={checkGlobalAuthBox} />);
 
-        // Click self registration checkbox
+        // Click self-registration checkbox
         const firstCheckBox = document.querySelector('input[type="checkbox"]');
         await userEvent.click(firstCheckBox);
         expect(checkGlobalAuthBox).toHaveBeenCalled();
@@ -27,32 +20,20 @@ describe('<GlobalSettings/>', () => {
     test('An authCount of 1 eliminates the option to auto-create authenticated users', () => {
         const checkGlobalAuthBox = jest.fn();
         const { rerender } = render(
-            <GlobalSettings
-                authCount={3}
-                canEdit={true}
-                globalSettings={GLOBAL_SETTINGS}
-                onChange={checkGlobalAuthBox}
-            />
+            <GlobalSettings authCount={3} canEdit globalSettings={GLOBAL_SETTINGS} onChange={checkGlobalAuthBox} />
         );
 
-        expect(document.querySelectorAll('input[type="checkbox"]').length).toBe(3);
+        expect(document.querySelectorAll('input[type="checkbox"]')).toHaveLength(4);
         rerender(
-            <GlobalSettings
-                authCount={1}
-                canEdit={true}
-                globalSettings={GLOBAL_SETTINGS}
-                onChange={checkGlobalAuthBox}
-            />
+            <GlobalSettings authCount={1} canEdit globalSettings={GLOBAL_SETTINGS} onChange={checkGlobalAuthBox} />
         );
-        expect(document.querySelectorAll('input[type="checkbox"]').length).toBe(2);
+        expect(document.querySelectorAll('input[type="checkbox"]')).toHaveLength(3);
         expect(document.querySelector('.panel-body').innerHTML).not.toMatch(/Auto-create authenticated users/);
     });
 
     test('view-only mode', () => {
-        render(
-            <GlobalSettings globalSettings={GLOBAL_SETTINGS} canEdit={false} authCount={3} onChange={jest.fn()} />
-        );
+        render(<GlobalSettings authCount={3} canEdit={false} globalSettings={GLOBAL_SETTINGS} onChange={jest.fn()} />);
 
-        expect(document.querySelectorAll('input[disabled=""]')).toHaveLength(4);
+        expect(document.querySelectorAll('input[disabled=""]')).toHaveLength(5);
     });
 });