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
