Re: pecl.php.net auth from master.php.net

On Sun, Jun 26, 2011 at 10:53 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > On Sun, Jun 26, 2011 at 10:09 PM, Hannes Magnusson > <hannes.magnusson@gmail.com> wrote: >> On Sun, Jun 26, 2011 at 20:00, Ferenc Kovacs <tyra3l@gmail.com> wrote: >>> On Sun, Jun 26, 2011 at 7:16 PM, Hannes Magnusson >>> <hannes.magnusson@gmail.com> wrote: >>>> On Sun, Jun 26, 2011 at 19:09, Ferenc Kovacs <tyra3l@gmail.com> wrote: >>>>> On Sun, Jun 26, 2011 at 5:44 PM, Hannes Magnusson >>>>> <hannes.magnusson@gmail.com> wrote: >>>>>> On Sun, Jun 26, 2011 at 03:00, Ferenc Kovacs <tyra3l@gmail.com> >>>>>> wrote: >>>>>>> Hi. >>>>>>> >>>>>>> I've started implementing the master authentication into the >>>>>>> peclweb >>>>>>> codebase, this was we wouldn't have to store passwords there. >>>>>>> I've successively implemented the login/logout stuff, and it is >>>>>>> working (you need the AUTH_TOKEN environment variable to be set to be >>>>>>> able to test this). >>>>>>> I wanted to ask your opinion about the diff, before proceeding. >>>>>>> more info about this change can be found on >>>>>>> https://wiki.php.net/pecl/web/todo >>>>>> >>>>>> I don't think you can drop users without svn account as there are a >>>>>> good number of exts that do not use our svn, so not all authors do >>>>>> have svn accounts. >>>>>> >>>>>> Also, there is no guarantee that pecl usernames map correctly to the >>>>>> svn usernames... >>>>>> >>>>>> If you have however taken this into account and checked if there >>>>>> aren't that many 'edge cases', then I don't have any >>>>>> objections :) >>>>>> >>>>>> -Hannes >>>>>> >>>>> >>>>> Hi. >>>>> >>>>> Pierre said that it could/should be done this way. >>>>> I will look into this, but Pierre suggested that we should fix the >>>>> problematic users on case-by-case basis. >>>>> btw: what do you think about the patch? >>>> >>>> >>>> If you sent one, it didn't come through. The list stripsout pretty >>>> much everything except for text/plain >>>> >>>> -Hannes >>>> >>> >>> I was afraid of this. :/ >>> attaching again with .txt extension. >> >> >> It looks like you are authenticating against master on every request? >> And unset($_SESSION); is generally discouraged (see >> http://php.net/session_unset). >> Other then that, seems ok. >> >> -Hannes >> > > thanks, I will fix those. > > Tyrael > I fixed the session_unset, but the authenticate on every request is by design. At least the original codebase did this, instead of using some session variable (is_logged or similar). I didn't wanted my patch to be too intrusive, but I think it would be safe to start trusting session flag instead of hammering the master.php.net on each request. Tyrael