Customized patch-set feedback

[GoogleCodeExporter]

I've been asked to implement and customize PWM for a large University in the
Netherlands. The project has landed in the test phase, so further development
cycles should be far apart and it has become time to disclose my efforts to
the community at large.

Since there were many tweaks and changes it's quite difficult to explain the
diff, but I'll try anyway. Hopes are, a number of these changes will find their
way back into the codebase so future patching will be less cumbersome.

The patch is against 1.6.0 because that was the latest version available when I
started working on the project last year.

1. One of the biggest changes was a mandatory roundtrip to UpdateProfile before
allowing ChangePassword I did this by implementing an extra checkProfile check
in the ChangePassword servlet. Then, if needed redirect to UpdateProfile
while setting ChangePassword as ForwardeURL, and resetting it back when the
Profile is set. Let me say I am fully aware that some of my solutions do not
deserve first price as they were implemented under both monetary and my
java(in)ability constraints. This patch could use some love in the sense that
there probably should be introduced an extra forward/return parameter in the
ChangePasswordBean and it should be configurable, which is true for most of my
changes.
The reason for the mandatory roundtrip before passwordchange is that this way
user are 'punished' (with a non functional account) for not supplying crucial
password reset self-service information, hoping that will reduce support calls
even more once PWM is taken into production.

2. There was need for a validated phone number input field, which I added to
PwmSetting.properties and the logic to FormConfiguration.java. The validation
logic (Helper.testPhoneNumber) is hardcoded to contain a starting + and 10 to 12
digits.

3. Instead of asking security responses, they wanted to ask for one random
attribute chosen from the list of attributes, but only if it contains a value. I
built most of the needed logic for the random attribute question completely in
forgottenpassword-responses.jsp because that was the quickest way to reach the
goal (and easiest to debug). Probably should be moved up to the
ForgottenPasswordServlet instead.

4. Paranoia demanded for the password reset token to be stored and retrieved as
an (md5) hash instead of plaintext in PWMDB (TokenManager.java). I also
implemented a token retrieval routine (showTokens) in pwm.util.Mainclass to
prove my change :/

5. The role of Helpdesk had to be changed from password reset/unlock smurfs to
Self-service enablers. This resulted in removing the password reset and account
unlock capabilities and adding mandatory user attributes reviewing and setting
so that the user is able to self-service after consulting support. This may
sound backwards, but in the end hopefully results in more educated self-service
capable users (HelpdeskServlet/helpdesk.jsp). This also required changes to the
Helpdesk attributes type (form).

6. There are probably some leftovers of debug statements or dangling imports I
thought I needed but didn't use after all. I don't use an IDE to write the code
so I'm not alerted for these kind of mistakes/omissions.

Best regards,
Martin

Original issue reported on code.google.com by mrva...@gmail.com on 2 Mar 2012 at 9:36

Attachments:

• pwm_v1.6.0.diff

[GoogleCodeExporter]

Excellent!  Thank you very much for your interest and especially than you for 
submitting your improvements!  Will review and add to the svn trunk as 
appropriate.

Original comment by jrivard on 8 Mar 2012 at 10:58

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Help

[GoogleCodeExporter]

telephone number support in forms added in svn revision 377

Original comment by jrivard on 25 Mar 2012 at 6:07

[GoogleCodeExporter]

item 3 implemented in svn revision 383

Original comment by jrivard on 12 Apr 2012 at 6:02

[GoogleCodeExporter]

item 1 implemented in svn revision 385 as option "Force Profile Update"

Original comment by jrivard on 17 Apr 2012 at 3:48

[GoogleCodeExporter]

secure (encrypted) storage of token payloads and hashed storage of token keys, 
along with the pure-crypto token mechanism (with no storage a all) pretty much 
covers item 4.  MainClass token reader coming shortly.

Original comment by jrivard on 21 May 2012 at 3:36

[GoogleCodeExporter]

Hi Jason. Thank you for adopting the majority of the items, this will make 
maintaining the PWM app much easier in the future! They are curious however 
howyou think about item 5. Will this be a custom patch indefinately for them or 
are you contemplating adopting the change (in one way or another) in the future?

Original comment by mrva...@gmail.com on 22 Aug 2012 at 7:24

[GoogleCodeExporter]

Current trunk builds should have enough capabilities to cover item #5 now, via 
helpdesk -viewable responses and arbitrary ldap/webservice actions.  Please 
re-open this issue if something is still missing.

Original comment by jrivard on 11 Mar 2013 at 6:23

• Changed state: Done

[GoogleCodeExporter]

I have recently implemented a 1.7 install and recognised many of these changes. 
The 'force update profile' however only redirects the user after having set the 
password. The idea was not having a working account until the profile was 
updated correctly (set pwd after update profile). Did you implement it this way 
on purpose?

Original comment by mrva...@gmail.com on 12 Mar 2014 at 8:22