> On 2012-01-09 17:41:59, mfranklin wrote: > > A couple of notes: > > > > 1) I think there is room for consolidation of similar functionality into > > cohesive units (IE UserService) > > 2) Your services all use checked exceptions, the rest of Rave uses > > RuntimeExceptions > > 3) I would review the model for resetting a password via e-mail. There are > > a couple of attacks that could be made if you were able to get the > > forgotPasswordHash vi Man in the middle or other means. Though, these are > > common in a lot of e-mail reset mechanisms > >
Hi Matt, do you have any suggestions for nr. 3)? > On 2012-01-09 17:41:59, mfranklin wrote: > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultNewPasswordService.java, > > line 114 > > <https://reviews.apache.org/r/3427/diff/1/?file=67120#file67120line114> > > > > Only problem with saving this to the database is that there is no time > > constraint on how long this reset hash is good for, unless you define a > > cleanup job. hm. what about another field (time stamp) which could be used to make certain requests invalid? User would be prompted to make another request if old one is expired. - marijan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3427/#review4267 ----------------------------------------------------------- On 2012-01-08 00:50:10, marijan milicevic wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3427/ > ----------------------------------------------------------- > > (Updated 2012-01-08 00:50:10) > > > Review request for rave. > > > Summary > ------- > > - configure mail settings to be able to test, see comments in: > /rave/rave-components/rave-core/src/main/resources/org/apache/rave/core-applicationContext.xml > > > Diffs > ----- > > https://svn.apache.org/repos/asf/incubator/rave/trunk/pom.xml 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/pom.xml > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/NewUser.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/User.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/UserRepository.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/impl/JpaUserRepository.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/ChangePasswordService.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/NewPasswordService.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultChangePasswordService.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultNewPasswordService.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/main/resources/org/apache/rave/core-applicationContext.xml > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-core/src/test/resources/portal.properties > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ChangePasswordController.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/NewAccountController.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/PasswordReminderController.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/ReminderController.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/controller/UsernameReminderController.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ModelKeys.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/util/ViewNames.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/validator/ChangePasswordValidator.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/validator/NewAccountValidator.java > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/validator/NewPasswordValidator.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/ChangePasswordControllerTest.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-components/rave-web/src/test/java/org/apache/rave/portal/web/controller/PasswordReminderControllerTest.java > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/resources/messages.properties > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/resources/portal.properties > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/applicationContext-security.xml > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/dispatcher-servlet.xml > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/changepassword.jsp > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/newpassword.jsp > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/retrieveusername.jsp > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/mailtemplates/password_reminder.ftl > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/WEB-INF/mailtemplates/username_reminder.ftl > PRE-CREATION > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal-resources/src/main/webapp/login.jsp > 1228743 > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/pom.xml > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/main/dist/NOTICE > 1228743 > > https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal/src/test/resources/portal.properties > 1228743 > > Diff: https://reviews.apache.org/r/3427/diff > > > Testing > ------- > > > Thanks, > > marijan > >
