From e6e2781c70ffbbffe92cbaf87e8607854c3d8da1 Mon Sep 17 00:00:00 2001 From: Puja Sharma Date: Wed, 13 Apr 2022 15:43:39 +0530 Subject: [PATCH] feat(jans-config-api): user management enhancement to chk mandatory feilds --- .../rest/resource/auth/BaseResource.java | 6 ++ .../rest/resource/auth/UserResource.java | 78 ++++++++++--------- .../configapi/service/auth/UserService.java | 47 +++++------ 3 files changed, 74 insertions(+), 57 deletions(-) diff --git a/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/BaseResource.java b/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/BaseResource.java index 073c14b6885..a35e098d94f 100644 --- a/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/BaseResource.java +++ b/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/BaseResource.java @@ -52,6 +52,12 @@ public static void checkNotNull(String attribute, String attributeName) { throw new BadRequestException(getMissingAttributeError(attributeName)); } } + + public static void throwMissingAttributeError(String attributeName) { + if (StringUtils.isNotEmpty(attributeName)) { + throw new BadRequestException(getMissingAttributeError(attributeName)); + } + } public static void checkNotEmpty(List list, String attributeName) { if (list == null || list.isEmpty()) { diff --git a/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/UserResource.java b/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/UserResource.java index bd463e2fc22..d985d2722c1 100644 --- a/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/UserResource.java +++ b/jans-config-api/server/src/main/java/io/jans/configapi/rest/resource/auth/UserResource.java @@ -15,7 +15,6 @@ import io.jans.configapi.service.auth.UserService; import io.jans.configapi.util.ApiAccessConstants; import io.jans.configapi.util.ApiConstants; -import io.jans.configapi.util.AttributeNames; import io.jans.orm.model.PagedResult; import io.jans.util.StringHelper; @@ -46,7 +45,7 @@ public class UserResource extends BaseResource { @Inject UserService userSrv; - + @GET @ProtectedApi(scopes = { ApiAccessConstants.USER_READ_ACCESS }) public Response getUsers(@DefaultValue(DEFAULT_LIST_SIZE) @QueryParam(value = ApiConstants.LIMIT) int limit, @@ -56,7 +55,7 @@ public Response getUsers(@DefaultValue(DEFAULT_LIST_SIZE) @QueryParam(value = Ap @QueryParam(value = ApiConstants.SORT_ORDER) String sortOrder) throws IllegalAccessException, InvocationTargetException { if (logger.isDebugEnabled()) { - logger.debug("User search param - limit:{}, pattern:{}, startIndex:{}, sortBy:{}, sortOrder:{}", + logger.error("User search param - limit:{}, pattern:{}, startIndex:{}, sortBy:{}, sortOrder:{}", escapeLog(limit), escapeLog(pattern), escapeLog(startIndex), escapeLog(sortBy), escapeLog(sortOrder)); } @@ -64,7 +63,7 @@ public Response getUsers(@DefaultValue(DEFAULT_LIST_SIZE) @QueryParam(value = Ap limit, null, userSrv.getUserExclusionAttributesAsString()); List users = this.doSearch(searchReq); - logger.debug("User search result:{}", users); + logger.error("User search result:{}", users); return Response.ok(users).build(); } @@ -75,11 +74,11 @@ public Response getUsers(@DefaultValue(DEFAULT_LIST_SIZE) @QueryParam(value = Ap public Response getUserByInum(@PathParam(ApiConstants.INUM) @NotNull String inum) throws IllegalAccessException, InvocationTargetException { if (logger.isDebugEnabled()) { - logger.debug("User search by inum:{}", escapeLog(inum)); + logger.error("User search by inum:{}", escapeLog(inum)); } User user = userSrv.getUserBasedOnInum(inum); checkResourceNotNull(user, USER); - logger.debug("user:{}", user); + logger.error("user:{}", user); // excludedAttributes user = excludeUserAttributes(user); @@ -89,30 +88,38 @@ public Response getUserByInum(@PathParam(ApiConstants.INUM) @NotNull String inum @POST @ProtectedApi(scopes = { ApiAccessConstants.USER_WRITE_ACCESS }) - public Response createUser(@Valid User user) throws IllegalAccessException, InvocationTargetException { + public Response createUser(@Valid User user) + throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { if (logger.isDebugEnabled()) { - logger.debug("User details to be added - user:{}", escapeLog(user)); + logger.error("User details to be added - user:{}", escapeLog(user)); } + + // checking mandatory attributes + checkMissingAttributes(user); + user = userSrv.addUser(user, true); - logger.debug("User created {}", user); - - - + logger.error("User created {}", user); + // excludedAttributes user = excludeUserAttributes(user); - + return Response.status(Response.Status.CREATED).entity(user).build(); } @PUT @ProtectedApi(scopes = { ApiAccessConstants.USER_WRITE_ACCESS }) - public Response updateUser(@Valid User user) throws IllegalAccessException, InvocationTargetException { + public Response updateUser(@Valid User user) + throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { if (logger.isDebugEnabled()) { - logger.debug("User details to be updated - user:{}", escapeLog(user)); + logger.error("User details to be updated - user:{}", escapeLog(user)); } + + // checking mandatory attributes + checkMissingAttributes(user); + user = userSrv.updateUser((user)); - logger.debug("Updated user:{}", user); - + logger.error("Updated user:{}", user); + // excludedAttributes user = excludeUserAttributes(user); @@ -123,9 +130,10 @@ public Response updateUser(@Valid User user) throws IllegalAccessException, Invo @ProtectedApi(scopes = { ApiAccessConstants.USER_WRITE_ACCESS }) @Path(ApiConstants.INUM_PATH) public Response patchUser(@PathParam(ApiConstants.INUM) @NotNull String inum, - @NotNull UserPatchRequest userPatchRequest) throws IllegalAccessException, InvocationTargetException, JsonPatchException, IOException { + @NotNull UserPatchRequest userPatchRequest) + throws IllegalAccessException, InvocationTargetException, JsonPatchException, IOException { if (logger.isDebugEnabled()) { - logger.debug("User:{} to be patched with :{} ", escapeLog(inum), escapeLog(userPatchRequest)); + logger.error("User:{} to be patched with :{} ", escapeLog(inum), escapeLog(userPatchRequest)); } // check if user exists User existingUser = userSrv.getUserBasedOnInum(inum); @@ -133,8 +141,8 @@ public Response patchUser(@PathParam(ApiConstants.INUM) @NotNull String inum, // patch user existingUser = userSrv.patchUser(inum, userPatchRequest); - logger.debug("Patched user:{}", existingUser); - + logger.error("Patched user:{}", existingUser); + // excludedAttributes existingUser = excludeUserAttributes(existingUser); @@ -146,7 +154,7 @@ public Response patchUser(@PathParam(ApiConstants.INUM) @NotNull String inum, @ProtectedApi(scopes = { ApiAccessConstants.USER_DELETE_ACCESS }) public Response deleteUser(@PathParam(ApiConstants.INUM) @NotNull String inum) { if (logger.isDebugEnabled()) { - logger.debug("User to be deleted - inum:{} ", escapeLog(inum)); + logger.error("User to be deleted - inum:{} ", escapeLog(inum)); } User user = userSrv.getUserBasedOnInum(inum); checkResourceNotNull(user, USER); @@ -156,21 +164,21 @@ public Response deleteUser(@PathParam(ApiConstants.INUM) @NotNull String inum) { private List doSearch(SearchRequest searchReq) throws IllegalAccessException, InvocationTargetException { if (logger.isDebugEnabled()) { - logger.debug("User search params - searchReq:{} ", escapeLog(searchReq)); + logger.error("User search params - searchReq:{} ", escapeLog(searchReq)); } PagedResult pagedResult = userSrv.searchUsers(searchReq); if (logger.isTraceEnabled()) { - logger.trace("PagedResult - pagedResult:{}", pagedResult); + logger.error("PagedResult - pagedResult:{}", pagedResult); } List users = new ArrayList<>(); if (pagedResult != null) { - logger.trace("Users fetched - pagedResult.getEntries():{}", pagedResult.getEntries()); + logger.error("Users fetched - pagedResult.getEntries():{}", pagedResult.getEntries()); users = pagedResult.getEntries(); } if (logger.isDebugEnabled()) { - logger.debug("Users fetched - users:{}", users); + logger.error("Users fetched - users:{}", users); } // excludedAttributes @@ -178,20 +186,20 @@ private List doSearch(SearchRequest searchReq) throws IllegalAccessExcepti return users; } - + private User excludeUserAttributes(User user) throws IllegalAccessException, InvocationTargetException { return userSrv.excludeAttributes(user, userSrv.getUserExclusionAttributesAsString()); } - - - private void checkMissingAttributes(User user) throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { + + private void checkMissingAttributes(User user) + throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { String missingAttributes = userSrv.checkMandatoryFields(user); - - if(StringHelper.isEmpty(missingAttributes)) { + logger.error("missingAttributes:{}", missingAttributes); + + if (StringHelper.isEmpty(missingAttributes)) { return; } - - checkNotNull(missingAttributes, "Mandatory User Attributes"); + + throwMissingAttributeError(missingAttributes); } - } diff --git a/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/UserService.java b/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/UserService.java index 58263e9be61..2353faf3bf5 100644 --- a/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/UserService.java +++ b/jans-config-api/server/src/main/java/io/jans/configapi/service/auth/UserService.java @@ -103,12 +103,12 @@ public User patchUser(String inum, UserPatchRequest userPatchRequest) throws Jso return null; } - logger.debug("User to be patched- user:{}", user); + logger.error("User to be patched- user:{}", user); // apply direct patch for basic attributes if (StringUtils.isNotEmpty(userPatchRequest.getJsonPatchString())) { - logger.debug("Patch basic attributes"); + logger.error("Patch basic attributes"); user = Jackson.applyPatch(userPatchRequest.getJsonPatchString(), user); - logger.debug("User after patching basic attributes - user:{}", user); + logger.error("User after patching basic attributes - user:{}", user); } // patch for customAttributes @@ -116,11 +116,11 @@ public User patchUser(String inum, UserPatchRequest userPatchRequest) throws Jso updateCustomAttributes(user, userPatchRequest.getCustomAttributes()); } - logger.debug("User before patch user:{}", user); + logger.error("User before patch user:{}", user); // persist user user = updateUser(user); - logger.debug("User after patch user:{}", user); + logger.error("User after patch user:{}", user); return user; } @@ -130,24 +130,24 @@ public User getUserBasedOnInum(String inum) { try { result = getUserByInum(inum); } catch (Exception ex) { - logger.debug("Failed to load user entry", ex); + logger.error("Failed to load user entry", ex); } return result; } private User updateCustomAttributes(User user, List customAttributes) { - logger.debug("Custom Attributes to update for - user:{}, customAttributes:{} ", user, customAttributes); + logger.error("Custom Attributes to update for - user:{}, customAttributes:{} ", user, customAttributes); if (customAttributes != null && !customAttributes.isEmpty()) { for (CustomObjectAttribute attribute : customAttributes) { CustomObjectAttribute existingAttribute = getCustomAttribute(user, attribute.getName()); - logger.debug("Existing CustomAttributes with existingAttribute:{} ", existingAttribute); + logger.error("Existing CustomAttributes with existingAttribute:{} ", existingAttribute); // add if (existingAttribute == null) { boolean result = addUserAttribute(user, attribute.getName(), attribute.getValues(), attribute.isMultiValued()); - logger.debug("Result of adding CustomAttributes attribute:{} , result:{} ", attribute, result); + logger.error("Result of adding CustomAttributes attribute:{} , result:{} ", attribute, result); } // remove attribute else if (attribute.getValue() == null || attribute.getValues() == null) { @@ -160,7 +160,7 @@ else if (attribute.getValue() == null || attribute.getValues() == null) { existingAttribute.setValues(attribute.getValues()); } // Final attribute - logger.debug("Finally user CustomAttributes user.getCustomAttributes:{} ", user.getCustomAttributes()); + logger.error("Finally user CustomAttributes user.getCustomAttributes:{} ", user.getCustomAttributes()); } } @@ -170,43 +170,43 @@ else if (attribute.getValue() == null || attribute.getValues() == null) { public List excludeAttributes(List users, String commaSeparatedString) throws IllegalAccessException, InvocationTargetException { - logger.debug("Attributes:{} to be excluded from users:{} ", commaSeparatedString, users); + logger.error("Attributes:{} to be excluded from users:{} ", commaSeparatedString, users); for (User user : users) { excludeAttributes(user, commaSeparatedString); } - logger.debug("Users:{} after excluding attribute:{} ", users, commaSeparatedString); + logger.error("Users:{} after excluding attribute:{} ", users, commaSeparatedString); return users; } public User excludeAttributes(User user, String commaSeparatedString) throws IllegalAccessException, InvocationTargetException { - logger.debug("Attributes:{} to be excluded from user:{} ", commaSeparatedString, user); + logger.error("Attributes:{} to be excluded from user:{} ", commaSeparatedString, user); if (user == null || StringUtils.isEmpty(commaSeparatedString)) { return user; } List excludedAttributes = Arrays.asList(commaSeparatedString.split(",")); - logger.debug("Attributes List:{} to be excluded ", excludedAttributes); + logger.error("Attributes List:{} to be excluded ", excludedAttributes); List allFields = authUtil.getAllFields(user.getClass()); - logger.debug("All user fields :{} ", allFields); + logger.error("All user fields :{} ", allFields); HashMap map = new HashMap<>(); for (String attribute : excludedAttributes) { - logger.debug("User class allFields:{} conatins attribute:{} ? :{} ", allFields, attribute, + logger.error("User class allFields:{} conatins attribute:{} ? :{} ", allFields, attribute, authUtil.containsField(allFields, attribute)); if (authUtil.containsField(allFields, attribute)) { - logger.debug("User class contains attribute:{} ! ", attribute); + logger.error("User class contains attribute:{} ! ", attribute); map.put(attribute, null); } else { - logger.debug("Removing custom attribute:{} from user:{} ", attribute, user); + logger.error("Removing custom attribute:{} from user:{} ", attribute, user); user.removeAttribute(attribute); } } - logger.debug("Attributes map:{} to be excluded ", map); + logger.error("Attributes map:{} to be excluded ", map); if (!map.isEmpty()) { - logger.debug("Removing simple attributes:{} from user object ", map); + logger.error("Removing simple attributes:{} from user object ", map); BeanUtilsBean.getInstance().getConvertUtils().register(false, false, 0); BeanUtils.populate(user, map); } @@ -245,12 +245,15 @@ public String checkMandatoryFields(User user) attributeValue = user.getAttribute(attribute); logger.error("User custom attribute:{} - attributeValue:{} ", attribute, attributeValue); } - - if(attributeValue == null) { + + if (attributeValue == null) { missingAttributes.append(attribute).append(","); } } logger.error("Checking mandatory missingAttributes:{} ", missingAttributes); + missingAttributes.replace(missingAttributes.lastIndexOf(","), missingAttributes.length(), ""); + + logger.error("Returning missingAttributes:{} ", missingAttributes); return missingAttributes.toString(); }