This is an automated email from the ASF dual-hosted git repository.
mosi pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new d9a0a9a FINERACT-803: validate username uniqueness
new fb1e3d7 Merge pull request #660 from jamesidw/FINERACT-803
d9a0a9a is described below
commit d9a0a9ac63a7ebe8e4ca24a71fac840982f0d494
Author: Sidney Wasibani <wasib...@kanzucode.com>
AuthorDate: Wed Nov 27 04:15:31 2019 +0300
FINERACT-803: validate username uniqueness
FINERACT-803: treat existing username in update, to same user, as valid
FINERACT-803: skip username uniqueness validation for updates
FINERACT-803: add integration test for username uniqueness validation
FINERACT-803: change the validation error message to show the errant
username
FINERACT-803: restore username uniqueness validation for updates
FINERACT-803: apply minor code clean up suggestions
---
.../integrationtests/UserAdministrationTest.java | 122 +++++++++++++++++++++
.../useradministration/users/UserHelper.java | 29 ++++-
.../core/data/DataValidatorBuilder.java | 6 +
...pUserWritePlatformServiceJpaRepositoryImpl.java | 2 +-
.../service/UserDataValidator.java | 22 +++-
5 files changed, 173 insertions(+), 8 deletions(-)
diff --git
a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/UserAdministrationTest.java
b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/UserAdministrationTest.java
new file mode 100644
index 0000000..0985ee1
--- /dev/null
+++
b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/UserAdministrationTest.java
@@ -0,0 +1,122 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fineract.integrationtests;
+
+import com.jayway.restassured.builder.RequestSpecBuilder;
+import com.jayway.restassured.builder.ResponseSpecBuilder;
+import com.jayway.restassured.http.ContentType;
+import com.jayway.restassured.specification.RequestSpecification;
+import com.jayway.restassured.specification.ResponseSpecification;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import org.apache.fineract.integrationtests.common.Utils;
+import org.apache.fineract.integrationtests.common.organisation.StaffHelper;
+import
org.apache.fineract.integrationtests.useradministration.roles.RolesHelper;
+import
org.apache.fineract.integrationtests.useradministration.users.UserHelper;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class UserAdministrationTest {
+
+ private ResponseSpecification responseSpec;
+ private RequestSpecification requestSpec;
+ private List<Integer> transientUsers = new ArrayList<>();
+
+ private ResponseSpecification expectStatusCode(int code) {
+ return new ResponseSpecBuilder().expectStatusCode(code).build();
+ }
+
+ @Before
+ public void setup() {
+ Utils.initializeRESTAssured();
+ this.requestSpec = new
RequestSpecBuilder().setContentType(ContentType.JSON).build();
+ this.requestSpec.header("Authorization", "Basic " +
Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
+ this.responseSpec = expectStatusCode(200);
+ }
+
+ @After
+ public void tearDown() {
+ for(Integer userId : this.transientUsers) {
+ UserHelper.deleteUser(this.requestSpec, this.responseSpec, userId);
+ }
+ this.transientUsers.clear();
+ }
+
+ @Test
+ public void testCreateNewUserBlocksDuplicateUsername() {
+ final Integer roleId = RolesHelper.createRole(this.requestSpec,
this.responseSpec);
+ Assert.assertNotNull(roleId);
+
+ final Integer staffId = StaffHelper.createStaff(this.requestSpec,
this.responseSpec);
+ Assert.assertNotNull(staffId);
+
+ final Integer userId = (Integer)
UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId,
"alphabet", "resourceId");
+ Assert.assertNotNull(userId);
+ this.transientUsers.add(userId);
+
+ final List errors = (List) UserHelper.createUser(this.requestSpec,
expectStatusCode(400), roleId, staffId, "alphabet", "errors");
+ Map reason = (Map) errors.get(0);
+ Assert.assertEquals("User with username 'alphabet' already exists.",
reason.get("defaultUserMessage"));
+ }
+
+ @Test
+ public void testUpdateUserAcceptsNewOrSameUsername() {
+ final Integer roleId = RolesHelper.createRole(this.requestSpec,
this.responseSpec);
+ Assert.assertNotNull(roleId);
+
+ final Integer staffId = StaffHelper.createStaff(this.requestSpec,
this.responseSpec);
+ Assert.assertNotNull(staffId);
+
+ final Integer userId = (Integer)
UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId,
"alphabet", "resourceId");
+ Assert.assertNotNull(userId);
+ this.transientUsers.add(userId);
+
+ final Integer userId2 = (Integer)
UserHelper.updateUser(this.requestSpec, this.responseSpec, userId, "renegade",
"resourceId");
+ Assert.assertNotNull(userId2);
+
+ final Integer userId3 = (Integer)
UserHelper.updateUser(this.requestSpec, this.responseSpec, userId, "renegade",
"resourceId");
+ Assert.assertNotNull(userId3);
+ }
+
+ @Test
+ public void testUpdateUserBlockDuplicateUsername() {
+ final Integer roleId = RolesHelper.createRole(this.requestSpec,
this.responseSpec);
+ Assert.assertNotNull(roleId);
+
+ final Integer staffId = StaffHelper.createStaff(this.requestSpec,
this.responseSpec);
+ Assert.assertNotNull(staffId);
+
+ final Integer userId = (Integer)
UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId,
"alphabet", "resourceId");
+ Assert.assertNotNull(userId);
+ this.transientUsers.add(userId);
+
+ final Integer userId2 = (Integer)
UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId,
"bilingual", "resourceId");
+ Assert.assertNotNull(userId2);
+ this.transientUsers.add(userId2);
+
+ final List errors = (List) UserHelper.updateUser(this.requestSpec,
expectStatusCode(400), userId2, "alphabet", "errors");
+ Map reason = (Map) errors.get(0);
+ Assert.assertEquals("User with username 'alphabet' already exists.",
reason.get("defaultUserMessage"));
+ }
+
+}
diff --git
a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java
b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java
index 8acabda..ea5b4c8 100644
---
a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java
+++
b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java
@@ -32,21 +32,40 @@ public class UserHelper {
return Utils.performServerPost(requestSpec, responseSpec,
CREATE_USER_URL, getTestCreateUserAsJSON(roleId, staffId), "resourceId");
}
+ public static Object createUser(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec, int roleId, int staffId, String
username, String attribute) {
+ return Utils.performServerPost(requestSpec, responseSpec,
CREATE_USER_URL, getTestCreateUserAsJSON(roleId, staffId, username), attribute);
+ }
+
public static String getTestCreateUserAsJSON(int roleId, int staffId) {
- String json = "{ \"username\": \"" +
Utils.randomNameGenerator("User_Name_", 3)
+ return "{ \"username\": \"" + Utils.randomNameGenerator("User_Name_",
3)
+ "\", \"firstname\": \"Test\", \"lastname\": \"User\",
\"email\": \"whate...@mifos.org\","
+ " \"officeId\": \"1\", \"staffId\": " + "\""
- + Integer.toString(staffId)+"\",\"roles\": [\""
- + Integer.toString(roleId) + "\"], \"sendPasswordToEmail\":
false}";
- System.out.println(json);
- return json;
+ + staffId +"\",\"roles\": [\""
+ + roleId + "\"], \"sendPasswordToEmail\": false}";
+ }
+ private static String getTestCreateUserAsJSON(int roleId, int staffId,
String username) {
+ return "{ \"username\": \"" + username
+ + "\", \"firstname\": \"Test\", \"lastname\": \"User\", \"email\":
\"whate...@mifos.org\","
+ + " \"officeId\": \"1\", \"staffId\": " + "\""
+ + staffId +"\",\"roles\": [\""
+ + roleId + "\"], \"sendPasswordToEmail\": false}";
+ }
+
+ private static String getTestUpdateUserAsJSON(String username) {
+ return "{ \"username\": \"" + username
+ + "\", \"firstname\": \"Test\", \"lastname\": \"User\", \"email\":
\"whate...@mifos.org\","
+ + " \"officeId\": \"1\"}";
}
public static Integer deleteUser(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec, final Integer userId) {
return Utils.performServerDelete(requestSpec, responseSpec,
createRoleOperationURL(userId), "resourceId");
}
+ public static Object updateUser(final RequestSpecification requestSpec,
final ResponseSpecification responseSpec, int userId, String username, String
attribute) {
+ return Utils.performServerPut(requestSpec, responseSpec,
createRoleOperationURL(userId), getTestUpdateUserAsJSON(username), attribute);
+ }
+
private static String createRoleOperationURL(final Integer userId) {
return USER_URL + "/" + userId + "?" + Utils.TENANT_IDENTIFIER;
}
diff --git
a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java
b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java
index 8191cbc..8824e28 100644
---
a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java
+++
b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java
@@ -104,6 +104,12 @@ public class DataValidatorBuilder {
return this;
}
+ public void failWithMessage(final String errorCode, final String
errorMessage, final Object... defaultUserMessageArgs) {
+ String validationErrorCode = "validation.msg." + this.resource + "." +
this.parameter + "." + errorCode;
+ final ApiParameterError error =
ApiParameterError.parameterError(validationErrorCode, errorMessage,
this.parameter, this.value, defaultUserMessageArgs);
+ this.dataValidationErrors.add(error);
+ }
+
public void failWithCode(final String errorCode, final Object...
defaultUserMessageArgs) {
final StringBuilder validationErrorCode = new
StringBuilder("validation.msg.").append(this.resource).append(".")
.append(this.parameter).append(".").append(errorCode);
diff --git
a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java
b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java
index 15fa769..0176c27 100644
---
a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java
+++
b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java
@@ -192,7 +192,7 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl
implements AppUserWrit
this.context.authenticatedUser(new
CommandWrapperBuilder().updateUser(null).build());
- this.fromApiJsonDeserializer.validateForUpdate(command.json());
+ this.fromApiJsonDeserializer.validateForUpdate(command.json(),
userId);
final AppUser userToUpdate =
this.appUserRepository.findById(userId)
.orElseThrow(() -> new UserNotFoundException(userId));
diff --git
a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java
b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java
index 57a72c7..58f5bb9 100644
---
a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java
+++
b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java
@@ -32,6 +32,8 @@ import
org.apache.fineract.infrastructure.core.data.DataValidatorBuilder;
import org.apache.fineract.infrastructure.core.exception.InvalidJsonException;
import
org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException;
import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper;
+import org.apache.fineract.useradministration.domain.AppUser;
+import org.apache.fineract.useradministration.domain.AppUserRepository;
import org.apache.fineract.useradministration.domain.PasswordValidationPolicy;
import
org.apache.fineract.useradministration.domain.PasswordValidationPolicyRepository;
import org.springframework.beans.factory.annotation.Autowired;
@@ -55,10 +57,13 @@ public final class UserDataValidator {
private final PasswordValidationPolicyRepository passwordValidationPolicy;
+ private final AppUserRepository appUserRepository;
+
@Autowired
- public UserDataValidator(final FromJsonHelper fromApiJsonHelper, final
PasswordValidationPolicyRepository passwordValidationPolicy) {
+ public UserDataValidator(final FromJsonHelper fromApiJsonHelper, final
PasswordValidationPolicyRepository passwordValidationPolicy, AppUserRepository
appUserRepository) {
this.fromApiJsonHelper = fromApiJsonHelper;
this.passwordValidationPolicy = passwordValidationPolicy;
+ this.appUserRepository = appUserRepository;
}
public void validateForCreate(final String json) {
@@ -75,6 +80,14 @@ public final class UserDataValidator {
final String username =
this.fromApiJsonHelper.extractStringNamed("username", element);
baseDataValidator.reset().parameter("username").value(username).notBlank().notExceedingLengthOf(100);
+ if (!StringUtils.isEmpty(username)) {
+ AppUser exists = appUserRepository.findAppUserByName(username);
+ if (exists != null) {
+ final String errorMessage = "User with username '" + username
+ "' already exists.";
+
baseDataValidator.reset().parameter("username").failWithMessage("constraint.violation.duplicate.username",
errorMessage);
+ }
+ }
+
final String firstname =
this.fromApiJsonHelper.extractStringNamed("firstname", element);
baseDataValidator.reset().parameter("firstname").value(firstname).notBlank().notExceedingLengthOf(100);
@@ -149,7 +162,7 @@ public final class UserDataValidator {
if (!dataValidationErrors.isEmpty()) { throw new
PlatformApiDataValidationException(dataValidationErrors); }
}
- public void validateForUpdate(final String json) {
+ public void validateForUpdate(final String json, final Long userId) {
if (StringUtils.isBlank(json)) { throw new InvalidJsonException(); }
final Type typeOfMap = new TypeToken<Map<String, Object>>()
{}.getType();
@@ -173,6 +186,11 @@ public final class UserDataValidator {
if (this.fromApiJsonHelper.parameterExists("username", element)) {
final String username =
this.fromApiJsonHelper.extractStringNamed("username", element);
baseDataValidator.reset().parameter("username").value(username).notBlank().notExceedingLengthOf(100);
+ AppUser current = appUserRepository.findAppUserByName(username);
+ if (current != null && !current.hasIdOf(userId)) {
+ final String errorMessage = "User with username '" + username
+ "' already exists.";
+
baseDataValidator.reset().parameter("username").failWithMessage("constraint.violation.duplicate.username",
errorMessage);
+ }
}
if (this.fromApiJsonHelper.parameterExists("firstname", element)) {
