Merge pull request #840 from MolSSI/pgpass

Better handling of passwords in init-db

qcarchivetesting/qcarchivetesting/config_files/gha_fractal_server.yaml

@@ -5,6 +5,8 @@ database:
   host: localhost
   logfile: qcfractal_postgres.log
   own: true
+  username: qcfractal
+  password: qcfractal1234
   port: 9876
 enable_security: false
 api:

qcarchivetesting/qcarchivetesting/config_files/qcf_basic.yaml

@@ -6,6 +6,8 @@ database:
   own: true
   port: 9987
   database_name: qcfractal_default
+  username: qcfractal
+  password: qcfractal1234
   logfile: qcfractal_postgres.log
 api:
   secret_key: THISISASECRETKEY 

qcarchivetesting/qcarchivetesting/migration_data/qcfractal_config_v0.15.8.yaml

@@ -5,9 +5,9 @@ database:
   host: localhost
   logfile: qcfractal_postgres.log
   own: true
-  password: null
+  password: qcfractal1234
   port: 5444
-  username: null
+  username: qcfractal
 fractal:
   allow_read: true
   compress_response: true

qcfractal/qcfractal/config.py

@@ -137,8 +137,8 @@ class DatabaseConfig(ConfigBase):
         description="The port the database is running on. If own = True, a database will be started, binding to this port",
     )
     database_name: str = Field("qcfractal_default", description="The database name to connect to.")
-    username: Optional[str] = Field(None, description="The database username to connect with")
-    password: Optional[str] = Field(None, description="The database password to connect with")
+    username: str = Field(..., description="The database username to connect with")
+    password: str = Field(..., description="The database password to connect with")
     query: Dict[str, str] = Field({}, description="Extra connection query parameters at the end of the URL string")
 
     own: bool = Field(
@@ -561,8 +561,13 @@ def write_initial_configuration(file_path: str, full_config: bool = True):
     secret_key = secrets.token_urlsafe(32)
     jwt_secret_key = secrets.token_urlsafe(32)
 
+    db_config = {
+        "username": "qcfractal",
+        "password": secrets.token_urlsafe(32),
+    }
+
     default_config = FractalConfig(
-        base_folder=base_folder, api={"secret_key": secret_key, "jwt_secret_key": jwt_secret_key}
+        base_folder=base_folder, api={"secret_key": secret_key, "jwt_secret_key": jwt_secret_key}, database=db_config
     )
 
     default_config.database.port = find_open_port(5432)
@@ -580,7 +585,7 @@ def write_initial_configuration(file_path: str, full_config: bool = True):
             "service_frequency": True,
             "max_active_services": True,
             "heartbeat_frequency": True,
-            "database": {"own", "host", "port", "database_name", "base_folder"},
+            "database": {"own", "host", "port", "database_name", "base_folder", "username", "password"},
             "api": {"secret_key", "jwt_secret_key", "host", "port"},
         }
 

qcfractal/qcfractal/postgres_harness.py

@@ -4,6 +4,7 @@
 import os
 import pathlib
 import re
+import secrets
 import shutil
 import subprocess
 import time
@@ -123,7 +124,9 @@ def _get_tool(self, tool: str) -> str:
             )
         return tool_path
 
-    def _run_subprocess(self, command: List[str], env: Optional[Dict[str, Any]] = None) -> Tuple[int, str, str]:
+    def _run_subprocess(
+        self, command: List[str], env: Optional[Dict[str, Any]] = None, shell: bool = False
+    ) -> Tuple[int, str, str]:
         """
         Runs a command using subprocess, and output stdout into the logger
 
@@ -143,8 +146,19 @@ def _run_subprocess(self, command: List[str], env: Optional[Dict[str, Any]] = No
         if env is not None:
             full_env.update(env)
 
-        proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=full_env)
         self._logger.debug("Running subprocess: " + str(command))
+        if shell:
+            proc = subprocess.run(
+                " ".join(command),
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+                env=full_env,
+                shell=True,
+                executable="/bin/bash",
+            )
+        else:
+            proc = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=full_env)
+
         stdout = proc.stdout.decode()
         stderr = proc.stderr.decode()
         if len(stdout) > 0:
@@ -454,27 +468,19 @@ def postgres_initialized(self):
         psql_conf_file = os.path.join(self.config.data_directory, "postgresql.conf")
         return os.path.exists(psql_conf_file)
 
-    def initialize_postgres(self, auth_method: str = "trust") -> None:
+    def initialize_postgres(self) -> None:
         """Initializes a postgresql instance and starts it
 
         The data directory and port from the configuration is used for the postgres instance
 
         This does not create the QCFractal database or its tables
