Re: [PEPr] Call for votes on Authentication::Auth_HTTP_Digest

On Wed, 3 Mar 2004 08:13:23 +0100 Martin Jansen <mj@php.net> wrote: > On Sun Feb 29, 2004 at 02:0122PM +0900, Rui Hirokawa wrote: > > I replaced tab with spaces and made a unified diff. > > > > > > http://www.geocities.jp/rui_hirokawa/php/scripts/Auth_HTTP-digest.patch.gz > > Looks cool. A few points though: > > - Can you replace the usage of apache_request_headers() with > getallheaders()? > > This way the code will also work with PHP < 4.3.0. Sure. It is very easy to to. > > - At some places you did not follow the coding standards strictly. > (This can be fixed once the digest authentication is in CVS.) > > - return PEAR::raiseError('authMethod is invalid.'); > > should be something like > > return PEAR::raiseError('Invalid HTTP authentication method.'); O.K. > > - I really do not like the fact that the database is accessed directly > in login(). Isn't there an easy way to handle this inside the storage > containers? I tried to implement login() in more generic way, but I failed. It is because authentication like md5('password from user'+'temporal parameter1 ') == md5('password from container'+'temporal parameter2 ') is not supported in the current implementation. It is necessary to implement the digest authentication. I think we should add a method like getPassword() into Auth() to retrieve password from container. I also recommend to move the authentication part from containers class to Auth() class for clarification. -- Rui Hirokawa <rui_hirokawa@ybb.ne.jp>