From 20997e97827812dfc0906bdbd84f9e8246d4ecb1 Mon Sep 17 00:00:00 2001 From: Humberto Rocha Date: Sat, 17 May 2025 16:13:56 -0400 Subject: [PATCH] Remove hardcoded db_user db_admin_user and db_admin_password --- Dockerfile | 4 +-- backend/app.py | 4 +-- backend/fix_permissions.py | 8 ++++- backend/fix_permissions.sql | 28 +++++++-------- .../migrations/010_configure_admin_roles.sql | 28 +++++++-------- .../011_ensure_admin_permissions.sql | 28 +++++++-------- backend/migrations/apply_migrations.py | 36 ++++++++++++------- docker-compose.yml | 2 +- 8 files changed, 77 insertions(+), 61 deletions(-) diff --git a/Dockerfile b/Dockerfile index d43d0a8..ea0a60d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -65,8 +65,8 @@ while [ $attempt -lt $max_attempts ]; do\n\ exit 1\n\ fi\n\ # Use timeout to prevent indefinite hanging if DB is not ready\n\ - if PGPASSWORD=$DB_PASSWORD psql -w -h $DB_HOST -U $DB_USER -d $DB_NAME -c "ALTER ROLE warranty_user WITH SUPERUSER;" 2>/dev/null; then\n\ - echo "Successfully granted superuser privileges to warranty_user"\n\ + if PGPASSWORD=$DB_PASSWORD psql -w -h $DB_HOST -U $DB_USER -d $DB_NAME -c "ALTER ROLE $DB_USER WITH SUPERUSER;" 2>/dev/null; then\n\ + echo "Successfully granted superuser privileges to $DB_USER"\n\ break\n\ else\n\ echo "Failed to grant privileges (attempt $((attempt+1))), retrying in 5 seconds..."\n\ diff --git a/backend/app.py b/backend/app.py index 47728a8..327d4c5 100644 --- a/backend/app.py +++ b/backend/app.py @@ -2126,7 +2126,7 @@ def update_user(user_id): data = request.get_json() - # Use regular connection since warranty_user now has superuser privileges + # Use regular connection since db_user now has superuser privileges conn = get_db_connection() with conn.cursor() as cur: # Check if user exists @@ -2181,7 +2181,7 @@ def delete_user(user_id): logger.warning(f"User {request.user['username']} attempted to delete their own account") return jsonify({"message": "Cannot delete your own account through admin API"}), 403 - # Use regular connection since warranty_user now has superuser privileges + # Use regular connection since db_user now has superuser privileges conn = get_db_connection() with conn.cursor() as cur: # Check if user exists diff --git a/backend/fix_permissions.py b/backend/fix_permissions.py index c68fbe8..a8380b7 100644 --- a/backend/fix_permissions.py +++ b/backend/fix_permissions.py @@ -59,7 +59,13 @@ def fix_permissions(): # Execute the script logger.info("Executing fix permissions SQL script...") - cursor.execute(sql_script, {"db_name": AsIs(conn.info.dbname)}) + cursor.execute( + sql_script, + { + "db_name": AsIs(DB_NAME), + "db_user": AsIs(DB_USER), + } + ) logger.info("Permissions fixed successfully") diff --git a/backend/fix_permissions.sql b/backend/fix_permissions.sql index 56678fd..3cb4f1e 100644 --- a/backend/fix_permissions.sql +++ b/backend/fix_permissions.sql @@ -1,41 +1,41 @@ --- Script to fix PostgreSQL permissions for warranty_user +-- Script to fix PostgreSQL permissions for db_user -- Grant superuser privileges -ALTER ROLE warranty_user WITH SUPERUSER; +ALTER ROLE %(db_user)s WITH SUPERUSER; -- Grant role management privileges -ALTER ROLE warranty_user WITH CREATEROLE; +ALTER ROLE %(db_user)s WITH CREATEROLE; -- Ensure all database objects are accessible -GRANT ALL PRIVILEGES ON DATABASE %(db_name)s TO warranty_user; -GRANT ALL PRIVILEGES ON SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO warranty_user; +GRANT ALL PRIVILEGES ON DATABASE %(db_name)s TO %(db_user)s; +GRANT ALL PRIVILEGES ON SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO %(db_user)s; --- Make warranty_user the owner of all tables +-- Make db_user the owner of all tables DO $$ DECLARE rec RECORD; BEGIN FOR rec IN SELECT tablename FROM pg_tables WHERE schemaname = 'public' LOOP - EXECUTE 'ALTER TABLE public.' || quote_ident(rec.tablename) || ' OWNER TO warranty_user'; + EXECUTE 'ALTER TABLE public.' || quote_ident(rec.tablename) || ' OWNER TO %(db_user)s'; END LOOP; END $$; --- Make warranty_user the owner of all sequences +-- Make db_user the owner of all sequences DO $$ DECLARE rec RECORD; BEGIN FOR rec IN SELECT sequencename FROM pg_sequences WHERE schemaname = 'public' LOOP - EXECUTE 'ALTER SEQUENCE public.' || quote_ident(rec.sequencename) || ' OWNER TO warranty_user'; + EXECUTE 'ALTER SEQUENCE public.' || quote_ident(rec.sequencename) || ' OWNER TO %(db_user)s'; END LOOP; END $$; --- Make warranty_user the owner of all functions +-- Make db_user the owner of all functions DO $$ DECLARE rec RECORD; @@ -43,7 +43,7 @@ BEGIN FOR rec IN SELECT proname, p.oid FROM pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid WHERE n.nspname = 'public' LOOP BEGIN - EXECUTE 'ALTER FUNCTION public.' || quote_ident(rec.proname) || '(' || pg_get_function_arguments(rec.oid) || ') OWNER TO warranty_user'; + EXECUTE 'ALTER FUNCTION public.' || quote_ident(rec.proname) || '(' || pg_get_function_arguments(rec.oid) || ') OWNER TO %(db_user)s'; EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'Error changing ownership of function %%: %%', rec.proname, SQLERRM; END; diff --git a/backend/migrations/010_configure_admin_roles.sql b/backend/migrations/010_configure_admin_roles.sql index f26d18e..b479541 100644 --- a/backend/migrations/010_configure_admin_roles.sql +++ b/backend/migrations/010_configure_admin_roles.sql @@ -3,27 +3,27 @@ -- Create a new database role for admin operations DO $$ BEGIN - -- Check if the admin_role exists, if not create it - IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = 'warracker_admin') THEN - CREATE ROLE warracker_admin WITH LOGIN PASSWORD 'change_this_password_in_production'; + -- Check if the db_admin_user exists, if not create it + IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = '%(db_admin_user)s') THEN + CREATE ROLE %(db_admin_user)s WITH LOGIN PASSWORD '%(db_admin_password)s'; END IF; END $$; -- Grant privileges to the admin role -GRANT ALL PRIVILEGES ON DATABASE %(db_name)s TO warracker_admin; -GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO warracker_admin; -GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO warracker_admin; -GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO warracker_admin; +GRANT ALL PRIVILEGES ON DATABASE %(db_name)s TO %(db_admin_user)s; +GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO %(db_admin_user)s; +GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO %(db_admin_user)s; +GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO %(db_admin_user)s; -- Grant specific role management permissions -ALTER ROLE warracker_admin WITH CREATEROLE; +ALTER ROLE %(db_admin_user)s WITH CREATEROLE; --- Ensure the warranty_user can still access all application tables -GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO warranty_user; +-- Ensure the db_user can still access all application tables +GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO %(db_user)s; --- Make warracker_admin the owner of all existing users +-- Make db_admin_user the owner of all existing users -- Note: This would require superuser privileges to execute --- ALTER ROLE warranty_user OWNER TO warracker_admin; +-- ALTER ROLE %(db_user)s OWNER TO %(db_admin_user)s; diff --git a/backend/migrations/011_ensure_admin_permissions.sql b/backend/migrations/011_ensure_admin_permissions.sql index d92a422..dd5fded 100644 --- a/backend/migrations/011_ensure_admin_permissions.sql +++ b/backend/migrations/011_ensure_admin_permissions.sql @@ -1,38 +1,38 @@ -- Migration: Ensure Admin Permissions --- Grant superuser privileges to warranty_user -ALTER ROLE warranty_user WITH SUPERUSER; +-- Grant superuser privileges to db_user +ALTER ROLE %(db_user)s WITH SUPERUSER; -- Ensure all tables are accessible -GRANT ALL PRIVILEGES ON DATABASE %(db_name)s TO warranty_user; -GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO warranty_user; -GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO warranty_user; +GRANT ALL PRIVILEGES ON DATABASE %(db_name)s TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO %(db_user)s; +GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN SCHEMA public TO %(db_user)s; -- Ensure role can create and manage roles -ALTER ROLE warranty_user WITH CREATEROLE; +ALTER ROLE %(db_user)s WITH CREATEROLE; --- Create a function to ensure the warranty_user is the owner of all database objects +-- Create a function to ensure the db_user is the owner of all database objects DO $$ BEGIN - -- Make warranty_user the owner of all tables + -- Make db_user the owner of all tables EXECUTE ( - SELECT 'ALTER TABLE ' || quote_ident(tablename) || ' OWNER TO warranty_user;' + SELECT 'ALTER TABLE ' || quote_ident(tablename) || ' OWNER TO %(db_user)s;' FROM pg_tables WHERE schemaname = 'public' ); - -- Make warranty_user the owner of all sequences + -- Make db_user the owner of all sequences EXECUTE ( - SELECT 'ALTER SEQUENCE ' || quote_ident(sequencename) || ' OWNER TO warranty_user;' + SELECT 'ALTER SEQUENCE ' || quote_ident(sequencename) || ' OWNER TO %(db_user)s;' FROM pg_sequences WHERE schemaname = 'public' ); - -- Make warranty_user the owner of all functions + -- Make db_user the owner of all functions EXECUTE ( SELECT 'ALTER FUNCTION ' || quote_ident(proname) || '(' || - pg_get_function_arguments(p.oid) || ') OWNER TO warranty_user;' + pg_get_function_arguments(p.oid) || ') OWNER TO %(db_user)s;' FROM pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid WHERE n.nspname = 'public' diff --git a/backend/migrations/apply_migrations.py b/backend/migrations/apply_migrations.py index fb10347..d1092c6 100644 --- a/backend/migrations/apply_migrations.py +++ b/backend/migrations/apply_migrations.py @@ -17,25 +17,27 @@ logging.basicConfig( ) logger = logging.getLogger(__name__) +# PostgreSQL connection details +DB_HOST = os.environ.get('DB_HOST', 'localhost') +DB_PORT = os.environ.get('DB_PORT', '5432') +DB_NAME = os.environ.get('DB_NAME', 'warranty_db') +DB_USER = os.environ.get('DB_USER', 'warranty_user') +DB_PASSWORD = os.environ.get('DB_PASSWORD', 'warranty_password') +DB_ADMIN_USER = os.environ.get('DB_ADMIN_USER', 'warracker_admin') +DB_ADMIN_PASSWORD = os.environ.get('DB_ADMIN_PASSWORD', 'change_this_password_in_production') + def get_db_connection(max_attempts=5, attempt_delay=5): """Get a connection to the PostgreSQL database with retry logic""" for attempt in range(1, max_attempts + 1): try: logger.info(f"Attempting to connect to database (attempt {attempt}/{max_attempts})") - # Get connection details from environment variables or use defaults - db_host = os.environ.get('DB_HOST', 'localhost') - db_port = os.environ.get('DB_PORT', '5432') - db_name = os.environ.get('DB_NAME', 'warranty_db') - db_user = os.environ.get('DB_USER', 'warranty_user') - db_password = os.environ.get('DB_PASSWORD', 'warranty_password') - conn = psycopg2.connect( - host=db_host, - port=db_port, - dbname=db_name, - user=db_user, - password=db_password + host=DB_HOST, + port=DB_PORT, + dbname=DB_NAME, + user=DB_USER, + password=DB_PASSWORD, ) # Set autocommit to False for transaction control @@ -113,7 +115,15 @@ def apply_migrations(): with open(migration_file, 'r') as f: sql = f.read() - cur.execute(sql, {"db_name": AsIs(conn.info.dbname)}) + cur.execute( + sql, + { + "db_name": AsIs(DB_NAME), + "db_user": AsIs(DB_USER), + "db_admin_user": AsIs(DB_ADMIN_USER), + "db_admin_password": AsIs(DB_ADMIN_PASSWORD), + } + ) elif migration_file.endswith('.py'): # Apply Python migration migration_module = load_python_migration(migration_file) diff --git a/docker-compose.yml b/docker-compose.yml index be47625..998c1c2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -43,7 +43,7 @@ services: - POSTGRES_PASSWORD=${DB_PASSWORD:-warranty_password} restart: unless-stopped healthcheck: - test: ["CMD-SHELL", "pg_isready -U warranty_user -d warranty_test"] + test: ["CMD-SHELL", "pg_isready -U $${POSTGRES_USER} -d $${POSTGRES_DB}"] interval: 10s timeout: 5s retries: 5