-
-
-        Parameters
-        ----------
-        auth_method
-            [ADVANCED] The authentication method to use for host and local connections.
-            The default is "trust" which is insecure but is easy for local installations.
         """
 
         # Can only initialize if we are expected to manage it
         assert self.config.own
 
-        # auth method must be 'trust' if user/password not given
-        if auth_method != "trust" and (self.config.username is None or self.config.password is None):
-            raise RuntimeError("Cannot use auth_method other than 'trust' if username or password are not given")
+        if not self.config.username or not self.config.password:
+            raise RuntimeError("Username or password are not given")
 
         self._logger.info("Initializing the Postgresql database")
 
@@ -489,22 +495,15 @@ def initialize_postgres(self, auth_method: str = "trust") -> None:
 
         initdb_path = self._get_tool("initdb")
 
-        cmd = [initdb_path, "-D", self.config.data_directory, "--auth", auth_method]
+        cmd = [initdb_path, "-D", self.config.data_directory, "--auth", "scram-sha-256"]
         if self.config.username is not None:
             cmd += ["--username", self.config.username]
 
         # Initdb requires passwords come from a file
-        if self.config.password is not None:
-            pw_file_path = os.path.join(self.config.base_folder, ".initdb_pwfile")
-            with open(pw_file_path, "w") as pw_tmp:
-                pw_tmp.write(self.config.password)
-            cmd += ["--pwfile", pw_file_path]
+        env = {"PG_SUPER_PASSWORD": self.config.password}
+        cmd += ['--pwfile=<(printf "%s\n" ${PG_SUPER_PASSWORD})']
 
-        try:
-            retcode, stdout, stderr = self._run_subprocess(cmd)
-        finally:
-            if self.config.password is not None:
-                os.remove(pw_file_path)
+        retcode, stdout, stderr = self._run_subprocess(cmd, env=env, shell=True)
 
         if retcode != 0 or "Success." not in stdout:
             err_msg = f"Error initializing a PostgreSQL instance:\noutput:\n{stdout}\nstderr:\n{stderr}"
@@ -617,6 +616,8 @@ def create_snowflake_postgres(data_dir: str) -> PostgresHarness:
         "base_folder": data_dir,
         "own": True,
         "host": sock_dir,
+        "username": "qcfractal_snowflake",
+        "password": secrets.token_urlsafe(32),
     }
 
     config = DatabaseConfig(**db_config)

qcfractal/qcfractal/test_db_connection.py

@@ -21,8 +21,15 @@
             "testing_db_maint_1",
             {"connect_timeout": 10},
         ),
-        ("192.168.1.234", 6543, "test_user_1", None, "testing_db_2", "testing_db_maint_2", {"connect_timeout": 10}),
-        ("db.example.com", 9988, None, None, "testing_db_3", "testing_db_maint_3", {"connect_timeout": 10}),
+        (
+            "192.168.1.234",
+            6543,
+            "test_user_1",
+            "test_pass_2",
+            "testing_db_2",
+            "testing_db_maint_2",
+            {"connect_timeout": 10},
+        ),
         (
             "/var/run/postgresql",
             9876,
@@ -142,7 +149,7 @@ def test_db_connection_hosts(tmp_path_factory):
     pg_harness = PostgresHarness(db_config)
 
     # Change trust method so we can actually check passwords
-    pg_harness.initialize_postgres(auth_method="scram-sha-256")
+    pg_harness.initialize_postgres()
     pg_harness.create_database(create_tables=True)
     assert pg_harness.can_connect()
 
@@ -178,7 +185,7 @@ def test_db_connection_full_uri(tmp_path_factory):
     pg_harness = PostgresHarness(db_config)
 
     # Change trust method so we can actually check passwords
-    pg_harness.initialize_postgres(auth_method="scram-sha-256")
+    pg_harness.initialize_postgres()
     pg_harness.create_database(create_tables=True)
     assert pg_harness.can_connect()