[MW-1476] Add user lockout API and unlock endpoint

CHANGELOG.md

@@ -3,6 +3,7 @@ Upcoming Version (WIP)
 
 Improvements:
 * Stabilized consul registration and health checks
+* [MW-1476](https://openlmis.atlassian.net/browse/MW-1476): Add user lockout API and unlock endpoint
 
 Bug fixes:
 * [OLMIS-8223](https://openlmis.atlassian.net/browse/OLMIS-8223): Fix placeholder bug in messages

src/integration-test/java/org/openlmis/auth/web/TokenIntegrationTest.java

@@ -18,22 +18,34 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.BDDMockito.given;
 import static org.openlmis.auth.web.TestWebData.Fields;
 import static org.openlmis.auth.web.TestWebData.GrantTypes;
 import static org.openlmis.auth.web.TestWebData.Tokens.DURATION;
 
 import java.util.Optional;
+import java.util.UUID;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.openlmis.auth.ApiKeyDataBuilder;
 import org.openlmis.auth.DummyUserMainDetailsDto;
 import org.openlmis.auth.domain.ApiKey;
 import org.openlmis.auth.domain.Client;
+import org.openlmis.auth.domain.UnsuccessfulAuthenticationAttempt;
+import org.openlmis.auth.repository.UnsuccessfulAuthenticationAttemptRepository;
+import org.openlmis.auth.repository.UserRepository;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.oauth2.common.OAuth2AccessToken;
 
 public class TokenIntegrationTest extends BaseWebIntegrationTest {
 
+  @Autowired
+  private UserRepository userRepository;
+
+  @Autowired
+  private UnsuccessfulAuthenticationAttemptRepository attemptCounterRepository;
+
   @BeforeClass
   public static void setUpClass() {
     System.setProperty("TOKEN_DURATION", String.valueOf(DURATION));
@@ -62,6 +74,43 @@ public void shouldSetExpirationForTokens() {
     assertEquals(DURATION, token.getExpiresIn(), 5.0);
   }
 
+  @Test
+  public void shouldPersistFailedLoginAttemptCounter() {
+    // Guards against the failed-attempt counter being rolled back when authenticate() throws
+    // (the whole attempt runs in one transaction, so the increment must commit despite the
+    // thrown AuthenticationException).
+    UUID userId = UUID.fromString(DummyUserMainDetailsDto.REFERENCE_ID);
+    resetLockoutState(userId);
+
+    Client client = mockUserClient();
+    startRequest()
+        .auth()
+        .preemptive()
+        .basic(client.getClientId(), client.getClientSecret())
+        .queryParam(Fields.GRANT_TYPE, GrantTypes.PASSWORD)
+        .queryParam(Fields.USERNAME, DummyUserMainDetailsDto.USERNAME)
+        .queryParam(Fields.PASSWORD, "wrong-password")
+        .when()
+        .post("/api/oauth/token")
+        .then()
+        .statusCode(400);
+
+    Optional<UnsuccessfulAuthenticationAttempt> counter =
+        attemptCounterRepository.findByUserId(userId);
+    assertTrue(counter.isPresent());
+    assertEquals(Integer.valueOf(1), counter.get().getAttemptCounter());
+
+    resetLockoutState(userId);
+  }
+
+  private void resetLockoutState(UUID userId) {
+    attemptCounterRepository.findByUserId(userId).ifPresent(attemptCounterRepository::delete);
+    userRepository.findById(userId).ifPresent(user -> {
+      user.setLockedOut(false);
+      userRepository.save(user);
+    });
+  }
+
   @Test
   public void shouldNotSetExpirationForApiKeys() {
     // given

src/integration-test/java/org/openlmis/auth/web/UserControllerIntegrationTest.java

@@ -21,6 +21,7 @@
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -50,20 +51,24 @@
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.openlmis.auth.DummyUserMainDetailsDto;
 import org.openlmis.auth.domain.Client;
 import org.openlmis.auth.domain.PasswordResetToken;
+import org.openlmis.auth.domain.UnsuccessfulAuthenticationAttempt;
 import org.openlmis.auth.domain.User;
 import org.openlmis.auth.dto.PasswordResetRequestDto;
+import org.openlmis.auth.dto.UnlockResponseDto;
 import org.openlmis.auth.dto.UserAuthDetailsResponseDto;
 import org.openlmis.auth.dto.UserDto;
 import org.openlmis.auth.dto.referencedata.UserMainDetailsDto;
 import org.openlmis.auth.exception.PermissionMessageException;
 import org.openlmis.auth.i18n.MessageKeys;
 import org.openlmis.auth.repository.PasswordResetTokenRepository;
+import org.openlmis.auth.repository.UnsuccessfulAuthenticationAttemptRepository;
 import org.openlmis.auth.repository.UserRepository;
 import org.openlmis.auth.service.PasswordResetRegistryService;
 import org.openlmis.auth.service.PermissionService;
@@ -87,6 +92,8 @@ public class UserControllerIntegrationTest extends BaseWebIntegrationTest {
 
   private static final String RESOURCE_URL = "/api/users/auth";
   private static final String BATCH_RESOURCE_URL = RESOURCE_URL + "/batch";
+  private static final String UNLOCK_URL = RESOURCE_URL + "/unlock";
+  private static final String LOCKED_OUT_PARAM = "lockedOut";
   private static final String ID_URL = RESOURCE_URL + "/{id}";
   private static final String RESET_PASS_URL = RESOURCE_URL + "/passwordReset";
   private static final String FORGOT_PASS_URL = RESOURCE_URL + "/forgotPassword";
@@ -100,6 +107,9 @@ public class UserControllerIntegrationTest extends BaseWebIntegrationTest {
   @Autowired
   private UserRepository userRepository;
 
+  @Autowired
+  private UnsuccessfulAuthenticationAttemptRepository attemptCounterRepository;
+
   @Autowired
   private PasswordResetTokenRepository passwordResetTokenRepository;
 
@@ -150,6 +160,17 @@ public void setUp() {
     userContactDetailsDto.setReferenceDataUserId(user.getId());
   }
 
+  @After
+  public void removeTestLockedUsers() {
+    for (User user : userRepository.findAll()) {
+      if (user.getUsername() != null && user.getUsername().startsWith("lockedUser_")) {
+        attemptCounterRepository.findByUserId(user.getId())
+            .ifPresent(attemptCounterRepository::delete);
+        userRepository.delete(user);
+      }
+    }
+  }
+
   @Test
   public void shouldSaveUser() {
     userDto.setId(UUID.randomUUID());
@@ -476,6 +497,110 @@ public void shouldGetAuthUsers() {
     assertEquals(1, actual.size());
   }
 
+  @Test
+  public void shouldGetAuthUsersWithActualLockoutState() {
+    User locked = saveLockedUser();
+
+    UserDto[] users = startRequest(USER_TOKEN)
+        .header(CONTENT_TYPE_HEADER, APPLICATION_JSON_VALUE)
+        .when()
+        .get(BATCH_RESOURCE_URL)
+        .then()
+        .statusCode(200)
+        .extract()
+        .as(UserDto[].class);
+
+    UserDto returned = findById(users, locked.getId());
+    assertNotNull(returned);
+    assertTrue(returned.isLockedOut());
+  }
+
+  @Test
+  public void shouldReturnLockedUsersWhenFilteringByLockedOutTrue() {
+    User locked = saveLockedUser();
+
+    UserDto[] lockedUsers = startRequest(USER_TOKEN)
+        .queryParam(LOCKED_OUT_PARAM, "true")
+        .header(CONTENT_TYPE_HEADER, APPLICATION_JSON_VALUE)
+        .when()
+        .get(BATCH_RESOURCE_URL)
+        .then()
+        .statusCode(200)
+        .extract()
+        .as(UserDto[].class);
+
+    assertNotNull(findById(lockedUsers, locked.getId()));
+    assertTrue(Arrays.stream(lockedUsers).allMatch(UserDto::isLockedOut));
+  }
+
+  @Test
+  public void shouldNotReturnLockedUsersWhenFilteringByLockedOutFalse() {
+    User locked = saveLockedUser();
+
+    UserDto[] unlockedUsers = startRequest(USER_TOKEN)
+        .queryParam(LOCKED_OUT_PARAM, "false")
+        .header(CONTENT_TYPE_HEADER, APPLICATION_JSON_VALUE)
+        .when()
+        .get(BATCH_RESOURCE_URL)
+        .then()
+        .statusCode(200)
+        .extract()
+        .as(UserDto[].class);
+
+    assertNull(findById(unlockedUsers, locked.getId()));
+  }
+
+  @Test
+  public void shouldUnlockUsersAndResetCounter() {
+    User locked = saveLockedUser();
+    UnsuccessfulAuthenticationAttempt counter = new UnsuccessfulAuthenticationAttempt(locked);
+    counter.incrementCounter();
+    attemptCounterRepository.save(counter);
+
+    UUID missingId = UUID.randomUUID();
+
+    UnlockResponseDto response = startRequest(USER_TOKEN)
+        .header(CONTENT_TYPE_HEADER, APPLICATION_JSON_VALUE)
+        .body(Arrays.asList(locked.getId(), missingId))
+        .given()
+        .post(UNLOCK_URL)
+        .then()
+        .statusCode(200)
+        .extract()
+        .as(UnlockResponseDto.class);
+
+    verify(permissionService).canManageUsers(null);
+    assertTrue(response.getUnlocked().contains(locked.getId()));
+    assertTrue(response.getNotFound().contains(missingId));
+
+    User unlocked = userRepository.findById(locked.getId()).orElse(null);
+    assertNotNull(unlocked);
+    assertFalse(unlocked.isLockedOut());
+
+    UnsuccessfulAuthenticationAttempt resetCounter =
+        attemptCounterRepository.findByUserId(locked.getId()).orElse(null);
+    assertNotNull(resetCounter);
+    assertEquals(Integer.valueOf(0), resetCounter.getAttemptCounter());
+  }
+
+  @Test
+  public void shouldNotUnlockUsersWhenUserHasNoPermission() {
+    PermissionMessageException ex = buildUserManagerPermissionError();
+    doThrow(ex).when(permissionService).canManageUsers(null);
+
+    String message = startRequest(USER_TOKEN)
+        .header(CONTENT_TYPE_HEADER, APPLICATION_JSON_VALUE)
+        .body(Arrays.asList(UUID.randomUUID()))
+        .given()
+        .post(UNLOCK_URL)
+        .then()
+        .statusCode(403)
+        .extract()
+        .path(Fields.MESSAGE);
+
+    assertEquals(getMessage(ex.asMessage()), message);
+  }
+
   private ValidatableResponse passwordReset(String password, String token) {
     return passwordReset(DummyUserMainDetailsDto.USERNAME, password, token);
   }
@@ -560,4 +685,18 @@ private UserDto convertToDto(User user) {
     dto.setUsername(user.getUsername());
     return dto;
   }
+
+  private User saveLockedUser() {
+    UserDto dto = new UserDto();
+    dto.setId(UUID.randomUUID());
+    dto.setUsername("lockedUser_" + UUID.randomUUID());
+    dto.setEnabled(true);
+    dto.setLockedOut(true);
+
+    return userRepository.save(User.newInstance(dto));
+  }
+
+  private UserDto findById(UserDto[] users, UUID id) {
+    return Arrays.stream(users).filter(u -> id.equals(u.getId())).findFirst().orElse(null);
+  }
 }

src/main/java/org/openlmis/auth/dto/UnlockResponseDto.java

@@ -0,0 +1,39 @@
+/*
+ * This program is part of the OpenLMIS logistics management information system platform software.
+ * Copyright © 2017 VillageReach
+ *
+ * This program is free software: you can redistribute it and/or modify it under the terms
+ * of the GNU Affero General Public License as published by the Free Software Foundation, either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the GNU Affero General Public License for more details. You should have received a copy of
+ * the GNU Affero General Public License along with this program. If not, see
+ * http://www.gnu.org/licenses.  For additional information contact info@OpenLMIS.org.
+ */
+
+package org.openlmis.auth.dto;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.Setter;
+import lombok.ToString;
+
+@NoArgsConstructor
+@AllArgsConstructor
+@Getter
+@Setter
+@ToString
+public class UnlockResponseDto {
+
+  private List<UUID> unlocked = new ArrayList<>();
+
+  private List<UUID> notFound = new ArrayList<>();
+
+  private List<UUID> failed = new ArrayList<>();
+}

src/main/java/org/openlmis/auth/repository/UserRepository.java

@@ -15,9 +15,12 @@
 
 package org.openlmis.auth.repository;
 
+import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
+import javax.persistence.LockModeType;
 import org.openlmis.auth.domain.User;
+import org.springframework.data.jpa.repository.Lock;
 import org.springframework.data.jpa.repository.Modifying;
 import org.springframework.data.jpa.repository.Query;
 import org.springframework.data.repository.CrudRepository;
@@ -27,6 +30,11 @@ public interface UserRepository extends CrudRepository<User, UUID> {
 
   User findOneByUsernameIgnoreCase(@Param("username") String username);
 
+  // Loads a user with a pessimistic write lock, serializing admin unlock vs. authentication.
+  @Lock(LockModeType.PESSIMISTIC_WRITE)
+  @Query("SELECT u FROM User u WHERE u.id = :id")
+  Optional<User> findByIdForUpdate(@Param("id") UUID id);
+
   @Modifying
   @Query(value = "DELETE FROM auth.auth_users au "
       + "WHERE au.id IN (:userIds)",

src/main/java/org/openlmis/auth/security/OlmisAuthenticationProvider.java

@@ -26,8 +26,10 @@
 import org.springframework.security.authentication.LockedException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
+import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.userdetails.UserDetails;
+import org.springframework.transaction.annotation.Transactional;
 
 public class OlmisAuthenticationProvider extends DaoAuthenticationProvider {
 
@@ -43,10 +45,24 @@ public class OlmisAuthenticationProvider extends DaoAuthenticationProvider {
   @Autowired
   private UnsuccessfulAuthenticationAttemptRepository attemptCounterRepository;
 
+  /**
+   * Wraps the attempt in one transaction so the lock taken below is held across the whole
+   * lockout-state read-modify-write. noRollbackFor is required because a failed login throws an
+   * AuthenticationException after incrementing the counter - without it the counter (and lockout)
+   * would be rolled back. Adds locking only; does not change the lockout logic.
+   */
+  @Override
+  @Transactional(noRollbackFor = AuthenticationException.class)
+  public Authentication authenticate(Authentication authentication) throws AuthenticationException {
+    return super.authenticate(authentication);
+  }
+
   @Override
   protected void additionalAuthenticationChecks(UserDetails userDetails,
       UsernamePasswordAuthenticationToken authentication) throws AuthenticationException {
     User user = userRepository.findOneByUsernameIgnoreCase(userDetails.getUsername());
+    // pessimistic write lock to serialize with a concurrent administrative unlock of this user
+    user = userRepository.findByIdForUpdate(user.getId()).orElse(user);
     UnsuccessfulAuthenticationAttempt counter = attemptCounterRepository
         .findByUserId(user.getId())
         .orElse(new UnsuccessfulAuthenticationAttempt(user));