Sure, Will test this thoroughly after complete investigation.
Kind Regards, On Wed, Oct 19, 2016 at 1:27 PM, Dave Page <dp...@pgadmin.org> wrote: > Patch applied. > > Fahar, can you please test this thoroughly in desktop and server modes, > with both fresh and upgraded installations? > > https://redmine.postgresql.org/issues/1849 > > Packagers: This change means that packages are no longer forced to create > a config_local.py file, and there is no longer any need to explicitly set > SECURITY_PASSWORD_SALT, SECURITY_KEY and CSRF_SESSION_KEY in the config > (in fact, they should be removed for new installations, if you have > included them in 1.0) > > Thanks. > > > On Wed, Oct 19, 2016 at 6:46 AM, Ashesh Vashi < > ashesh.va...@enterprisedb.com> wrote: > >> Hi Dave, >> >> On Sat, Oct 15, 2016 at 8:02 AM, Dave Page <dp...@pgadmin.org> wrote: >> >>> Hi >>> >>> >>> On Friday, October 14, 2016, Dave Page <dp...@pgadmin.org> wrote: >>> >>>> Hi >>>> >>>> On Thursday, October 13, 2016, Ashesh Vashi < >>>> ashesh.va...@enterprisedb.com> wrote: >>>> >>>>> Hi Dave, >>>>> >>>>> On Tue, Oct 11, 2016 at 9:10 PM, Dave Page <dp...@pgadmin.org> wrote: >>>>> >>>>>> Hi Ashesh, >>>>>> >>>>>> Can you please review the attached patch, and apply if you're happy >>>>>> with it? >>>>>> >>>>> Overall the patch looked good to me. >>>>> But - I encounter an issue in 'web' mode, which wont happen with >>>>> 'runtime'. >>>>> >>>>> Steps for reproduction on existing pgAdmin 4 environment with 'web' >>>>> mode. >>>>> - Apply the patch >>>>> - Start the pgAdmin4 application (stand alone application). >>>>> - Open pgAdmin home page. >>>>> - Log out (if already login). >>>>> >>>>> And, you will see an exception. >>>>> >>>>> I have figure out the issue with the patch. >>>>> We were setting the SECURITY_PASSWORD_SALT, after initializing the >>>>> Security object. >>>>> Hence - it could not set the SECURITY_KEY, and SECURITY_PASSWORD_SALT >>>>> properly. >>>>> >>>> >>>> Hmm. >>>> >>>> >>>>> >>>>> I had moved the Security object initialization after fetching these >>>>> configurations from the database. >>>>> I have attached a addon patch for the same. >>>>> >>>> >>>> OK, thanks. >>>> >>>> >>>>> >>>>> Now - I run into another issue. >>>>> Because - the existing password was hashed using the old >>>>> SECURITY_PASSWORD_SALT, I am no more able to login to pgAdmin 4. >>>>> >>>>> I think - we need to think about different strategy for upgrading the >>>>> configuration file in the 'web' mode. >>>>> I was thinking - we can store the existing security configurations in >>>>> the database during upgrade process in 'web' mode. >>>>> >>>> >>>> My concern with that is that we'll likely be storing the default config >>>> values in many cases, thus for those users, perpetuating the problem. >>>> >>>> I guess what we need to do is re-encrypt the password during the >>>> upgrade - however, that makes me think; we then have both the key and the >>>> encrypted passwords in the same database which is clearly not a good idea. >>>> Sigh... Needs more thought. >>>> >>> >>> OK, so I've been thinking about this and experimenting for a couple of >>> hours, as well as annoying the crap out of Magnus by thinking out loud in >>> his general direction, and it looks like this isn't a major problem as from >>> what I can see, SECURITY_PASSWORD_SALT is (aside from really being a key >>> not a salt) not the only salting that's done. >>> >>> It looks like it's used system-wide as the key to generate an HMAC of >>> the users password, which is then passed to passlib which salts and hashes >>> it. I did some testing, and found that two users with the same password end >>> up with different hashes in the database, so clearly there is also per-user >>> salting happening. I also created two users, then dropped the database and >>> created the same user accounts with the same passwords again, and found >>> that the resulting hashes were different in both databases - thus there is >>> something else ensuring the hashes are unique across different >>> installations/databases. >>> >>> So, I believe we can do as you suggest and migrate existing values for >>> SECURITY_PASSWORD_SALT, given that there's clearly some other per user and >>> per installation/database salting going on anyway. New installations can >>> have the random value for SECURITY_PASSWORD_SALT. >>> >> We do not need to generate the random SECURITY_PASSWORD_SALT during >> upgrade mode, which was wrong added in my addon patch. >> >> Please find the updated patch. >> >> Otherwise - looks good to me. >> Please commit the new patch (if you're ok with the change). >> >> >> -- >> >> Thanks & Regards, >> >> Ashesh Vashi >> EnterpriseDB INDIA: Enterprise PostgreSQL Company >> <http://www.enterprisedb.com/> >> >> >> *http://www.linkedin.com/in/asheshvashi* >> <http://www.linkedin.com/in/asheshvashi> >> >>> >>> I don't believe SECURITY_KEY and CSRF_SESSION_KEY are issues either, as >>> they're used for purposes that are essentially ephemeral, and thus can be >>> changed during an upgrade. >>> >>> Adding Magnus as I'd appreciate any thoughts he may have. >>> >>> Patch attached - please review (Ashesh, but others too would be >>> appreciated)! >>> >>> Thanks. >>> >>> >>> -- >>> Dave Page >>> Blog: http://pgsnake.blogspot.com >>> Twitter: @pgsnake >>> >>> EnterpriseDB UK: http://www.enterprisedb.com >>> The Enterprise PostgreSQL Company >>> >>> >> > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > -- Syed Fahar Abbas Quality Management Group EnterpriseDB Corporation Phone Office: +92-51-835-8874 Phone Direct: +92-51-8466803 Mobile: +92-333-5409707 Skype ID: syed.fahar.abbas Website: www.enterprisedb.com
