public inbox for [email protected]  
help / color / mirror / Atom feed
From: Murtuza Zabuawala <[email protected]>
To: pgadmin-hackers <[email protected]>
Subject: [pgAdmin4][RM#2813] Do not prompt for database server password once user saves it
Date: Thu, 9 Apr 2020 19:00:46 +0530
Message-ID: <CAKKotZQcSMRu-DN6rrzNs=8EAB5F1MSjQSFnvDSXvZsH-xpEkw@mail.gmail.com> (raw)

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


Attachments:

  [application/octet-stream] RM_2813.diff (10.4K, 3-RM_2813.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..0002f5627 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
@@ -841,6 +838,15 @@ class ServerNode(PGChildNodeView):
                             u"Unable to connect to server:\n\n%s" % errmsg)
                     )
                 else:
+                    # 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
+                    if 'save_password' in data and data['save_password'] and \
+                            config.ALLOW_SAVE_PASSWORD:
+                        # SQLite has no boolean type so 1 is True
+                        setattr(server, 'save_password', 1)
+                        db.session.commit()
+
                     if 'save_password' in data and data['save_password'] and \
                             have_password and config.ALLOW_SAVE_PASSWORD:
                         setattr(server, 'password', password)
@@ -1046,7 +1052,9 @@ 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 not server.save_password:
+                prompt_password = True
+            elif conn_passwd is None and server.password is None and \
                     server.passfile is None and server.service is None:
                 prompt_password = True
             elif server.passfile and server.passfile != '':
@@ -1056,7 +1064,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 +1110,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 +1161,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 +1565,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]
  Subject: Re: [pgAdmin4][RM#2813] Do not prompt for database server password once user saves it
  In-Reply-To: <CAKKotZQcSMRu-DN6rrzNs=8EAB5F1MSjQSFnvDSXvZsH-xpEkw@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