public inbox for [email protected]  
help / color / mirror / Atom feed
From: Murtuza Zabuawala <[email protected]>
To: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][RM#2813] Do not prompt for database server password once user saves it
Date: Fri, 10 Apr 2020 16:00:00 +0530
Message-ID: <CAKKotZR2YvOUC9WVkjzTDX1fNDtwSKXoMhc2hRMV7WiUm6QYUQ@mail.gmail.com> (raw)
In-Reply-To: <CANxoLDdz8cGiG-q5UE+b2kCowYvR2EB3016BkqPwy=QfpL=eMw@mail.gmail.com>
References: <CAKKotZQcSMRu-DN6rrzNs=8EAB5F1MSjQSFnvDSXvZsH-xpEkw@mail.gmail.com>
	<CANxoLDdz8cGiG-q5UE+b2kCowYvR2EB3016BkqPwy=QfpL=eMw@mail.gmail.com>

Hi Akshay,

Thanks for reviewing, PFA updated patch, it was a minor logical conditional
issue.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



On Fri, Apr 10, 2020 at 2:17 PM Akshay Joshi <[email protected]>
wrote:

> Hi Murtuza
>
> The issue is still reproducible when you add a new server with Save
> Password is checked. Disconnect the server and connect again, it ask for
> the Password.
> Please fix the above and resend the patch.
>
> On Thu, Apr 9, 2020 at 7:01 PM Murtuza Zabuawala <
> [email protected]> wrote:
>
>> Hi,
>>
>> There was an issue where the user used trust mode to connect to the
>> database server and saves a blank password, and next time when the user
>> tries to connect from pgadmin, pgadmin again asks for the password.
>>
>>
>> --
>> Regards,
>> Murtuza Zabuawala
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
>
> *Sr. Software Architect*
> *EnterpriseDB Software India Private Limited*
> *Mobile: +91 976-788-8246*
>


Attachments:

  [application/octet-stream] RM_2813_v1.diff (9.9K, 3-RM_2813_v1.diff)
  download | inline diff:
diff --git a/web/migrations/versions/d39482714a2e_.py b/web/migrations/versions/d39482714a2e_.py
new file mode 100644
index 000000000..66a51db87
--- /dev/null
+++ b/web/migrations/versions/d39482714a2e_.py
@@ -0,0 +1,47 @@
+##########################################################################
+#
+# pgAdmin 4 - PostgreSQL Tools
+#
+# Copyright (C) 2013 - 2020, The pgAdmin Development Team
+# This software is released under the PostgreSQL Licence
+#
+##########################################################################
+
+"""Add a column to save password option which will be useful when Trust mode
+
+Revision ID: d39482714a2e
+Revises: 7fedf8531802
+Create Date: 2020-04-09 13:20:13.939775
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from pgadmin.model import db
+
+# revision identifiers, used by Alembic.
+revision = 'd39482714a2e'
+down_revision = '7fedf8531802'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    db.engine.execute(
+        'ALTER TABLE server ADD COLUMN save_password INTEGER DEFAULT 0'
+    )
+    # If password is already exists for any existing server then change the
+    # save_password column to 1 (True) else set 0
+    db.engine.execute(
+        """
+        UPDATE server SET save_password = (
+            CASE WHEN password IS NOT NULL AND password != '' THEN
+                1
+            ELSE
+                0
+            END
+        )
+        """
+    )
+
+def downgrade():
+    pass
diff --git a/web/pgadmin/browser/server_groups/servers/__init__.py b/web/pgadmin/browser/server_groups/servers/__init__.py
index 1a9eb6a8f..1a70a5f2f 100644
--- a/web/pgadmin/browser/server_groups/servers/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/__init__.py
@@ -152,8 +152,7 @@ class ServerModule(sg.ServerGroupPluginModule):
                 user=manager.user_info if connected else None,
                 in_recovery=in_recovery,
                 wal_pause=wal_paused,
-                is_password_saved=True if server.password is not None
-                else False,
+                is_password_saved=bool(server.save_password),
                 is_tunnel_password_saved=True
                 if server.tunnel_password is not None else False,
                 was_connected=was_connected,
@@ -359,8 +358,7 @@ class ServerNode(PGChildNodeView):
                     user=manager.user_info if connected else None,
                     in_recovery=in_recovery,
                     wal_pause=wal_paused,
-                    is_password_saved=True if server.password is not None
-                    else False,
+                    is_password_saved=bool(server.save_password),
                     is_tunnel_password_saved=True
                     if server.tunnel_password is not None else False,
                     errmsg=errmsg
@@ -421,8 +419,7 @@ class ServerNode(PGChildNodeView):
                 user=manager.user_info if connected else None,
                 in_recovery=in_recovery,
                 wal_pause=wal_paused,
-                is_password_saved=True if server.password is not None
-                else False,
+                is_password_saved=bool(server.save_password),
                 is_tunnel_password_saved=True
                 if server.tunnel_password is not None else False,
                 errmsg=errmsg
@@ -768,6 +765,8 @@ class ServerNode(PGChildNodeView):
                 port=data.get('port'),
                 maintenance_db=data.get('db', None),
                 username=data.get('username'),
+                save_password=1 if data.get('save_password', False) and
+                config.ALLOW_SAVE_PASSWORD else 0,
                 ssl_mode=data.get('sslmode'),
                 comment=data.get('comment', None),
                 role=data.get('role', None),
@@ -1046,7 +1045,7 @@ class ServerNode(PGChildNodeView):
 
         if 'password' not in data:
             conn_passwd = getattr(conn, 'password', None)
-            if conn_passwd is None and server.password is None and \
+            if conn_passwd is None and not server.save_password and \
                     server.passfile is None and server.service is None:
                 prompt_password = True
             elif server.passfile and server.passfile != '':
@@ -1056,7 +1055,7 @@ class ServerNode(PGChildNodeView):
         else:
             password = data['password'] if 'password' in data else None
             save_password = data['save_password']\
-                if password and 'save_password' in data else False
+                if 'save_password' in data else False
 
             # Encrypt the password before saving with user's login
             # password key.
@@ -1102,9 +1101,15 @@ class ServerNode(PGChildNodeView):
         else:
             if save_password and config.ALLOW_SAVE_PASSWORD:
                 try:
+                    # If DB server is running in trust mode then password may
+                    # not be available but we don't need to ask password
+                    # every time user try to connect
+                    # 1 is True in SQLite as no boolean type
+                    setattr(server, 'save_password', 1)
                     # Save the encrypted password using the user's login
-                    # password key.
-                    setattr(server, 'password', password)
+                    # password key, if there is any password to save
+                    if password:
+                        setattr(server, 'password', password)
                     db.session.commit()
                 except Exception as e:
                     # Release Connection
@@ -1147,8 +1152,7 @@ class ServerNode(PGChildNodeView):
                     'user': manager.user_info,
                     'in_recovery': in_recovery,
                     'wal_pause': wal_paused,
-                    'is_password_saved': True if server.password is not None
-                    else False,
+                    'is_password_saved': bool(server.save_password),
                     'is_tunnel_password_saved': True
                     if server.tunnel_password is not None else False,
                 }
@@ -1552,6 +1556,10 @@ class ServerNode(PGChildNodeView):
                 )
 
             setattr(server, 'password', None)
+            # If password was saved then clear the flag also
+            # 0 is False in SQLite db
+            if server.save_password:
+                setattr(server, 'save_password', 0)
             db.session.commit()
         except Exception as e:
             current_app.logger.error(
diff --git a/web/pgadmin/browser/server_groups/servers/tests/test_server_add.py b/web/pgadmin/browser/server_groups/servers/tests/test_server_add.py
index fd554af94..7fe3cf6a8 100644
--- a/web/pgadmin/browser/server_groups/servers/tests/test_server_add.py
+++ b/web/pgadmin/browser/server_groups/servers/tests/test_server_add.py
@@ -8,7 +8,7 @@
 ##########################################################################
 
 import json
-
+import copy
 from pgadmin.utils.route import BaseTestGenerator
 from regression.python_test_utils import test_utils as utils
 
@@ -38,3 +38,49 @@ class ServersAddTestCase(BaseTestGenerator):
     def tearDown(self):
         """This function delete the server from SQLite """
         utils.delete_server_with_api(self.tester, self.server_id)
+
+
+class AddServersWithSavePasswordTestCase(BaseTestGenerator):
+    """ This class will add the servers under default server group. """
+
+    scenarios = [
+        # Fetch the default url for server object
+        ('Add server with password and save password to true',
+         dict(url='/browser/server/obj/', with_pwd=True, with_save=True)),
+        ('Add server with password and save password to false',
+         dict(url='/browser/server/obj/', with_pwd=True, with_save=False)),
+        ('Add server without password and save password to true',
+         dict(url='/browser/server/obj/', with_pwd=False, with_save=True)),
+    ]
+
+    def setUp(self):
+        pass
+
+    def runTest(self):
+        """ This function will add the server under default server group."""
+        url = "{0}{1}/".format(self.url, utils.SERVER_GROUP)
+        _server = copy.deepcopy(self.server)
+        # Update the flag as required
+        _server['save_password'] = self.with_save
+        if not self.with_pwd:
+            # Remove the password from server object
+            del _server['db_password']
+
+        response = self.tester.post(url, data=json.dumps(_server),
+                                    content_type='html/json')
+        self.assertEquals(response.status_code, 200)
+        response_data = json.loads(response.data.decode('utf-8'))
+        self.server_id = response_data['node']['_id']
+        server_dict = {"server_id": int(self.server_id)}
+        # Fetch the node info to check if password was saved or not
+        response = self.tester.get(self.url.replace('obj', 'nodes') +
+                                   str(utils.SERVER_GROUP) + '/' +
+                                   str(self.server_id),
+                                   follow_redirects=True)
+        self.assertEquals(response.status_code, 200)
+        self.assertTrue('is_password_saved' in response.json['result'])
+        utils.write_node_info("sid", server_dict)
+
+    def tearDown(self):
+        """This function delete the server from SQLite """
+        utils.delete_server_with_api(self.tester, self.server_id)
diff --git a/web/pgadmin/model/__init__.py b/web/pgadmin/model/__init__.py
index e3af660b0..ab9858326 100644
--- a/web/pgadmin/model/__init__.py
+++ b/web/pgadmin/model/__init__.py
@@ -119,6 +119,11 @@ class Server(db.Model):
     maintenance_db = db.Column(db.String(64), nullable=True)
     username = db.Column(db.String(64), nullable=False)
     password = db.Column(db.String(64), nullable=True)
+    save_password = db.Column(
+        db.Integer(),
+        db.CheckConstraint('save_password >= 0 AND save_password <= 1'),
+        nullable=False
+    )
     role = db.Column(db.String(64), nullable=True)
     ssl_mode = db.Column(
         db.String(16),


view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [pgAdmin4][RM#2813] Do not prompt for database server password once user saves it
  In-Reply-To: <CAKKotZR2YvOUC9WVkjzTDX1fNDtwSKXoMhc2hRMV7WiUm6QYUQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox