Author: tchemit Date: 2012-08-13 15:08:27 +0200 (Mon, 13 Aug 2012) New Revision: 3603 Url: http://chorem.org/repositories/revision/pollen/3603 Log: refs #746: Improve security model (let's have a unique request to find voter account, and anohter one to have a restricted voter) Modified: trunk/pollen-persistence/src/main/java/org/chorem/pollen/business/persistence/PollAccountDAOImpl.java trunk/pollen-persistence/src/test/java/org/chorem/pollen/business/persistence/PollAccountDAOImplTest.java Modified: trunk/pollen-persistence/src/main/java/org/chorem/pollen/business/persistence/PollAccountDAOImpl.java =================================================================== --- trunk/pollen-persistence/src/main/java/org/chorem/pollen/business/persistence/PollAccountDAOImpl.java 2012-08-13 13:07:02 UTC (rev 3602) +++ trunk/pollen-persistence/src/main/java/org/chorem/pollen/business/persistence/PollAccountDAOImpl.java 2012-08-13 13:08:27 UTC (rev 3603) @@ -23,6 +23,8 @@ package org.chorem.pollen.business.persistence; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; +import org.apache.commons.lang3.StringUtils; import org.chorem.pollen.PollenPersistenceUtil; import org.nuiton.topia.TopiaException; import org.nuiton.topia.persistence.TopiaFilterPagerUtil; @@ -32,140 +34,117 @@ public class PollAccountDAOImpl<E extends PollAccount> extends PollAccountDAOAbstract<E> { /** - * Get a restricted poll account for a given poll - * ( using his {@code pollId} ) using the - * {@link PollAccount#getAccountId()} of the pollAccount. + * Get a restricted poll account for a given poll (by his {@link Poll#getPollId()}) + * using the {@link PollAccount#getAccountId()} of the pollAccount + * and/or the {@link PollAccount#getEmail()}. + * <p/> + * <strong>Note:</strong> {@code accountId} and {@code user} can not + * be null at the same time. * * @param pollId pollId of the poll * @param accountId the accountId of the pollAccount to find + * @param user the connected user account to test * @return the found pollAccount, or {@code null} if not foud * @throws TopiaException if any db pb */ - public E findRestrictedPollAccountByAccountId(String pollId, - String accountId) throws TopiaException { + public E findRestrictedPollAccount(String pollId, + String accountId, + UserAccount user) throws TopiaException { - Preconditions.checkNotNull(pollId); - Preconditions.checkNotNull(accountId); + Preconditions.checkArgument(StringUtils.isNotBlank(pollId)); + Preconditions.checkArgument(StringUtils.isNotBlank(accountId) || + user != null); -// TopiaQuery query = new TopiaQuery(PersonToList.class, "p"). -// addFrom(Poll.class, "poll"). -// setSelect("p." + PersonToList.PROPERTY_POLL_ACCOUNT). -// addWhere("poll." + Poll.PROPERTY_POLL_ID, TopiaQuery.Op.EQ, pollId). -// addWhere("p." + PersonToList.PROPERTY_POLL_ACCOUNT + "." + PollAccount.PROPERTY_ACCOUNT_ID, TopiaQuery.Op.EQ, accountId). -// addInElements("p", "poll." + Poll.PROPERTY_VOTING_LIST + "." + VotingList.PROPERTY_POLL_ACCOUNT_PERSON_TO_LIST); -// E result = findByQuery(query); - String hql = "SELECT p.pollAccount FROM PersonToListImpl p, PollImpl poll WHERE " + "poll.pollId = :pollId AND " + - "p.pollAccount.accountId = :accountId AND " + - "p in elements (poll.votingList.pollAccountPersonToList)"; + "p in elements (poll.votingList.pollAccountPersonToList) AND "; - E result = PollenPersistenceUtil.findUnique( - this, hql, "pollId", pollId, "accountId", accountId); - return result; - } - /** - * Get a restricted poll account for a given poll - * ( using his {@code pollId} ) using the - * {@link PollAccount#getEmail()} of the pollAccount. - * - * @param pollId pollId of the poll - * @param email the email of the pollAccount to find - * @return the found pollAccount, or {@code null} if not found - * @throws TopiaException if any db pb - */ - public E findRestrictedPollAccountByEmail(String pollId, - String email) throws TopiaException { + boolean withAccountId = StringUtils.isNotBlank(accountId); + boolean withEmail = user != null; - Preconditions.checkNotNull(pollId); - Preconditions.checkNotNull(email); + List<String> params = Lists.newArrayList("pollId", pollId); -// TopiaQuery query = new TopiaQuery(PersonToList.class, "p"). -// addFrom(Poll.class, "poll"). -// setSelect("p." + PersonToList.PROPERTY_POLL_ACCOUNT). -// addWhere("poll." + Poll.PROPERTY_POLL_ID, TopiaQuery.Op.EQ, pollId). -// addWhere("p." + PersonToList.PROPERTY_POLL_ACCOUNT + "." + PollAccount.PROPERTY_EMAIL, TopiaQuery.Op.EQ, email). -// addInElements("p", "poll." + Poll.PROPERTY_VOTING_LIST + "." + VotingList.PROPERTY_POLL_ACCOUNT_PERSON_TO_LIST); -// E result = findByQuery(query); + if (withAccountId && withEmail) { - String hql = "SELECT p.pollAccount FROM PersonToListImpl p, PollImpl poll WHERE " + - "poll.pollId = :pollId AND " + - "p.pollAccount.email = :email AND " + - "p in elements (poll.votingList.pollAccountPersonToList)"; + // try with both email or accountId + params.add("accountId"); + params.add(accountId); + params.add("email"); + params.add(user.getEmail()); + hql += "(p.pollAccount.accountId = :accountId OR p.pollAccount.email = :email)"; + } else if (withAccountId) { - E result = PollenPersistenceUtil.findUnique( - this, hql, "pollId", pollId, "email", email); + // try only with accountId + params.add("accountId"); + params.add(accountId); + hql += "p.pollAccount.accountId = :accountId"; + } else { + + // try only with email + params.add("email"); + params.add(user.getEmail()); + hql += "p.pollAccount.email = :email"; + } + + E result = PollenPersistenceUtil.findUnique(this, hql, params.toArray()); return result; } /** * Get the pollAccount of a voter for a given {@code pollId} using his - * {@code accountId}. + * {@code accountId} or {@code user}. + * <p/> + * <strong>Note:</strong> {@code accountId} and {@code user} can not + * be null at the same time. * * @param pollId the poll where to seek * @param accountId the account id to test + * @param user the connected user account to test * @return the found pollAccount, or {@code null} if not found * @throws TopiaException if any db pb */ - public E findVoterPollAccountByAccountId(String pollId, - String accountId) throws TopiaException { + public E findVoterPollAccount(String pollId, + String accountId, + UserAccount user) throws TopiaException { - Preconditions.checkNotNull(pollId); - Preconditions.checkNotNull(accountId); + Preconditions.checkArgument(StringUtils.isNotBlank(pollId)); + Preconditions.checkArgument(StringUtils.isNotBlank(accountId) || + user != null); -// TopiaQuery query = createQuery("e") -// .addFrom(Poll.class, "p") -// .addFrom(Vote.class, "v") -// .addEquals("p." + Poll.PROPERTY_POLL_ID, pollId) -// .addInElements("v", "p." + Poll.PROPERTY_VOTE) -// .addWhere("e = v." + Vote.PROPERTY_POLL_ACCOUNT) -// .addEquals("e." + PollAccount.PROPERTY_ACCOUNT_ID, accountId); -// PollAccount result = findByQuery(query); - String hql = "SELECT e FROM PollAccountImpl e, PollImpl p, VoteImpl v WHERE " + "p.pollId = :pollId AND " + "v in elements (p.vote) AND " + - "e = v.pollAccount AND " + - "e.accountId = :accountId"; + "e = v.pollAccount AND "; - E result = PollenPersistenceUtil.findUnique( - this, hql, "pollId", pollId, "accountId", accountId); - return result; - } + boolean withAccountId = StringUtils.isNotBlank(accountId); + boolean withEmail = user != null; - /** - * Get the pollAccount of a voter for a given {@code pollId} using his - * {@code user}. - * - * @param pollId the poll id where to seek - * @param user the user account to test - * @return the found pollAccount, or {@code null} if not found - * @throws TopiaException if any db pb - */ - public E findVoterPollAccountByUserAccount(String pollId, - UserAccount user) throws TopiaException { + List<Object> params = Lists.<Object>newArrayList("pollId", pollId); - Preconditions.checkNotNull(pollId); - Preconditions.checkNotNull(user); + if (withAccountId && withEmail) { -// TopiaQuery query = createQuery("e") -// .addFrom(Poll.class, "p") -// .addFrom(Vote.class, "v") -// .addEquals("p." + Poll.PROPERTY_POLL_ID, pollId) -// .addInElements("v", "p." + Poll.PROPERTY_VOTE) -// .addWhere("e = v." + Vote.PROPERTY_POLL_ACCOUNT) -// .addEquals("e." + PollAccount.PROPERTY_USER_ACCOUNT, user); -// PollAccount result = findByQuery(query); + // try with both email or accountId + params.add("accountId"); + params.add(accountId); + params.add("user"); + params.add(user); + hql += "(e.accountId = :accountId OR e.userAccount = :user)"; + } else if (withAccountId) { - String hql = "SELECT e FROM PollAccountImpl e, PollImpl p, VoteImpl v WHERE " + - "p.pollId = :pollId AND " + - "v in elements (p.vote) AND " + - "e = v.pollAccount AND " + - "e.userAccount = :user"; + // try only with accountId + params.add("accountId"); + params.add(accountId); + hql += "e.accountId = :accountId"; + } else { - E result = PollenPersistenceUtil.findUnique( - this, hql, "pollId", pollId, "user", user); + // try only with email + params.add("user"); + params.add(user); + hql += "e.userAccount = :user"; + } + + E result = PollenPersistenceUtil.findUnique(this, hql, params.toArray()); return result; } @@ -217,23 +196,23 @@ "e.email = :email"; boolean result; String pollAccountId = pollAccount.getTopiaId(); - if (pollAccountId == null) { - // no check necessary, it's a new pollAccount + List<Object> params = Lists.<Object>newArrayList( + "person", personListToUpdate, + "email", pollAccount.getEmail()); - result = PollenPersistenceUtil.exists( - this, hql, - "person", personListToUpdate, - "email", pollAccount.getEmail()); - } else { + if (StringUtils.isNotBlank(pollAccountId)) { + + // check also by account topiaId + hql += " AND e.topiaId = :topiaId"; - result = PollenPersistenceUtil.exists( - this, hql, - "person", personListToUpdate, - "email", pollAccount.getEmail(), - "topiaId", pollAccountId); + params.add("topiaId"); + params.add(pollAccountId); + } + + result = PollenPersistenceUtil.exists(this, hql, params.toArray()); return result; } Modified: trunk/pollen-persistence/src/test/java/org/chorem/pollen/business/persistence/PollAccountDAOImplTest.java =================================================================== --- trunk/pollen-persistence/src/test/java/org/chorem/pollen/business/persistence/PollAccountDAOImplTest.java 2012-08-13 13:07:02 UTC (rev 3602) +++ trunk/pollen-persistence/src/test/java/org/chorem/pollen/business/persistence/PollAccountDAOImplTest.java 2012-08-13 13:08:27 UTC (rev 3603) @@ -57,20 +57,13 @@ String accountEmail3 = "email3"; @Test - public void findVoterPollAccountByAccountId() throws Exception { + public void findVoterPollAccount() throws Exception { if (log.isWarnEnabled()) { - log.warn("Missing PollAccountDAOImpl#findVoterPollAccountByAccountId request test!"); + log.warn("Missing PollAccountDAOImpl#findVoterPollAccount request test!"); } } @Test - public void findVoterPollAccountByUserAccount() throws Exception { - if (log.isWarnEnabled()) { - log.warn("Missing PollAccountDAOImpl#findVoterPollAccountByUserAccount request test!"); - } - } - - @Test public void findFavoriteListUsers() throws TopiaException { if (log.isWarnEnabled()) { log.warn("Missing PollAccountDAOImpl#findFavoriteListUsers request test!"); @@ -84,8 +77,9 @@ } } + @Test - public void findRestrictedPollAccountByAccountId() throws Exception { + public void findRestrictedPollAccount() throws Exception { TopiaContext tx = beginTransaction(); @@ -94,6 +88,11 @@ VotingListDAO votingListDAO = PollenDAOHelper.getVotingListDAO(tx); PersonToListDAO personToListDAO = PollenDAOHelper.getPersonToListDAO(tx); + UserAccount userAccount = new UserAccountImpl(accountEmail); + UserAccount userAccount2 = new UserAccountImpl(accountEmail2); + UserAccount userAccount3 = new UserAccountImpl(accountEmail3); + UserAccount userAccountFake = new UserAccountImpl("HumFAKE!"); + PollAccount pollAccount = pollAccountDAO.create( PollAccount.PROPERTY_ACCOUNT_ID, accountId, PollAccount.PROPERTY_EMAIL, accountEmail); @@ -130,113 +129,80 @@ pollAccount2.addVotingListPersonToList(personToList2); pollAccount3.addVotingListPersonToList(personToList3); - PollAccount restrictedPollAccount = - pollAccountDAO.findRestrictedPollAccountByAccountId(pollId, accountId); + PollAccount restrictedPollAccount; - Assert.assertNotNull(restrictedPollAccount); - Assert.assertEquals(accountId, restrictedPollAccount.getAccountId()); - Assert.assertNotNull(restrictedPollAccount.getVotingListPersonToList()); - Assert.assertTrue(restrictedPollAccount.getVotingListPersonToList().contains(personToList)); + // -- PollAccount -- // - PollAccount restrictedPollAccount2 = - pollAccountDAO.findRestrictedPollAccountByAccountId(pollId, accountId2); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId, null); + assertRestrictedAccount(restrictedPollAccount, accountId, null, personToList); - Assert.assertNotNull(restrictedPollAccount2); - Assert.assertEquals(accountId2, restrictedPollAccount2.getAccountId()); - Assert.assertNotNull(restrictedPollAccount2.getVotingListPersonToList()); - Assert.assertFalse(restrictedPollAccount2.getVotingListPersonToList().contains(personToList)); - Assert.assertTrue(restrictedPollAccount2.getVotingListPersonToList().contains(personToList2)); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId, userAccountFake); + assertRestrictedAccount(restrictedPollAccount, accountId, null, personToList); - PollAccount restrictedPollAccount3 = - pollAccountDAO.findRestrictedPollAccountByAccountId(pollId, accountId3); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, null, userAccount); + assertRestrictedAccount(restrictedPollAccount, null, userAccount, personToList); - Assert.assertNotNull(restrictedPollAccount3); - Assert.assertEquals(accountId3, restrictedPollAccount3.getAccountId()); - Assert.assertNotNull(restrictedPollAccount3.getVotingListPersonToList()); - Assert.assertFalse(restrictedPollAccount3.getVotingListPersonToList().contains(personToList)); - Assert.assertTrue(restrictedPollAccount3.getVotingListPersonToList().contains(personToList3)); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, "HummFAKE", userAccount); + assertRestrictedAccount(restrictedPollAccount, null, userAccount, personToList); - PollAccount restrictedPollAccount4 = - pollAccountDAO.findRestrictedPollAccountByAccountId(pollId, System.nanoTime() + "--"); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId, userAccount); + assertRestrictedAccount(restrictedPollAccount, accountId, userAccount, personToList); - Assert.assertNull(restrictedPollAccount4); - } + // -- PollAccount2 -- // - @Test - public void findRestrictedPollAccountByEmail() throws Exception { - TopiaContext tx = beginTransaction(); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId2, null); - PollDAO pollDAO = PollenDAOHelper.getPollDAO(tx); - PollAccountDAO pollAccountDAO = PollenDAOHelper.getPollAccountDAO(tx); - VotingListDAO votingListDAO = PollenDAOHelper.getVotingListDAO(tx); - PersonToListDAO personToListDAO = PollenDAOHelper.getPersonToListDAO(tx); + assertRestrictedAccount(restrictedPollAccount, accountId2, null, personToList2); - PollAccount pollAccount = pollAccountDAO.create( - PollAccount.PROPERTY_ACCOUNT_ID, accountId, - PollAccount.PROPERTY_EMAIL, accountEmail); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId2, userAccountFake); + assertRestrictedAccount(restrictedPollAccount, accountId2, null, personToList2); - PollAccount pollAccount2 = pollAccountDAO.create( - PollAccount.PROPERTY_ACCOUNT_ID, accountId2, - PollAccount.PROPERTY_EMAIL, accountEmail2); - PollAccount pollAccount3 = pollAccountDAO.create( - PollAccount.PROPERTY_ACCOUNT_ID, accountId3, - PollAccount.PROPERTY_EMAIL, accountEmail3); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, null, userAccount2); + assertRestrictedAccount(restrictedPollAccount, null, userAccount2, personToList2); - Poll poll = pollDAO.create(Poll.PROPERTY_POLL_ID, pollId); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, "HUMFake!", userAccount2); + assertRestrictedAccount(restrictedPollAccount, null, userAccount2, personToList2); - VotingList votingList = votingListDAO.create(); - poll.addVotingList(votingList); - VotingList votingList2 = votingListDAO.create(); - poll.addVotingList(votingList2); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId2, userAccount2); + assertRestrictedAccount(restrictedPollAccount, accountId2, userAccount2, personToList2); - PersonToList personToList = personToListDAO.create( - PersonToList.PROPERTY_POLL_ACCOUNT, pollAccount, - PersonToList.PROPERTY_VOTING_LIST, votingList - ); + // -- PollAccount3-- // - PersonToList personToList2 = personToListDAO.create( - PersonToList.PROPERTY_POLL_ACCOUNT, pollAccount2, - PersonToList.PROPERTY_VOTING_LIST, votingList - ); - PersonToList personToList3 = personToListDAO.create( - PersonToList.PROPERTY_POLL_ACCOUNT, pollAccount3, - PersonToList.PROPERTY_VOTING_LIST, votingList2 - ); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId3, null); + assertRestrictedAccount(restrictedPollAccount, accountId3, null, personToList3); - pollAccount.addVotingListPersonToList(personToList); - pollAccount2.addVotingListPersonToList(personToList2); - pollAccount3.addVotingListPersonToList(personToList3); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId3, userAccountFake); + assertRestrictedAccount(restrictedPollAccount, accountId3, null, personToList3); - PollAccount restrictedPollAccount = - pollAccountDAO.findRestrictedPollAccountByEmail(pollId, accountEmail); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, null, userAccount3); + assertRestrictedAccount(restrictedPollAccount, null, userAccount3, personToList3); - Assert.assertNotNull(restrictedPollAccount); - Assert.assertEquals(accountId, restrictedPollAccount.getAccountId()); - Assert.assertNotNull(restrictedPollAccount.getVotingListPersonToList()); - Assert.assertTrue(restrictedPollAccount.getVotingListPersonToList().contains(personToList)); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, "HUMFake!", userAccount3); + assertRestrictedAccount(restrictedPollAccount, null, userAccount3, personToList3); - PollAccount restrictedPollAccount2 = - pollAccountDAO.findRestrictedPollAccountByEmail(pollId, accountEmail2); + restrictedPollAccount = pollAccountDAO.findRestrictedPollAccount(pollId, accountId3, userAccount3); + assertRestrictedAccount(restrictedPollAccount, accountId3, userAccount3, personToList3); - Assert.assertNotNull(restrictedPollAccount2); - Assert.assertEquals(accountId2, restrictedPollAccount2.getAccountId()); - Assert.assertNotNull(restrictedPollAccount2.getVotingListPersonToList()); - Assert.assertFalse(restrictedPollAccount2.getVotingListPersonToList().contains(personToList)); - Assert.assertTrue(restrictedPollAccount2.getVotingListPersonToList().contains(personToList2)); + restrictedPollAccount = + pollAccountDAO.findRestrictedPollAccount(pollId, System.nanoTime() + "--", null); - PollAccount restrictedPollAccount3 = - pollAccountDAO.findRestrictedPollAccountByEmail(pollId, accountEmail3); - - Assert.assertNotNull(restrictedPollAccount3); - Assert.assertEquals(accountId3, restrictedPollAccount3.getAccountId()); - Assert.assertNotNull(restrictedPollAccount3.getVotingListPersonToList()); - Assert.assertFalse(restrictedPollAccount3.getVotingListPersonToList().contains(personToList)); - Assert.assertTrue(restrictedPollAccount3.getVotingListPersonToList().contains(personToList3)); - - PollAccount restrictedPollAccount4 = - pollAccountDAO.findRestrictedPollAccountByEmail(pollId, System.nanoTime() + "--"); - - Assert.assertNull(restrictedPollAccount4); + Assert.assertNull(restrictedPollAccount); } + protected void assertRestrictedAccount(PollAccount restrictedPollAccount, + String accountId, + UserAccount useraccount, + PersonToList personToList) { + Assert.assertNotNull(restrictedPollAccount); + if (accountId != null) { + Assert.assertEquals(accountId, + restrictedPollAccount.getAccountId()); + } + if (useraccount != null) { + Assert.assertEquals(useraccount.getEmail(), + restrictedPollAccount.getEmail()); + } + Assert.assertNotNull(restrictedPollAccount.getVotingListPersonToList()); + Assert.assertTrue(restrictedPollAccount.getVotingListPersonToList().contains(personToList)); + } }