[Yanel-dev] Forgot password feature

[Guillaume Déflache]

Hi!

Prabodh Upreti schrieb:
>  >>IIUC if a successful request (email exists) was done, then for this 
> user a file will be created
> 
>  >>data-repo/data/change-password-requests/USER_ID.xml (whereas the path 
> change->>password-requests is configurable)

I am not sure the file-based storage code works, the saving and loading 
parts seemed not to match when I reviewed the patch ( cf. "untested" in 
http://bugzilla.wyona.com/cgi-bin/bugzilla/show_bug.cgi?id=7164#c2 ). 
Did something got lost in the zip-file/merge/review dance?
A simple way of testing that is ask for a password reset from a 
non-system default browser (I used Google Chrome), and then click on the 
click which normally uses the system-browser (Firefox here), which of 
course uses a different session => the file-based storage code gets 
triggered: I get an exception with current svn code.

Maybe we should really use a Yarep repo here, e.g. the VFS 
implementation ( 
http://svn.wyona.com/repos/public/yarep/trunk/src/impl/java/org/wyona/yarep/impl/repo/vfs/ 
) should be able to address Michi's scalability concerns.


>  >>with the following content
> 
> <?xml version="1.0" encoding="UTF-8"?>
> <user xmlns="http://www.wyona.org/yanel/1.0">
> <email>michael.wechner at wyona.com <mailto:michael.wechner at wyona.com></email>
> <starttime>1248374094694</starttime>
> <guid>f4c9fa73-b10a-4033-a31c-7d0339bd3937</guid>
> </user>
> 
>  >>How is <starttime> related to the expire date of this request?
>  
> Start time was when the request was initiated.  The expire time is this 
> plus the configured validation hour(i.e 24 hr)

Why not store the expiry date directly? Simpler to check and the delay 
may be changed by the administrator in the meantime, so the expiry date 
can change, which is weird IMHO... and it also prevents us to display 
the expiry date in the message itself, which is more user-friendly IMHO 
(think about clicking on expired-token messages and struggling to find a 
valid one if you have a slow mail delivery / intermittent online access).

Having it in ISO 8601 format would could help diagnostics too, cf. 
http://www.w3.org/QA/Tips/iso-date


>  >>What does <guid> stand for? I guess the content is the "reset 
> password request id", but if so, >>then why call it like that?
> You are right. I am using multiple names for a single item.  I started 
> with reset password request id then in the middle thought this was just 
> a guid so started using that name.  We can chooce whichever is more 
> appropriate and use that.  I personally like guid.

I for one don't care as long as it's consistently used and documented 
somewhere. "guid" is fine IMHO.


> Why save the email instead the user id?
>  >> I was looking at this file also for auditing purpose.  email is 
> really not required there but I thought just in case someone wanted to 
> examine the file, it would make more sense then user id.
> 
> Re scalability, if we have one million users and many people forget 
> their passwords, do we have to parse all these files to find the correct 
> "reset password request id"?
> 
>  >>Good point.  If I had a relational db I would not think much about 
> this and let the query pick the right record.  The db administrator 
> would manage the table growth.  Here I thought again we would like to 

I'd rather have "self-managed data", and we do not want to 
unconditionally require a DB for such a "simple" feature I guess.


> save the request for audit purpose.  Maybe we could delete the file and 
> just write it to a log file that pw was reset.  Also the code is 

The log file idea is a good idea, we could simply use a dedicated Log4J 
logger for that.


> currently going through all the file to check the content and match the 
> guid.  What I could do is name the file based on the guid and look for 
> that file name when the request comes(i.e guid 2234jlkjdjfsd.xml  )

Agreed (see above), for debugging/auditing one can always rgrep the 
directory.


> [...]
> Please let me know you thoughts and I will make the appropriate changes.

You are most welcome to work on everything here if you fancy it except 
maybe the storage stuff which Michi or I will probably try to fix ASAP 
anyway.

Cheers,
    Guillaume