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

On Sun, Jun 26, 2011 at 23:51, Ferenc Kovacs <tyra3l@gmail.com> wrote: > 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. It was by design as it was authenticating against a local database anyway.. A local session variable should be trustable :) -Hannes