From 4fab4e8df0defce8338b48eda0349a15724d1d4e Mon Sep 17 00:00:00 2001 From: Vitaly Takmazov Date: Tue, 21 Aug 2018 12:34:56 +0300 Subject: User refactoring --- juick-common/src/main/java/com/juick/User.java | 10 ---- .../com/juick/server/helpers/AnonymousUser.java | 10 ---- .../main/java/com/juick/service/UserService.java | 4 -- .../security/HashParamAuthenticationFilter.java | 2 +- .../service/security/JuickUserDetailsService.java | 2 +- .../CookieSimpleHashRememberMeServices.java | 2 +- .../RequestParamHashRememberMeServices.java | 2 +- .../java/com/juick/service/UserServiceImpl.java | 63 ++++------------------ .../java/com/juick/server/tests/ServerTests.java | 30 +++++++++++ 9 files changed, 45 insertions(+), 80 deletions(-) diff --git a/juick-common/src/main/java/com/juick/User.java b/juick-common/src/main/java/com/juick/User.java index b81739a2..7aa92469 100644 --- a/juick-common/src/main/java/com/juick/User.java +++ b/juick-common/src/main/java/com/juick/User.java @@ -44,7 +44,6 @@ public class User { private String authHash; private boolean banned; private String credentials; - private String lang; private List tokens; private List read; private List readers; @@ -72,7 +71,6 @@ public class User { .append("name", name) .append("fullName", fullName) .append("messagesCount", messagesCount) - .append("lang", lang) .append("banned", banned) .toString(); } @@ -154,14 +152,6 @@ public class User { this.credentials = credentials; } - public String getLang() { - return lang; - } - - public void setLang(String lang) { - this.lang = lang; - } - @XmlTransient public int getMessagesCount() { return messagesCount; diff --git a/juick-common/src/main/java/com/juick/server/helpers/AnonymousUser.java b/juick-common/src/main/java/com/juick/server/helpers/AnonymousUser.java index 122bbe29..9a201552 100644 --- a/juick-common/src/main/java/com/juick/server/helpers/AnonymousUser.java +++ b/juick-common/src/main/java/com/juick/server/helpers/AnonymousUser.java @@ -34,7 +34,6 @@ public final class AnonymousUser extends User { super.setAuthHash(getAuthHash()); super.setBanned(isBanned()); super.setCredentials(getCredentials()); - super.setLang(getLang()); } @Override @@ -82,11 +81,6 @@ public final class AnonymousUser extends User { return null; } - @Override - public String getLang() { - return "__"; - } - @Override public int getMessagesCount() { return 0; @@ -129,10 +123,6 @@ public final class AnonymousUser extends User { public void setCredentials(String credentials) { } - @Override - public void setLang(String lang) { - } - @Override public void setMessagesCount(int messagesCount) { } diff --git a/juick-common/src/main/java/com/juick/service/UserService.java b/juick-common/src/main/java/com/juick/service/UserService.java index 08a7a6ed..ef49a8ab 100644 --- a/juick-common/src/main/java/com/juick/service/UserService.java +++ b/juick-common/src/main/java/com/juick/service/UserService.java @@ -46,12 +46,8 @@ public interface UserService { User getUserByName(String username); - User getFullyUserByName(String username); - User getUserByEmail(String email); - List getFullyUsersByNames(Collection usernames); - User getUserByJID(String jid); List getUsersByName(Collection unames); diff --git a/juick-common/src/main/java/com/juick/service/security/HashParamAuthenticationFilter.java b/juick-common/src/main/java/com/juick/service/security/HashParamAuthenticationFilter.java index b56b98c8..9215d09a 100644 --- a/juick-common/src/main/java/com/juick/service/security/HashParamAuthenticationFilter.java +++ b/juick-common/src/main/java/com/juick/service/security/HashParamAuthenticationFilter.java @@ -69,7 +69,7 @@ public class HashParamAuthenticationFilter extends OncePerRequestFilter { User user = userService.getUserByHash(hash); if (!user.isAnonymous()) { - User userWithPassword = userService.getFullyUserByName(user.getName()); + User userWithPassword = userService.getUserByName(user.getName()); userWithPassword.setAuthHash(userService.getHashByUID(userWithPassword.getUid())); Authentication authentication = new RememberMeAuthenticationToken( ((AbstractRememberMeServices)rememberMeServices).getKey(), new JuickUser(userWithPassword), JuickUser.USER_AUTHORITY); diff --git a/juick-common/src/main/java/com/juick/service/security/JuickUserDetailsService.java b/juick-common/src/main/java/com/juick/service/security/JuickUserDetailsService.java index f6ae8909..adb0ab44 100644 --- a/juick-common/src/main/java/com/juick/service/security/JuickUserDetailsService.java +++ b/juick-common/src/main/java/com/juick/service/security/JuickUserDetailsService.java @@ -41,7 +41,7 @@ public class JuickUserDetailsService implements UserDetailsService { if (StringUtils.isBlank(username)) throw new UsernameNotFoundException("Invalid user name " + username); - com.juick.User user = userService.getFullyUserByName(username); + com.juick.User user = userService.getUserByName(username); if (user != null) { user.setAuthHash(userService.getHashByUID(user.getUid())); diff --git a/juick-common/src/main/java/com/juick/service/security/deprecated/CookieSimpleHashRememberMeServices.java b/juick-common/src/main/java/com/juick/service/security/deprecated/CookieSimpleHashRememberMeServices.java index bda5e902..e385d7dd 100644 --- a/juick-common/src/main/java/com/juick/service/security/deprecated/CookieSimpleHashRememberMeServices.java +++ b/juick-common/src/main/java/com/juick/service/security/deprecated/CookieSimpleHashRememberMeServices.java @@ -115,7 +115,7 @@ public class CookieSimpleHashRememberMeServices extends AbstractRememberMeServic Assert.isTrue(userOptional.isPresent()); - return new JuickUser(userService.getFullyUserByName(userOptional.get().getName())); + return new JuickUser(userService.getUserByName(userOptional.get().getName())); } @Override diff --git a/juick-common/src/main/java/com/juick/service/security/deprecated/RequestParamHashRememberMeServices.java b/juick-common/src/main/java/com/juick/service/security/deprecated/RequestParamHashRememberMeServices.java index 71159e17..3631e5a4 100644 --- a/juick-common/src/main/java/com/juick/service/security/deprecated/RequestParamHashRememberMeServices.java +++ b/juick-common/src/main/java/com/juick/service/security/deprecated/RequestParamHashRememberMeServices.java @@ -81,7 +81,7 @@ public class RequestParamHashRememberMeServices extends AbstractRememberMeServic if (StringUtils.isNotBlank(hash)) { User user = userService.getUserByHash(hash); if (!user.isAnonymous()) - return new JuickUser(userService.getFullyUserByName(user.getName())); + return new JuickUser(userService.getUserByName(user.getName())); } throw new UsernameNotFoundException("User not found by hash " + hash); } diff --git a/juick-server/src/main/java/com/juick/service/UserServiceImpl.java b/juick-server/src/main/java/com/juick/service/UserServiceImpl.java index 2de3dfc6..077fb01d 100644 --- a/juick-server/src/main/java/com/juick/service/UserServiceImpl.java +++ b/juick-server/src/main/java/com/juick/service/UserServiceImpl.java @@ -52,9 +52,8 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { user.setUid(rs.getInt(1)); user.setName(rs.getString(2)); - user.setBanned(rs.getBoolean(3)); - user.setLang(rs.getString(4)); - + user.setCredentials(rs.getString(3)); + user.setBanned(rs.getBoolean(4)); return user; } } @@ -121,7 +120,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { @Override public Optional getUserByUID(final int uid) { List list = getJdbcTemplate().query( - "SELECT id, nick, banned, lang FROM users WHERE id = ?", new UserMapper(), uid); + "SELECT id, nick, passw, banned FROM users WHERE id = ?", new UserMapper(), uid); return list.isEmpty() ? Optional.empty() : Optional.of(list.get(0)); } @@ -131,7 +130,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { public User getUserByName(final String username) { if (StringUtils.isNotBlank(username)) { List list = getJdbcTemplate().query( - "SELECT id, nick, banned, lang FROM users WHERE nick = ?", new UserMapper(), username); + "SELECT id, nick, passw, banned FROM users WHERE nick = ?", new UserMapper(), username); if (!list.isEmpty()) return list.get(0); @@ -140,23 +139,12 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { return AnonymousUser.INSTANCE; } - @Override - // No need marks with @Transactional annotation - public User getFullyUserByName(final String username) { - if (StringUtils.isNotBlank(username)) { - List list = getFullyUsersByNames(Collections.singletonList(username)); - if (!list.isEmpty()) - return list.get(0); - } - return null; - } - @Override @Transactional(readOnly = true) public User getUserByEmail(String email) { if (StringUtils.isNotBlank(email)) { List list = getJdbcTemplate().query( - "SELECT id, nick, banned, lang FROM users WHERE id = (SELECT DISTINCT user_id FROM emails WHERE email = ?)", + "SELECT id, nick, passw, banned FROM users WHERE id = (SELECT DISTINCT user_id FROM emails WHERE email = ?)", new UserMapper(), email); @@ -166,28 +154,6 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { return AnonymousUser.INSTANCE; } - @Transactional(readOnly = true) - @Override - public List getFullyUsersByNames(final Collection usernames) { - if (CollectionUtils.isEmpty(usernames)) - return Collections.emptyList(); - - return getNamedParameterJdbcTemplate().query( - "SELECT id, nick, passw, lang, banned FROM users WHERE nick in (:names)", - new MapSqlParameterSource("names", usernames), - (rs, rowNum) -> { - User user = new User(); - - user.setUid(rs.getInt(1)); - user.setName(rs.getString(2)); - user.setCredentials(rs.getString(3)); - user.setLang(rs.getString(4)); - user.setBanned(rs.getBoolean(5)); - - return user; - }); - } - @Transactional(readOnly = true) @Override public User getUserByJID(final String jid) { @@ -195,7 +161,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { if (StringUtils.isNotBlank(jid)) { List list = getJdbcTemplate().query( - "SELECT id, nick, banned, lang FROM users WHERE id = (SELECT user_id FROM jids WHERE jid = ?)", + "SELECT id, nick, passw, banned FROM users WHERE id = (SELECT user_id FROM jids WHERE jid = ?)", new UserMapper(), jid); @@ -212,7 +178,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { return Collections.emptyList(); return getNamedParameterJdbcTemplate().query( - "SELECT id, nick, banned, lang FROM users WHERE nick IN (:unames)", + "SELECT id, nick, passw, banned FROM users WHERE nick IN (:unames)", new MapSqlParameterSource("unames", unames), new UserMapper()); } @@ -224,7 +190,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { return Collections.emptyList(); return getNamedParameterJdbcTemplate().query( - "SELECT id, nick, banned, lang FROM users WHERE id IN (:ids)", + "SELECT id, nick, passw, banned FROM users WHERE id IN (:ids)", new MapSqlParameterSource("ids", uids), new UserMapper()); } @@ -279,7 +245,7 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { public com.juick.User getUserByHash(final String hash) { if (StringUtils.isNotBlank(hash)) { List list = getJdbcTemplate().query( - "SELECT logins.user_id, users.nick, users.banned, users.lang FROM logins " + + "SELECT logins.user_id, users.nick, users.passw, users.banned FROM logins " + "INNER JOIN users ON logins.user_id = users.id WHERE logins.hash = ?", new UserMapper(), hash); @@ -312,15 +278,8 @@ public class UserServiceImpl extends BaseJdbcService implements UserService { public int checkPassword(final String username, final String password) { if (StringUtils.isNotBlank(username)) { List list = getJdbcTemplate().query( - "SELECT id, nick, banned, passw FROM users WHERE nick = ?", - (rs, rowNum) -> { - User user = new User(); - user.setUid(rs.getInt(1)); - user.setName(rs.getString(2)); - user.setBanned(rs.getBoolean(3)); - user.setCredentials(rs.getString(4)); - return user; - }, + "SELECT id, nick, passw, banned FROM users WHERE nick = ?", + new UserMapper(), username); if (!list.isEmpty()) { diff --git a/juick-server/src/test/java/com/juick/server/tests/ServerTests.java b/juick-server/src/test/java/com/juick/server/tests/ServerTests.java index abeb7424..9f573e82 100644 --- a/juick-server/src/test/java/com/juick/server/tests/ServerTests.java +++ b/juick-server/src/test/java/com/juick/server/tests/ServerTests.java @@ -17,6 +17,7 @@ package com.juick.server.tests; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.jayway.jsonpath.JsonPath; @@ -59,6 +60,7 @@ import org.springframework.web.context.WebApplicationContext; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import org.w3c.dom.Document; +import org.w3c.dom.Element; import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; import org.xml.sax.SAXException; @@ -82,6 +84,7 @@ import java.io.*; import java.net.Socket; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.nio.file.*; import java.sql.Timestamp; import java.time.Instant; @@ -1225,4 +1228,31 @@ public class ServerTests { server.addConnectionIn(test); assertThat(getStatus.get().getInbound().size(), is(1)); } + @Test + public void credentialsShouldNeverBeSerialized() throws Exception { + int uid = userService.createUser("yyy", "xxxx"); + User yyy = userService.getUserByUID(uid).get(); + assertThat(yyy.getCredentials(), is("xxxx")); + ObjectMapper jsonMapper = new ObjectMapper(); + jsonMapper.setSerializationInclusion(JsonInclude.Include.NON_DEFAULT); + String jsonUser = jsonMapper.writeValueAsString(yyy); + Map user = JsonPath.read(jsonUser, "$"); + // only uid and name + assertThat(user.keySet().size(), is(2)); + + JAXBContext context = JAXBContext + .newInstance(User.class); + Marshaller m = context.createMarshaller(); + + StringWriter sw = new StringWriter(); + m.marshal(yyy, sw); + + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + DocumentBuilder db = dbf.newDocumentBuilder(); + Document doc = db.parse(new ByteArrayInputStream(sw.toString().getBytes(StandardCharsets.UTF_8))); + Element juickNode = doc.getDocumentElement(); + NamedNodeMap attrs = juickNode.getAttributes(); + // uid, name, xmlns, xmlns:user + assertThat(attrs.getLength(), is(4)); + } } -- cgit v1.2.3