From 87f20105767a99dc28d4afdbb9b5a6fa41605edf Mon Sep 17 00:00:00 2001
From: Rick Herrick <jrherrick@wustl.edu>
Date: Wed, 7 Sep 2016 18:16:49 -0500
Subject: [PATCH] XNAT-2840 One more stab at securing user wrappers while
keeping them functional.
---
.../java/org/nrg/xapi/model/users/User.java | 27 ++++--
.../org/nrg/xapi/rest/users/UsersApi.java | 23 ++---
.../model/users/TestUserSerialization.java | 96 +++++++++++++------
3 files changed, 93 insertions(+), 53 deletions(-)
diff --git a/src/main/java/org/nrg/xapi/model/users/User.java b/src/main/java/org/nrg/xapi/model/users/User.java
index 9e0f751e..8b774c1d 100644
--- a/src/main/java/org/nrg/xapi/model/users/User.java
+++ b/src/main/java/org/nrg/xapi/model/users/User.java
@@ -1,7 +1,6 @@
package org.nrg.xapi.model.users;
import com.fasterxml.jackson.annotation.JsonIgnore;
-import com.fasterxml.jackson.annotation.JsonProperty;
import io.swagger.annotations.ApiModel;
import io.swagger.annotations.ApiModelProperty;
import org.nrg.xdat.entities.UserAuthI;
@@ -11,6 +10,13 @@ import org.nrg.xft.security.UserI;
import java.util.Date;
+/**
+ * A transport container for user details. The {@link #isSecured() secured property} controls whether security-related
+ * properties like password and salt are available. When a new user object is created through one of the wrapper
+ * constructors, such as {@link #User(String)} or {@link #User(UserI)}, secure is set to true. This means that
+ * serializing beans with existing user accounts won't expose the password data. Newly created beans have secure set to
+ * false by default to allow for serializing the bean for REST calls with all data intact.
+ */
@ApiModel(description = "Contains the properties that define a user on the system.")
public class User {
public User() {
@@ -35,6 +41,7 @@ public class User {
_authorization = user.getAuthorization();
_isEnabled = user.isEnabled();
_isVerified = user.isVerified();
+ _secured = true;
}
public User(final XdatUser user) {
@@ -143,9 +150,8 @@ public class User {
* The user's encrypted password.
**/
@ApiModelProperty(value = "The user's encrypted password.")
- @JsonProperty(access = JsonProperty.Access.WRITE_ONLY)
public String getPassword() {
- return _password;
+ return _secured ? null : _password;
}
public void setPassword(String password) {
@@ -156,9 +162,8 @@ public class User {
* The _salt used to encrypt the user's _password.
**/
@ApiModelProperty(value = "The salt used to encrypt the user's password.")
- @JsonProperty(access = JsonProperty.Access.WRITE_ONLY)
public String getSalt() {
- return _salt;
+ return _secured ? null : _salt;
}
@SuppressWarnings("unused")
@@ -183,7 +188,7 @@ public class User {
**/
@ApiModelProperty(value = "The user's authorization record used when logging in.")
public UserAuthI getAuthorization() {
- return _authorization;
+ return _secured ? null : _authorization;
}
@SuppressWarnings("unused")
@@ -197,6 +202,15 @@ public class User {
return String.format("%s %s", getFirstName(), getLastName());
}
+ @ApiModelProperty(value = "Indicates whether the user object is secured, which causes secure fields like password and salt to return null.")
+ public boolean isSecured() {
+ return _secured;
+ }
+
+ public void setSecured(final boolean secured) {
+ _secured = secured;
+ }
+
@Override
public String toString() {
return "class User {\n" +
@@ -221,6 +235,7 @@ public class User {
private String _dbName;
private String _password;
private String _salt;
+ private boolean _secured;
private Date _lastModified;
private UserAuthI _authorization;
private Boolean _isAdmin;
diff --git a/src/main/java/org/nrg/xapi/rest/users/UsersApi.java b/src/main/java/org/nrg/xapi/rest/users/UsersApi.java
index 3e687d89..e3f1c8b0 100644
--- a/src/main/java/org/nrg/xapi/rest/users/UsersApi.java
+++ b/src/main/java/org/nrg/xapi/rest/users/UsersApi.java
@@ -78,34 +78,25 @@ public class UsersApi extends AbstractXapiRestController {
@ApiResponse(code = 500, message = "An unexpected error occurred.")})
@RequestMapping(value = "profiles", produces = MediaType.APPLICATION_JSON_VALUE, method = RequestMethod.GET)
@ResponseBody
- public ResponseEntity<List<Map<String, String>>> usersProfilesGet() {
+ public ResponseEntity<List<User>> usersProfilesGet() {
if (_preferences.getRestrictUserListAccessToAdmins()) {
final HttpStatus status = isPermitted();
if (status != null) {
return new ResponseEntity<>(status);
}
}
- List<UserI> users = getUserManagementService().getUsers();
- List<Map<String, String>> userMaps = null;
+ final List<UserI> users = getUserManagementService().getUsers();
+ final List<User> beans = new ArrayList<>();
if (users != null && users.size() > 0) {
- userMaps = new ArrayList<>();
for (UserI user : users) {
try {
- Map<String, String> userMap = new HashMap<>();
- userMap.put("firstname", user.getFirstname());
- userMap.put("lastname", user.getLastname());
- userMap.put("username", user.getUsername());
- userMap.put("email", user.getEmail());
- userMap.put("id", String.valueOf(user.getID()));
- userMap.put("enabled", String.valueOf(user.isEnabled()));
- userMap.put("verified", String.valueOf(user.isVerified()));
- userMaps.add(userMap);
+ beans.add(new User(user));
} catch (Exception e) {
_log.error("", e);
}
}
}
- return new ResponseEntity<>(userMaps, HttpStatus.OK);
+ return new ResponseEntity<>(beans, HttpStatus.OK);
}
@ApiOperation(value = "Get list of active users.", notes = "Returns a map of usernames for users that have at least one currently active session, i.e. logged in or associated with a valid application session. The number of active sessions and a list of the session IDs is associated with each user.", response = Map.class, responseContainer = "Map")
@@ -180,7 +171,7 @@ public class UsersApi extends AbstractXapiRestController {
@ApiResponse(code = 404, message = "User not found."),
@ApiResponse(code = 500, message = "An unexpected error occurred.")})
@RequestMapping(value = "{username}", produces = MediaType.APPLICATION_JSON_VALUE, method = RequestMethod.GET)
- public ResponseEntity<User> usersIdGet(@ApiParam(value = "ID of the user to fetch", required = true) @PathVariable("username") final String username) {
+ public ResponseEntity<User> getUser(@ApiParam(value = "Username of the user to fetch.", required = true) @PathVariable("username") final String username) {
HttpStatus status = isPermitted(username);
if (status != null) {
return new ResponseEntity<>(status);
@@ -225,7 +216,7 @@ public class UsersApi extends AbstractXapiRestController {
if (model.isEnabled() != null) {
user.setEnabled(model.isEnabled());
}
- if (model.isVerified()) {
+ if (model.isVerified() != null) {
user.setVerified(model.isVerified());
}
user.setPassword(model.getPassword());
diff --git a/src/test/java/org/nrg/xapi/model/users/TestUserSerialization.java b/src/test/java/org/nrg/xapi/model/users/TestUserSerialization.java
index 8ac87ebd..86bc2e28 100644
--- a/src/test/java/org/nrg/xapi/model/users/TestUserSerialization.java
+++ b/src/test/java/org/nrg/xapi/model/users/TestUserSerialization.java
@@ -22,39 +22,73 @@ public class TestUserSerialization {
}
@Test
- public void testHiddenProperties() throws IOException {
- final User input = new User();
- input.setUsername("name");
- input.setEmail("foo@bar.com");
- input.setPassword("password");
- input.setSalt("salt");
- input.setAdmin(false);
- input.setEnabled(true);
-
- final String json = _serializer.toJson(input);
- assertNotNull(json);
- assertTrue(StringUtils.isNotBlank(json));
+ public void testSecuredProperties() throws IOException {
+ final User secured = new User();
+ secured.setUsername("name");
+ secured.setEmail("foo@bar.com");
+ secured.setPassword("password");
+ secured.setSalt("salt");
+ secured.setAdmin(false);
+ secured.setEnabled(true);
+ secured.setSecured(true);
+
+ // Users are unsecured by default when created by default constructor.
+ final User unsecured = new User();
+ unsecured.setUsername("name");
+ unsecured.setEmail("foo@bar.com");
+ unsecured.setPassword("password");
+ unsecured.setSalt("salt");
+ unsecured.setAdmin(false);
+ unsecured.setEnabled(true);
+
+ final String secureJson = _serializer.toJson(secured);
+ assertNotNull(secureJson);
+ assertTrue(StringUtils.isNotBlank(secureJson));
+ final String unsecureJson = _serializer.toJson(unsecured);
+ assertNotNull(unsecureJson);
+ assertTrue(StringUtils.isNotBlank(unsecureJson));
// Here's where we make sure the password and salt aren't serialized.
- final JsonNode map = _serializer.deserializeJson(json);
- assertNotNull(map);
- assertTrue(map.has("username"));
- assertTrue(map.has("email"));
- assertFalse(map.has("password"));
- assertFalse(map.has("salt"));
- assertTrue(map.has("admin"));
- assertTrue(map.has("enabled"));
- assertFalse(map.has("verified"));
-
- final User output = _serializer.deserializeJson(json, User.class);
- assertNotNull(output);
- assertTrue(StringUtils.isNotBlank(output.getUsername()));
- assertTrue(StringUtils.isNotBlank(output.getEmail()));
- assertTrue(StringUtils.isBlank(output.getPassword()));
- assertTrue(StringUtils.isBlank(output.getSalt()));
- assertFalse(output.isAdmin());
- assertTrue(output.isEnabled());
- assertNull(output.isVerified());
+ final JsonNode securedMap = _serializer.deserializeJson(secureJson);
+ assertNotNull(securedMap);
+ assertTrue(securedMap.has("username"));
+ assertTrue(securedMap.has("email"));
+ assertFalse(securedMap.has("password"));
+ assertFalse(securedMap.has("salt"));
+ assertTrue(securedMap.has("admin"));
+ assertTrue(securedMap.has("enabled"));
+ assertFalse(securedMap.has("verified"));
+
+ // Here's where we make sure the password and salt ARE serialized.
+ final JsonNode unsecuredMap = _serializer.deserializeJson(unsecureJson);
+ assertNotNull(unsecuredMap);
+ assertTrue(unsecuredMap.has("username"));
+ assertTrue(unsecuredMap.has("email"));
+ assertTrue(unsecuredMap.has("password"));
+ assertTrue(unsecuredMap.has("salt"));
+ assertTrue(unsecuredMap.has("admin"));
+ assertTrue(unsecuredMap.has("enabled"));
+ assertFalse(unsecuredMap.has("verified"));
+
+ final User securedOutput = _serializer.deserializeJson(secureJson, User.class);
+ assertNotNull(securedOutput);
+ assertTrue(StringUtils.isNotBlank(securedOutput.getUsername()));
+ assertTrue(StringUtils.isNotBlank(securedOutput.getEmail()));
+ assertTrue(StringUtils.isBlank(securedOutput.getPassword()));
+ assertTrue(StringUtils.isBlank(securedOutput.getSalt()));
+ assertFalse(securedOutput.isAdmin());
+ assertTrue(securedOutput.isEnabled());
+ assertNull(securedOutput.isVerified());
+
+ final User unsecuredOutput = _serializer.deserializeJson(unsecureJson, User.class);
+ assertNotNull(unsecuredOutput);
+ assertTrue(StringUtils.isNotBlank(unsecuredOutput.getUsername()));
+ assertTrue(StringUtils.isNotBlank(unsecuredOutput.getEmail()));
+ assertTrue(StringUtils.isNotBlank(unsecuredOutput.getPassword()));
+ assertTrue(StringUtils.isNotBlank(unsecuredOutput.getSalt()));
+ assertFalse(unsecuredOutput.isAdmin());
+ assertTrue(unsecuredOutput.isEnabled());
+ assertNull(unsecuredOutput.isVerified());
}
private SerializerService _serializer;
--
GitLab