From bb7539dcb6271d0bfa2cab382d21aa473947d255 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Sat, 21 Aug 2021 00:34:10 -0400 Subject: [PATCH 01/18] wip: db api endpoints --- .gitignore | 1 + ceo_common/errors.py | 4 ++ ceo_common/interfaces/IDatabaseService.py | 14 ++++++ ceod/api/app_factory.py | 11 +++- ceod/api/database.py | 42 ++++++++++++++++ ceod/db/MySQLService.py | 47 +++++++++++++++++ ceod/db/PostgreSQLService.py | 61 +++++++++++++++++++++++ ceod/db/__init__.py | 2 + requirements.txt | 1 + 9 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 ceo_common/interfaces/IDatabaseService.py create mode 100644 ceod/api/database.py create mode 100644 ceod/db/MySQLService.py create mode 100644 ceod/db/PostgreSQLService.py create mode 100644 ceod/db/__init__.py diff --git a/.gitignore b/.gitignore index b41a4d6..170ae47 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ __pycache__/ *.o *.so /ceo_common/krb5/_krb5.c +.idea/ diff --git a/ceo_common/errors.py b/ceo_common/errors.py index 7d517d3..2a13412 100644 --- a/ceo_common/errors.py +++ b/ceo_common/errors.py @@ -49,3 +49,7 @@ class UserNotSubscribedError(Exception): class NoSuchListError(Exception): def __init__(self): super().__init__('mailing list does not exist') + + +class DatabaseConnectionError(Exception): + pass diff --git a/ceo_common/interfaces/IDatabaseService.py b/ceo_common/interfaces/IDatabaseService.py new file mode 100644 index 0000000..191344e --- /dev/null +++ b/ceo_common/interfaces/IDatabaseService.py @@ -0,0 +1,14 @@ +from zope.interface import Attribute, Interface +from .IUser import IUser + + +class IDatabaseService(Interface): + """Interface to create databases for users.""" + + type = Attribute('the type of database') + host = Attribute('the database address') + auth_username = Attribute('username of user creating connection') + auth_password = Attribute('password of user creating connection') + + def create_db(username: str) -> str: + """create a database for user and return its password""" diff --git a/ceod/api/app_factory.py b/ceod/api/app_factory.py index a7bc66d..0e2267e 100644 --- a/ceod/api/app_factory.py +++ b/ceod/api/app_factory.py @@ -9,10 +9,11 @@ from zope import component from .error_handlers import register_error_handlers from .krb5_cred_handlers import before_request, teardown_request from ceo_common.interfaces import IConfig, IKerberosService, ILDAPService, IFileService, \ - IMailmanService, IMailService, IUWLDAPService, IHTTPClient + IMailmanService, IMailService, IUWLDAPService, IHTTPClient, IDatabaseService from ceo_common.model import Config, HTTPClient, RemoteMailmanService from ceod.model import KerberosService, LDAPService, FileService, \ MailmanService, MailService, UWLDAPService +from ceod.db import MySQLService, PostgreSQLService def create_app(flask_config={}): @@ -107,3 +108,11 @@ def register_services(app): # UWLDAPService uwldap_srv = UWLDAPService() component.provideUtility(uwldap_srv, IUWLDAPService) + + # MySQLService + mysql_srv = MySQLService() + component.provideUtility(mysql_srv, IDatabaseService, 'mysql') + + # PostgreSQLService + psql_srv = PostgreSQLService() + component.provideUtility(psql_srv, IDatabaseService, 'postgresql') diff --git a/ceod/api/database.py b/ceod/api/database.py new file mode 100644 index 0000000..f0c2444 --- /dev/null +++ b/ceod/api/database.py @@ -0,0 +1,42 @@ +from flask import Blueprint, request +from zope import component +from ceod.api.utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ + user_is_in_group, requires_authentication_no_realm, \ + create_streaming_response, create_sync_response, development_only +from ceo_common.errors import UserNotFoundError, DatabaseConnectionError +from ceo_common.interfaces import IDatabaseService + + +bp = Blueprint('db', __name__) + +# could combine create_mysql_db and create_postgresql_db into one function + + +@bp.route('/mysql/', methods=['POST']) +@requires_authentication_no_realm +def create_mysql_db(auth_user: str, username: str): + if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): + return {'error': "not authorized to create databases for others"}, 403 + try: + db_srv = component.getUtility(IDatabaseService, 'mysql') + password = db_srv.create_db(username) + return {'password': password} + except UserNotFoundError: + return {'error': 'user not found'}, 404 + except DatabaseConnectionError: + return {'error': 'unable to connect to mysql server'}, 400 + + +@bp.route('/postgresql/', methods=['POST']) +@requires_authentication_no_realm +def create_postgresql_db(auth_user: str, username: str): + if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): + return {'error': "not authorized to create databases for others"}, 403 + try: + db_srv = component.getUtility(IDatabaseService, 'postgresql') + password = db_srv.create_db(username) + return {'password': password} + except UserNotFoundError: + return {'error': 'user not found'}, 404 + except DatabaseConnectionError: + return {'error': 'unable to connect to postgresql server'}, 400 diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py new file mode 100644 index 0000000..8baf1b7 --- /dev/null +++ b/ceod/db/MySQLService.py @@ -0,0 +1,47 @@ +from zope.interface import implementer +from zope import component +from ceo_common.interfaces import IDatabaseService, ILDAPService, IConfig +from ceo_common.errors import DatabaseConnectionError +from mysql.connector import connect, Error +import ceod.utils as utils + + +@implementer(IDatabaseService) +class MySQLService: + def __init__(self): + # how to set default values for these + self.type = 'mysql' + config = component.getUtility(IConfig) + self.host = config.get('mysql_host') + self.auth_username = config.get('mysql_username') + self.auth_password = config.get('mysql_password') + + def create_db(self, username: str) -> str: + component.getUtility(ILDAPService).get_user(username) # make sure user exists + password = utils.gen_password() + user = {'username': username, 'password': password} + try: + with connect( + host=self.host, + user=self.auth_username, + password=self.auth_password, + ) as con: + search_for_user = "SELECT user FROM mysql.user WHERE user='%(username)s'" + create_user = "CREATE USER '%(username)s'@'localhost' IDENTIFIED BY '%(password)s'" + create_database = "CREATE DATABASE %(username)s" + set_user_perms = "GRANT ALL PRIVILEGES ON %(username)s.* TO '%(username)s'@'localhost'" + flush_privileges = "FLUSH PRIVILEGES" + reset_password = "ALTER USER '%(username)s' IDENTIFIED BY '%(password)s'" + with con.cursor() as cursor: + cursor.execute(search_for_user, user) + response = cursor.fetchall() + if len(response) == 0: + cursor.execute(create_user, user) + cursor.execute(create_database, user) + cursor.execute(set_user_perms, user) + cursor.execute(flush_privileges) + else: + cursor.execute(reset_password, user) + return password + except Error: + raise DatabaseConnectionError() diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py new file mode 100644 index 0000000..1078c80 --- /dev/null +++ b/ceod/db/PostgreSQLService.py @@ -0,0 +1,61 @@ +from zope.interface import implementer +from zope import component +from ceo_common.interfaces import IDatabaseService, ILDAPService, IConfig +from ceo_common.errors import DatabaseConnectionError +import ceod.utils as utils +from psycopg2 import connect, Error + + +@implementer(IDatabaseService) +class PostgreSQLService: + def __init__(self): + self.type = 'postgresql' + config = component.getUtility(IConfig) + self.host = config.get('postgresql_host') + self.auth_username = config.get('postgresql_username') + self.auth_password = config.get('postgresql_password') + + # https://www.postgresql.org/docs/9.1/auth-pg-hba-conf.html + # pg_hba.conf only listen to localhost and only allow users to login to database with the same name as user + # local sameuser all localhost md5 + # need different line for syscom + + # Allow only postgres to create on the schema public + # REVOKE ALL ON SCHEMA public FROM PUBLIC; + # GRANT ALL ON SCHEMA public TO postgres; + + # by default all database created are open to connection from anyone + # only the owner (and superusers) can ever drop a database + + # note that pg_catalog allows access list of database and user names for everyone and cannot be disabled with breaking some things + + def create_db(self, username: str) -> str: + component.getUtility(ILDAPService).get_user(username) # make sure user exists + password = utils.gen_password() + user = {'username': username, 'password': password} + try: + with connect( + host=self.host, + user=self.auth_username, + password=self.auth_password, + ) as con: + # limit access to localhost? + search_for_user = "SELECT FROM pg_roles WHERE rolname='%(username)s'" + create_user = "CREATE USER %(username)s WITH NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '%(password)s'" + create_database = "CREATE DATABASE %(username)s" + set_db_perms = "REVOKE ALL ON DATABASE %(username)s FROM PUBLIC" + set_user_perms = "GRANT ALL ON DATABASE %(username)s TO %(username)s" + reset_password = "ALTER USER '%(username)s' WITH PASSWORD '%(password)s'" + with con.cursor() as cursor: + cursor.execute(search_for_user, user) + response = cursor.fetchall() + if len(response) == 0: + cursor.execute(create_user, user) + cursor.execute(create_database, user) + cursor.execute(set_db_perms, user) + cursor.execute(set_user_perms, user) + else: + cursor.execute(reset_password, user) + return password + except Error: + raise DatabaseConnectionError() diff --git a/ceod/db/__init__.py b/ceod/db/__init__.py new file mode 100644 index 0000000..b5373bd --- /dev/null +++ b/ceod/db/__init__.py @@ -0,0 +1,2 @@ +from .MySQLService import MySQLService +from .PostgreSQLService import PostgreSQLService \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index 402fafc..007aa66 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,4 @@ requests==2.26.0 requests-gssapi==1.2.3 zope.component==5.0.1 zope.interface==5.4.0 +mysql-connector-python==8.0.26 \ No newline at end of file -- 2.39.2 From 10f7aab3edcc8833c937dfa6c1c2ef6a05644423 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Sat, 21 Aug 2021 02:00:26 -0400 Subject: [PATCH 02/18] cleaning up --- README.md | 39 +++++++++++++++++++++++++++++++++++- ceod/api/database.py | 4 +++- ceod/db/MySQLService.py | 1 - ceod/db/PostgreSQLService.py | 16 +-------------- requirements.txt | 3 ++- 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index b286421..3315dd0 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,10 @@ +### TODO before merge +- testing and tests +- need someone to test isolation of PostgreSQL users +- handling for improper (missing values, unable to connect) config +- make MySQLService and PostgreSQLService look nicer to read +- check database.py + # pyceo [![Build Status](https://ci.csclub.uwaterloo.ca/api/badges/public/pyceo/status.svg?ref=refs/heads/v1)](https://ci.csclub.uwaterloo.ca/public/pyceo) @@ -33,7 +40,37 @@ On phosphoric-acid, you will additionally need to create a principal called `ceod/admin` (remember to addprinc **and** ktadd). #### Database -TODO - Andrew +Edit the `/etc/csc/ceod.ini` with the credentials required to access MySQL and PostgreSQL +``` +[mysql] +host = +username = +password = + +[postgresql] +host = +usrename = +password = +``` +#### PostgreSQL Database +PostgreSQL is not designed for isolation of users and by default will allow any user to connect and edit any database. To disallow users to create public schema we run +``` +su postgres +psql + +REVOKE ALL ON SCHEMA public FROM public; +GRANT ALL ON SCHEMA public TO postgres; +``` +We also want to change `pg_hba.conf` to only allow local connections and force the requested database to have the same name as the user creating the connection ([more info](https://www.postgresql.org/docs/9.1/auth-pg-hba-conf.html)) +``` +# TYPE DATABASE USER ADDRESS METHOD +local all postgres peer +local sameuser all md5 +``` +- peer authentication only requires that your os username matches the postgres username (no password) +- Users will have access to list of databases and users, and this cannot be disabled without possible issues ([more info](https://wiki.postgresql.org/wiki/Shared_Database_Hosting#template1)) +- [Managing rights in PostgreSQL](https://wiki.postgresql.org/images/d/d1/Managing_rights_in_postgresql.pdf) + #### Dependencies Next, install and activate a virtualenv: diff --git a/ceod/api/database.py b/ceod/api/database.py index f0c2444..2ed64e9 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -2,7 +2,7 @@ from flask import Blueprint, request from zope import component from ceod.api.utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ user_is_in_group, requires_authentication_no_realm, \ - create_streaming_response, create_sync_response, development_only + create_streaming_response, development_only from ceo_common.errors import UserNotFoundError, DatabaseConnectionError from ceo_common.interfaces import IDatabaseService @@ -10,6 +10,8 @@ from ceo_common.interfaces import IDatabaseService bp = Blueprint('db', __name__) # could combine create_mysql_db and create_postgresql_db into one function +# catch other less expected errors (mysql or psql error) +# handle if user somehow dropped their database @bp.route('/mysql/', methods=['POST']) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 8baf1b7..2d8aac0 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -9,7 +9,6 @@ import ceod.utils as utils @implementer(IDatabaseService) class MySQLService: def __init__(self): - # how to set default values for these self.type = 'mysql' config = component.getUtility(IConfig) self.host = config.get('mysql_host') diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 1078c80..4c198ff 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -15,20 +15,6 @@ class PostgreSQLService: self.auth_username = config.get('postgresql_username') self.auth_password = config.get('postgresql_password') - # https://www.postgresql.org/docs/9.1/auth-pg-hba-conf.html - # pg_hba.conf only listen to localhost and only allow users to login to database with the same name as user - # local sameuser all localhost md5 - # need different line for syscom - - # Allow only postgres to create on the schema public - # REVOKE ALL ON SCHEMA public FROM PUBLIC; - # GRANT ALL ON SCHEMA public TO postgres; - - # by default all database created are open to connection from anyone - # only the owner (and superusers) can ever drop a database - - # note that pg_catalog allows access list of database and user names for everyone and cannot be disabled with breaking some things - def create_db(self, username: str) -> str: component.getUtility(ILDAPService).get_user(username) # make sure user exists password = utils.gen_password() @@ -39,7 +25,7 @@ class PostgreSQLService: user=self.auth_username, password=self.auth_password, ) as con: - # limit access to localhost? + # only the owner (and superusers) can ever drop a database search_for_user = "SELECT FROM pg_roles WHERE rolname='%(username)s'" create_user = "CREATE USER %(username)s WITH NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '%(password)s'" create_database = "CREATE DATABASE %(username)s" diff --git a/requirements.txt b/requirements.txt index 007aa66..f3ed87b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,4 +7,5 @@ requests==2.26.0 requests-gssapi==1.2.3 zope.component==5.0.1 zope.interface==5.4.0 -mysql-connector-python==8.0.26 \ No newline at end of file +mysql-connector-python==8.0.26 +psycopg2==2.9.1 \ No newline at end of file -- 2.39.2 From af87d6a7e63e0c7713a99613aa012528acc01d75 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Tue, 24 Aug 2021 02:14:28 -0400 Subject: [PATCH 03/18] fixes to logic --- README.md | 3 -- ceo_common/errors.py | 8 +++- ceo_common/interfaces/IDatabaseService.py | 10 ++--- ceod/api/database.py | 43 +++++++++--------- ceod/db/MySQLService.py | 46 ++++++++++--------- ceod/db/PostgreSQLService.py | 54 ++++++++++++----------- ceod/db/utils.py | 5 +++ 7 files changed, 91 insertions(+), 78 deletions(-) create mode 100644 ceod/db/utils.py diff --git a/README.md b/README.md index 3315dd0..63a95e0 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,6 @@ ### TODO before merge - testing and tests - need someone to test isolation of PostgreSQL users -- handling for improper (missing values, unable to connect) config -- make MySQLService and PostgreSQLService look nicer to read -- check database.py # pyceo [![Build Status](https://ci.csclub.uwaterloo.ca/api/badges/public/pyceo/status.svg?ref=refs/heads/v1)](https://ci.csclub.uwaterloo.ca/public/pyceo) diff --git a/ceo_common/errors.py b/ceo_common/errors.py index 2a13412..7bd88cd 100644 --- a/ceo_common/errors.py +++ b/ceo_common/errors.py @@ -52,4 +52,10 @@ class NoSuchListError(Exception): class DatabaseConnectionError(Exception): - pass + def __init__(self): + super().__init__('unable to connect or authenticate to sql service') + + +class DatabasePermissionError(Exception): + def __init__(self): + super().__init__('unable to perform action due to lack of permissions') diff --git a/ceo_common/interfaces/IDatabaseService.py b/ceo_common/interfaces/IDatabaseService.py index 191344e..f43f1ea 100644 --- a/ceo_common/interfaces/IDatabaseService.py +++ b/ceo_common/interfaces/IDatabaseService.py @@ -5,10 +5,10 @@ from .IUser import IUser class IDatabaseService(Interface): """Interface to create databases for users.""" - type = Attribute('the type of database') - host = Attribute('the database address') - auth_username = Attribute('username of user creating connection') - auth_password = Attribute('password of user creating connection') + type = Attribute('the type of databases that will be created') + host = Attribute('the database host') + auth_username = Attribute('username to a privileged user on the database host') + auth_password = Attribute('password to a privileged user on the database host') def create_db(username: str) -> str: - """create a database for user and return its password""" + """try to create a database and user and return its password""" diff --git a/ceod/api/database.py b/ceod/api/database.py index 2ed64e9..f7695a1 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -3,15 +3,30 @@ from zope import component from ceod.api.utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ user_is_in_group, requires_authentication_no_realm, \ create_streaming_response, development_only -from ceo_common.errors import UserNotFoundError, DatabaseConnectionError -from ceo_common.interfaces import IDatabaseService +from ceo_common.errors import UserNotFoundError, DatabaseConnectionError, DatabasePermissionError +from ceo_common.interfaces import ILDAPService, IDatabaseService bp = Blueprint('db', __name__) -# could combine create_mysql_db and create_postgresql_db into one function -# catch other less expected errors (mysql or psql error) -# handle if user somehow dropped their database + +def create_db_from_type(db_type: str, username: str): + try: + if not username.isalnum(): # username should not contain symbols + raise UserNotFoundError + ldap_srv = component.getUtility(ILDAPService) + ldap_srv.get_user(username) # make sure user exists + db_srv = component.getUtility(IDatabaseService, db_type) + password = db_srv.create_db(username) + return {'password': password} + except UserNotFoundError: + return {'error': 'user not found'}, 404 + except DatabaseConnectionError: + return {'error': 'unable to connect or authenticate to sql server'}, 400 + except DatabasePermissionError: + return {'error': 'unable to perform action due to permissions'}, 502 + except: + return {'error': 'Unexpected error'}, 500 @bp.route('/mysql/', methods=['POST']) @@ -19,14 +34,7 @@ bp = Blueprint('db', __name__) def create_mysql_db(auth_user: str, username: str): if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): return {'error': "not authorized to create databases for others"}, 403 - try: - db_srv = component.getUtility(IDatabaseService, 'mysql') - password = db_srv.create_db(username) - return {'password': password} - except UserNotFoundError: - return {'error': 'user not found'}, 404 - except DatabaseConnectionError: - return {'error': 'unable to connect to mysql server'}, 400 + return create_db_from_type('mysql', username) @bp.route('/postgresql/', methods=['POST']) @@ -34,11 +42,4 @@ def create_mysql_db(auth_user: str, username: str): def create_postgresql_db(auth_user: str, username: str): if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): return {'error': "not authorized to create databases for others"}, 403 - try: - db_srv = component.getUtility(IDatabaseService, 'postgresql') - password = db_srv.create_db(username) - return {'password': password} - except UserNotFoundError: - return {'error': 'user not found'}, 404 - except DatabaseConnectionError: - return {'error': 'unable to connect to postgresql server'}, 400 + return create_db_from_type('postgresql', username) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 2d8aac0..9cc9ee5 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -1,9 +1,11 @@ from zope.interface import implementer from zope import component -from ceo_common.interfaces import IDatabaseService, ILDAPService, IConfig -from ceo_common.errors import DatabaseConnectionError -from mysql.connector import connect, Error -import ceod.utils as utils +from ceo_common.interfaces import IDatabaseService, IConfig +from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError +from ceod.utils import gen_password +from ceod.db.utils import response_is_empty +from mysql.connector import connect +from mysql.connector.errors import InterfaceError, ProgrammingError @implementer(IDatabaseService) @@ -16,31 +18,31 @@ class MySQLService: self.auth_password = config.get('mysql_password') def create_db(self, username: str) -> str: - component.getUtility(ILDAPService).get_user(username) # make sure user exists - password = utils.gen_password() - user = {'username': username, 'password': password} + password = gen_password() + search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" + search_for_db = f"SHOW DATABASES LIKE '{username}'" + create_user = f"CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s" + reset_password = f"ALTER USER '{username}' IDENTIFIED BY %(password)s" + create_database = f""" + CREATE DATABASE {username}; + GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'localhost'; + """ try: with connect( host=self.host, user=self.auth_username, password=self.auth_password, ) as con: - search_for_user = "SELECT user FROM mysql.user WHERE user='%(username)s'" - create_user = "CREATE USER '%(username)s'@'localhost' IDENTIFIED BY '%(password)s'" - create_database = "CREATE DATABASE %(username)s" - set_user_perms = "GRANT ALL PRIVILEGES ON %(username)s.* TO '%(username)s'@'localhost'" - flush_privileges = "FLUSH PRIVILEGES" - reset_password = "ALTER USER '%(username)s' IDENTIFIED BY '%(password)s'" with con.cursor() as cursor: - cursor.execute(search_for_user, user) - response = cursor.fetchall() - if len(response) == 0: - cursor.execute(create_user, user) - cursor.execute(create_database, user) - cursor.execute(set_user_perms, user) - cursor.execute(flush_privileges) + if response_is_empty(search_for_user, con): + cursor.execute(create_user, {password: password}) else: - cursor.execute(reset_password, user) + cursor.execute(reset_password, {password: password}) + if response_is_empty(search_for_db, con): + cursor.execute(create_database) + con.commit() return password - except Error: + except InterfaceError: raise DatabaseConnectionError() + except ProgrammingError: + raise DatabasePermissionError() diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 4c198ff..ba0ba64 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -1,9 +1,11 @@ from zope.interface import implementer from zope import component -from ceo_common.interfaces import IDatabaseService, ILDAPService, IConfig -from ceo_common.errors import DatabaseConnectionError -import ceod.utils as utils -from psycopg2 import connect, Error +from ceo_common.interfaces import IDatabaseService, IConfig +from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError +from ceod.utils import gen_password +from ceod.db.utils import response_is_empty +from psycopg2 import connect, OperationalError, ProgrammingError +from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT @implementer(IDatabaseService) @@ -16,32 +18,32 @@ class PostgreSQLService: self.auth_password = config.get('postgresql_password') def create_db(self, username: str) -> str: - component.getUtility(ILDAPService).get_user(username) # make sure user exists - password = utils.gen_password() - user = {'username': username, 'password': password} + password = gen_password() + search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" + search_for_db = f"SELECT FROM pg_database WHERE datname='{username}'" + create_user = f"CREATE USER {username} WITH PASSWORD %(password)s" + reset_password = f"ALTER USER {username} WITH PASSWORD %(password)s)" + create_database = f""" + CREATE DATABASE {username} OWNER {username}; + REVOKE ALL ON DATABASE {username} FROM PUBLIC; + """ try: with connect( - host=self.host, - user=self.auth_username, - password=self.auth_password, + host=self.host, + user=self.auth_username, + password=self.auth_password, ) as con: - # only the owner (and superusers) can ever drop a database - search_for_user = "SELECT FROM pg_roles WHERE rolname='%(username)s'" - create_user = "CREATE USER %(username)s WITH NOSUPERUSER NOCREATEDB NOCREATEROLE PASSWORD '%(password)s'" - create_database = "CREATE DATABASE %(username)s" - set_db_perms = "REVOKE ALL ON DATABASE %(username)s FROM PUBLIC" - set_user_perms = "GRANT ALL ON DATABASE %(username)s TO %(username)s" - reset_password = "ALTER USER '%(username)s' WITH PASSWORD '%(password)s'" + con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) with con.cursor() as cursor: - cursor.execute(search_for_user, user) - response = cursor.fetchall() - if len(response) == 0: - cursor.execute(create_user, user) - cursor.execute(create_database, user) - cursor.execute(set_db_perms, user) - cursor.execute(set_user_perms, user) + if response_is_empty(search_for_user, con): + cursor.execute(create_user, {password: password}) else: - cursor.execute(reset_password, user) + cursor.execute(reset_password, {password: password}) + if response_is_empty(search_for_db, con): + cursor.execute(create_database) return password - except Error: + except OperationalError: raise DatabaseConnectionError() + except ProgrammingError: + raise DatabasePermissionError() + diff --git a/ceod/db/utils.py b/ceod/db/utils.py new file mode 100644 index 0000000..bbac214 --- /dev/null +++ b/ceod/db/utils.py @@ -0,0 +1,5 @@ +def response_is_empty(query: str, connection) -> bool: + with connection.cursor() as cursor: + cursor.execute(query) + response = cursor.fetchall() + return len(response) == 0 -- 2.39.2 From 2b41423a88d7bcf14261d389936c11ac36cb567b Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Tue, 24 Aug 2021 22:31:50 -0400 Subject: [PATCH 04/18] add delete_db for testing --- README.md | 38 ++++++++++------------- ceo_common/interfaces/IDatabaseService.py | 4 ++- ceod/api/database.py | 32 +++++++++++++++++++ ceod/db/MySQLService.py | 27 ++++++++++++---- ceod/db/PostgreSQLService.py | 30 ++++++++++++++---- tests/ceod/db/__init__.py | 0 tests/ceod/db/test_mysql.py | 11 +++++++ tests/ceod/db/test_postgres.py | 2 ++ tests/ceod_dev.ini | 8 +++++ tests/ceod_test_local.ini | 8 +++++ 10 files changed, 126 insertions(+), 34 deletions(-) create mode 100644 tests/ceod/db/__init__.py create mode 100644 tests/ceod/db/test_mysql.py create mode 100644 tests/ceod/db/test_postgres.py diff --git a/README.md b/README.md index 63a95e0..7afa9bd 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,3 @@ -### TODO before merge -- testing and tests -- need someone to test isolation of PostgreSQL users - # pyceo [![Build Status](https://ci.csclub.uwaterloo.ca/api/badges/public/pyceo/status.svg?ref=refs/heads/v1)](https://ci.csclub.uwaterloo.ca/public/pyceo) @@ -37,37 +33,37 @@ On phosphoric-acid, you will additionally need to create a principal called `ceod/admin` (remember to addprinc **and** ktadd). #### Database -Edit the `/etc/csc/ceod.ini` with the credentials required to access MySQL and PostgreSQL +create superuser `mysql` with password `mysql` ``` -[mysql] -host = -username = -password = +mysql -u root -[postgresql] -host = -usrename = -password = +CREATE USER 'mysql'@'localhost' IDENTIFIED BY 'mysql'; +GRANT ALL PRIVILEGES ON *.* TO 'mysql'@'localhost' WITH GRANT OPTION; ``` -#### PostgreSQL Database -PostgreSQL is not designed for isolation of users and by default will allow any user to connect and edit any database. To disallow users to create public schema we run +modify superuser `postgres` for password authentication and restrict new users ``` su postgres psql +ALTER USER postgres WITH PASSWORD 'postgres'; REVOKE ALL ON SCHEMA public FROM public; GRANT ALL ON SCHEMA public TO postgres; ``` -We also want to change `pg_hba.conf` to only allow local connections and force the requested database to have the same name as the user creating the connection ([more info](https://www.postgresql.org/docs/9.1/auth-pg-hba-conf.html)) +create a new `pg_hba.conf` to force password authentication and reject non local ``` +cd /etc/postgres/// +mv pg_hba.conf pg_hba.conf.old +``` +``` +# new pg_hba.conf # TYPE DATABASE USER ADDRESS METHOD -local all postgres peer +local all postgres md5 local sameuser all md5 +host all all 0.0.0.0/0 reject +``` +``` +systemctl restart postgres ``` -- peer authentication only requires that your os username matches the postgres username (no password) -- Users will have access to list of databases and users, and this cannot be disabled without possible issues ([more info](https://wiki.postgresql.org/wiki/Shared_Database_Hosting#template1)) -- [Managing rights in PostgreSQL](https://wiki.postgresql.org/images/d/d1/Managing_rights_in_postgresql.pdf) - #### Dependencies Next, install and activate a virtualenv: diff --git a/ceo_common/interfaces/IDatabaseService.py b/ceo_common/interfaces/IDatabaseService.py index f43f1ea..fcfbd7d 100644 --- a/ceo_common/interfaces/IDatabaseService.py +++ b/ceo_common/interfaces/IDatabaseService.py @@ -6,9 +6,11 @@ class IDatabaseService(Interface): """Interface to create databases for users.""" type = Attribute('the type of databases that will be created') - host = Attribute('the database host') auth_username = Attribute('username to a privileged user on the database host') auth_password = Attribute('password to a privileged user on the database host') def create_db(username: str) -> str: """try to create a database and user and return its password""" + + def delete_db(username: str): + """remove user and delete their database""" diff --git a/ceod/api/database.py b/ceod/api/database.py index f7695a1..402a8ab 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -29,6 +29,24 @@ def create_db_from_type(db_type: str, username: str): return {'error': 'Unexpected error'}, 500 +def delete_db_from_type(db_type: str, username: str): + try: + if not username.isalnum(): # username should not contain symbols + raise UserNotFoundError + ldap_srv = component.getUtility(ILDAPService) + ldap_srv.get_user(username) # make sure user exists + db_srv = component.getUtility(IDatabaseService, db_type) + db_srv.delete_db(username) + except UserNotFoundError: + return {'error': 'user not found'}, 404 + except DatabaseConnectionError: + return {'error': 'unable to connect or authenticate to sql server'}, 400 + except DatabasePermissionError: + return {'error': 'unable to perform action due to permissions'}, 502 + except: + return {'error': 'Unexpected error'}, 500 + + @bp.route('/mysql/', methods=['POST']) @requires_authentication_no_realm def create_mysql_db(auth_user: str, username: str): @@ -37,9 +55,23 @@ def create_mysql_db(auth_user: str, username: str): return create_db_from_type('mysql', username) +@bp.route('/mysql/', methods=['DELETE']) +@authz_restrict_to_syscom +@development_only +def delete_mysql_db(username: str): + delete_db_from_type('mysql', username) + + @bp.route('/postgresql/', methods=['POST']) @requires_authentication_no_realm def create_postgresql_db(auth_user: str, username: str): if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): return {'error': "not authorized to create databases for others"}, 403 return create_db_from_type('postgresql', username) + + +@bp.route('/postgresql/', methods=['DELETE']) +@authz_restrict_to_syscom +@development_only +def delete_postgresql_db(username: str): + delete_db_from_type('postgresl', username) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 9cc9ee5..875c635 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -13,7 +13,6 @@ class MySQLService: def __init__(self): self.type = 'mysql' config = component.getUtility(IConfig) - self.host = config.get('mysql_host') self.auth_username = config.get('mysql_username') self.auth_password = config.get('mysql_password') @@ -22,27 +21,43 @@ class MySQLService: search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" search_for_db = f"SHOW DATABASES LIKE '{username}'" create_user = f"CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s" - reset_password = f"ALTER USER '{username}' IDENTIFIED BY %(password)s" + reset_password = f"ALTER USER '{username}'@'localhost' IDENTIFIED BY %(password)s" create_database = f""" CREATE DATABASE {username}; GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'localhost'; """ try: with connect( - host=self.host, + host='localhost', user=self.auth_username, password=self.auth_password, ) as con: with con.cursor() as cursor: if response_is_empty(search_for_user, con): - cursor.execute(create_user, {password: password}) + cursor.execute(create_user, {'password': password}) else: - cursor.execute(reset_password, {password: password}) + cursor.execute(reset_password, {'password': password}) if response_is_empty(search_for_db, con): cursor.execute(create_database) - con.commit() return password except InterfaceError: raise DatabaseConnectionError() except ProgrammingError: raise DatabasePermissionError() + + def delete_db(self, username: str): + drop_user = f"DROP USER IF EXISTS '{username}'@'localhost'" + drop_db = f"DROP DATABASE IF EXISTS {username}" + try: + with connect( + host='localhost', + user=self.auth_username, + password=self.auth_password, + ) as con: + with con.cursor() as cursor: + cursor.execute(drop_user) + cursor.execute(drop_db) + except InterfaceError: + raise DatabaseConnectionError() + except ProgrammingError: + raise DatabasePermissionError() diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index ba0ba64..15e9e0b 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -13,7 +13,6 @@ class PostgreSQLService: def __init__(self): self.type = 'postgresql' config = component.getUtility(IConfig) - self.host = config.get('postgresql_host') self.auth_username = config.get('postgresql_username') self.auth_password = config.get('postgresql_password') @@ -29,16 +28,16 @@ class PostgreSQLService: """ try: with connect( - host=self.host, - user=self.auth_username, - password=self.auth_password, + host='localhost', + user=self.auth_username, + password=self.auth_password, ) as con: con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) with con.cursor() as cursor: if response_is_empty(search_for_user, con): - cursor.execute(create_user, {password: password}) + cursor.execute(create_user, {'password': password}) else: - cursor.execute(reset_password, {password: password}) + cursor.execute(reset_password, {'password': password}) if response_is_empty(search_for_db, con): cursor.execute(create_database) return password @@ -47,3 +46,22 @@ class PostgreSQLService: except ProgrammingError: raise DatabasePermissionError() + def delete_db(self, username: str): + drop_user = f"DROP USER IF EXISTS {username}" + drop_db = f"DROP DATABASE IF EXISTS {username}" + try: + with connect( + host='localhost', + user=self.auth_username, + password=self.auth_password, + ) as con: + con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) + with con.cursor() as cursor: + cursor.execute(drop_user) + cursor.execute(drop_db) + except OperationalError: + raise DatabaseConnectionError() + except ProgrammingError: + raise DatabasePermissionError() + + diff --git a/tests/ceod/db/__init__.py b/tests/ceod/db/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/ceod/db/test_mysql.py b/tests/ceod/db/test_mysql.py new file mode 100644 index 0000000..ce1c76f --- /dev/null +++ b/tests/ceod/db/test_mysql.py @@ -0,0 +1,11 @@ +import pytest + +# ask for mysql and postgres with proper postgres configs and no public schema + +# tests are stateless +# each test should not require anything before or change anything +# this means you should delete user and databases created after done + +# attempt to create database +# password reset +# recreate database (user dropped database) diff --git a/tests/ceod/db/test_postgres.py b/tests/ceod/db/test_postgres.py new file mode 100644 index 0000000..a1ace31 --- /dev/null +++ b/tests/ceod/db/test_postgres.py @@ -0,0 +1,2 @@ +import pytest + diff --git a/tests/ceod_dev.ini b/tests/ceod_dev.ini index 6e7fd38..f18e2a8 100644 --- a/tests/ceod_dev.ini +++ b/tests/ceod_dev.ini @@ -52,3 +52,11 @@ office = cdrom,audio,video,www [auxiliary mailing lists] syscom = syscom,syscom-alerts exec = exec + +[mysql] +username = mysql +password = mysql + +[postgresql] +username = postgres +password = postgres diff --git a/tests/ceod_test_local.ini b/tests/ceod_test_local.ini index f14419e..c9b7325 100644 --- a/tests/ceod_test_local.ini +++ b/tests/ceod_test_local.ini @@ -49,3 +49,11 @@ syscom = office,staff [auxiliary mailing lists] syscom = syscom,syscom-alerts exec = exec + +[mysql] +username = mysql +password = mysql + +[postgresql] +username = postgres +password = postgres -- 2.39.2 From 90818e5b1ff0436d35e53f48cdd3ec89f75d9ffc Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 25 Aug 2021 01:43:13 -0400 Subject: [PATCH 05/18] fix --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7afa9bd..000827a 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ GRANT ALL ON SCHEMA public TO postgres; ``` create a new `pg_hba.conf` to force password authentication and reject non local ``` -cd /etc/postgres/// +cd /etc/postgresql/// mv pg_hba.conf pg_hba.conf.old ``` ``` @@ -62,7 +62,7 @@ local sameuser all md5 host all all 0.0.0.0/0 reject ``` ``` -systemctl restart postgres +systemctl restart postgresql ``` #### Dependencies -- 2.39.2 From 5f74302b17ade613dd973dcec38f9ccc06892963 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 25 Aug 2021 01:46:08 -0400 Subject: [PATCH 06/18] fix --- ceod/db/PostgreSQLService.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 15e9e0b..21bf808 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -21,7 +21,7 @@ class PostgreSQLService: search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" search_for_db = f"SELECT FROM pg_database WHERE datname='{username}'" create_user = f"CREATE USER {username} WITH PASSWORD %(password)s" - reset_password = f"ALTER USER {username} WITH PASSWORD %(password)s)" + reset_password = f"ALTER USER {username} WITH PASSWORD %(password)s" create_database = f""" CREATE DATABASE {username} OWNER {username}; REVOKE ALL ON DATABASE {username} FROM PUBLIC; -- 2.39.2 From dc4d60fba2c86b04195eee94259c14b362f181ec Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 25 Aug 2021 21:49:51 -0400 Subject: [PATCH 07/18] testing --- ceo/utils.py | 2 +- ceo_common/interfaces/IDatabaseService.py | 1 - ceo_common/interfaces/__init__.py | 1 + ceod/api/app_factory.py | 14 ++++++--- ceod/api/database.py | 14 ++++----- ceod/db/__init__.py | 2 +- tests/ceod/db/test_mysql.py | 35 +++++++++++++++++++++++ tests/ceod/db/test_postgres.py | 1 - tests/ceod_dev.ini | 1 + tests/ceod_test_local.ini | 1 + 10 files changed, 57 insertions(+), 15 deletions(-) diff --git a/ceo/utils.py b/ceo/utils.py index fc84265..b8bb194 100644 --- a/ceo/utils.py +++ b/ceo/utils.py @@ -10,7 +10,7 @@ def http_request(method: str, path: str, **kwargs) -> requests.Response: client = component.getUtility(IHTTPClient) cfg = component.getUtility(IConfig) if path.startswith('/api/db'): - host = cfg.get('ceod_db_host') + host = cfg.get('ceod_database_host') need_cred = False else: host = cfg.get('ceod_admin_host') diff --git a/ceo_common/interfaces/IDatabaseService.py b/ceo_common/interfaces/IDatabaseService.py index fcfbd7d..1062df2 100644 --- a/ceo_common/interfaces/IDatabaseService.py +++ b/ceo_common/interfaces/IDatabaseService.py @@ -1,5 +1,4 @@ from zope.interface import Attribute, Interface -from .IUser import IUser class IDatabaseService(Interface): diff --git a/ceo_common/interfaces/__init__.py b/ceo_common/interfaces/__init__.py index 56f2203..226b90f 100644 --- a/ceo_common/interfaces/__init__.py +++ b/ceo_common/interfaces/__init__.py @@ -8,3 +8,4 @@ from .IUWLDAPService import IUWLDAPService from .IMailService import IMailService from .IMailmanService import IMailmanService from .IHTTPClient import IHTTPClient +from .IDatabaseService import IDatabaseService diff --git a/ceod/api/app_factory.py b/ceod/api/app_factory.py index fa0a424..2dae0ed 100644 --- a/ceod/api/app_factory.py +++ b/ceod/api/app_factory.py @@ -38,6 +38,10 @@ def create_app(flask_config={}): from ceod.api import mailman app.register_blueprint(mailman.bp, url_prefix='/api/mailman') + if hostname == cfg.get('ceod_database_host'): + from ceod.api import database + app.register_blueprint(database.bp, url_prefix='/api/db') + from ceod.api import groups app.register_blueprint(groups.bp, url_prefix='/api/groups') @@ -109,9 +113,11 @@ def register_services(app): component.provideUtility(uwldap_srv, IUWLDAPService) # MySQLService - mysql_srv = MySQLService() - component.provideUtility(mysql_srv, IDatabaseService, 'mysql') + if hostname == cfg.get('ceod_database_host'): + mysql_srv = MySQLService() + component.provideUtility(mysql_srv, IDatabaseService, 'mysql') # PostgreSQLService - psql_srv = PostgreSQLService() - component.provideUtility(psql_srv, IDatabaseService, 'postgresql') + if hostname == cfg.get('ceod_database_host'): + psql_srv = PostgreSQLService() + component.provideUtility(psql_srv, IDatabaseService, 'postgresql') diff --git a/ceod/api/database.py b/ceod/api/database.py index 402a8ab..ff31b41 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -55,13 +55,6 @@ def create_mysql_db(auth_user: str, username: str): return create_db_from_type('mysql', username) -@bp.route('/mysql/', methods=['DELETE']) -@authz_restrict_to_syscom -@development_only -def delete_mysql_db(username: str): - delete_db_from_type('mysql', username) - - @bp.route('/postgresql/', methods=['POST']) @requires_authentication_no_realm def create_postgresql_db(auth_user: str, username: str): @@ -70,6 +63,13 @@ def create_postgresql_db(auth_user: str, username: str): return create_db_from_type('postgresql', username) +@bp.route('/mysql/', methods=['DELETE']) +@authz_restrict_to_syscom +@development_only +def delete_mysql_db(username: str): + delete_db_from_type('mysql', username) + + @bp.route('/postgresql/', methods=['DELETE']) @authz_restrict_to_syscom @development_only diff --git a/ceod/db/__init__.py b/ceod/db/__init__.py index b5373bd..6e93e5c 100644 --- a/ceod/db/__init__.py +++ b/ceod/db/__init__.py @@ -1,2 +1,2 @@ from .MySQLService import MySQLService -from .PostgreSQLService import PostgreSQLService \ No newline at end of file +from .PostgreSQLService import PostgreSQLService diff --git a/tests/ceod/db/test_mysql.py b/tests/ceod/db/test_mysql.py index ce1c76f..4c7da26 100644 --- a/tests/ceod/db/test_mysql.py +++ b/tests/ceod/db/test_mysql.py @@ -1,5 +1,40 @@ import pytest +from ceod.db.MySQLService import MySQLService +from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError +from mysql.connector import connect +from mysql.connector.errors import InterfaceError, ProgrammingError + + +def test_mysql_db_create(cfg): + mysql_srv = MySQLService() + password = mysql_srv.create_db('test_jdoe') + + with connect( + host=cfg.get('ceod_database_host'), + user='test_jdoe', + password=password, + ) as con: + with con.cursor() as cur: + cur.execute("SHOW DATABASES") + response = cur.fetchall() + assert len(response) == 2 + + mysql_srv.delete_db('test_jdoe') + + # user should be deleted + with pytest.raises(InterfaceError): + con = connect( + host=cfg.get('ceod_database_host'), + user='test_jdoe', + password=password, + ) + +# except InterfaceError: +# raise DatabaseConnectionError() +# except ProgrammingError: +# raise DatabasePermissionError() + # ask for mysql and postgres with proper postgres configs and no public schema # tests are stateless diff --git a/tests/ceod/db/test_postgres.py b/tests/ceod/db/test_postgres.py index a1ace31..8b13789 100644 --- a/tests/ceod/db/test_postgres.py +++ b/tests/ceod/db/test_postgres.py @@ -1,2 +1 @@ -import pytest diff --git a/tests/ceod_dev.ini b/tests/ceod_dev.ini index 33f2f3a..36ae91b 100644 --- a/tests/ceod_dev.ini +++ b/tests/ceod_dev.ini @@ -7,6 +7,7 @@ admin_host = phosphoric-acid # this is the host with NFS no_root_squash fs_root_host = phosphoric-acid mailman_host = mail +database_host = coffee krb5_cache_dir = /run/ceod/krb5_cache use_https = false port = 9987 diff --git a/tests/ceod_test_local.ini b/tests/ceod_test_local.ini index 9e04403..07d0221 100644 --- a/tests/ceod_test_local.ini +++ b/tests/ceod_test_local.ini @@ -7,6 +7,7 @@ uw_domain = uwaterloo.internal admin_host = phosphoric-acid fs_root_host = phosphoric-acid mailman_host = phosphoric-acid +database_host = phosphoric-acid krb5_cache_dir = /tmp/ceod_test_krb5_cache use_https = false port = 9987 -- 2.39.2 From 57180df040333f573cbe0bdb39eb655a9cbd38e3 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Thu, 26 Aug 2021 03:24:39 +0000 Subject: [PATCH 08/18] add libpq-dev as dependency --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1d409cb..6f23af9 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ messages get accepted (by default they get held). #### Dependencies Next, install and activate a virtualenv: ```sh -sudo apt install libkrb5-dev python3-dev +sudo apt install libkrb5-dev libpq-dev python3-dev python3 -m venv venv . venv/bin/activate pip install -r requirements.txt -- 2.39.2 From 8252afca16cedcfc0600999a97f740ee14cf71ed Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Thu, 26 Aug 2021 02:02:47 -0400 Subject: [PATCH 09/18] adjustments part 1 --- README.md | 2 +- ceo_common/errors.py | 5 +++++ ceod/api/database.py | 22 +++++++++++----------- ceod/db/MySQLService.py | 6 ++++-- ceod/db/PostgreSQLService.py | 6 ++++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index c350db4..500219b 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ mv pg_hba.conf pg_hba.conf.old # TYPE DATABASE USER ADDRESS METHOD local all postgres md5 local sameuser all md5 -host all all 0.0.0.0/0 reject +host sameuser all 0.0.0.0/0 md5 ``` ``` systemctl restart postgresql diff --git a/ceo_common/errors.py b/ceo_common/errors.py index 8c996ec..c640419 100644 --- a/ceo_common/errors.py +++ b/ceo_common/errors.py @@ -51,6 +51,11 @@ class NoSuchListError(Exception): super().__init__('mailing list does not exist') +class InvalidUsernameError(Exception): + def __init__(self): + super().__init__('Username contains characters that are not allowed') + + class DatabaseConnectionError(Exception): def __init__(self): super().__init__('unable to connect or authenticate to sql service') diff --git a/ceod/api/database.py b/ceod/api/database.py index ff31b41..3001ee6 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -3,7 +3,7 @@ from zope import component from ceod.api.utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ user_is_in_group, requires_authentication_no_realm, \ create_streaming_response, development_only -from ceo_common.errors import UserNotFoundError, DatabaseConnectionError, DatabasePermissionError +from ceo_common.errors import UserNotFoundError, DatabaseConnectionError, DatabasePermissionError, InvalidUsernameError from ceo_common.interfaces import ILDAPService, IDatabaseService @@ -13,7 +13,7 @@ bp = Blueprint('db', __name__) def create_db_from_type(db_type: str, username: str): try: if not username.isalnum(): # username should not contain symbols - raise UserNotFoundError + raise InvalidUsernameError() ldap_srv = component.getUtility(ILDAPService) ldap_srv.get_user(username) # make sure user exists db_srv = component.getUtility(IDatabaseService, db_type) @@ -21,30 +21,30 @@ def create_db_from_type(db_type: str, username: str): return {'password': password} except UserNotFoundError: return {'error': 'user not found'}, 404 + except InvalidUsernameError: + return {'error': 'username contains invalid characters'}, 400 except DatabaseConnectionError: - return {'error': 'unable to connect or authenticate to sql server'}, 400 + return {'error': 'unable to connect or authenticate to sql server'}, 500 except DatabasePermissionError: - return {'error': 'unable to perform action due to permissions'}, 502 - except: - return {'error': 'Unexpected error'}, 500 + return {'error': 'unable to perform action due to permissions'}, 500 def delete_db_from_type(db_type: str, username: str): try: if not username.isalnum(): # username should not contain symbols - raise UserNotFoundError + raise InvalidUsernameError() ldap_srv = component.getUtility(ILDAPService) ldap_srv.get_user(username) # make sure user exists db_srv = component.getUtility(IDatabaseService, db_type) db_srv.delete_db(username) except UserNotFoundError: return {'error': 'user not found'}, 404 + except InvalidUsernameError: + return {'error': 'username contains invalid characters'}, 400 except DatabaseConnectionError: - return {'error': 'unable to connect or authenticate to sql server'}, 400 + return {'error': 'unable to connect or authenticate to sql server'}, 500 except DatabasePermissionError: - return {'error': 'unable to perform action due to permissions'}, 502 - except: - return {'error': 'Unexpected error'}, 500 + return {'error': 'unable to perform action due to permissions'}, 500 @bp.route('/mysql/', methods=['POST']) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 875c635..bd41a9f 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -10,8 +10,10 @@ from mysql.connector.errors import InterfaceError, ProgrammingError @implementer(IDatabaseService) class MySQLService: + + type = 'mysql' + def __init__(self): - self.type = 'mysql' config = component.getUtility(IConfig) self.auth_username = config.get('mysql_username') self.auth_password = config.get('mysql_password') @@ -55,8 +57,8 @@ class MySQLService: password=self.auth_password, ) as con: with con.cursor() as cursor: - cursor.execute(drop_user) cursor.execute(drop_db) + cursor.execute(drop_user) except InterfaceError: raise DatabaseConnectionError() except ProgrammingError: diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 21bf808..009542d 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -10,8 +10,10 @@ from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT @implementer(IDatabaseService) class PostgreSQLService: + + type = 'postgresql' + def __init__(self): - self.type = 'postgresql' config = component.getUtility(IConfig) self.auth_username = config.get('postgresql_username') self.auth_password = config.get('postgresql_password') @@ -57,8 +59,8 @@ class PostgreSQLService: ) as con: con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) with con.cursor() as cursor: - cursor.execute(drop_user) cursor.execute(drop_db) + cursor.execute(drop_user) except OperationalError: raise DatabaseConnectionError() except ProgrammingError: -- 2.39.2 From ef3d130f7890ae0ad513170dd10f37b56d4f780a Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Thu, 26 Aug 2021 15:42:18 -0400 Subject: [PATCH 10/18] add pwreset endpoint --- .drone.yml | 2 +- ceo_common/interfaces/IDatabaseService.py | 5 +- ceod/api/database.py | 88 ++++++++++++++--------- ceod/db/MySQLService.py | 86 ++++++++++++---------- ceod/db/PostgreSQLService.py | 79 +++++++++++--------- tests/ceod/db/test_mysql.py | 20 +++++- 6 files changed, 173 insertions(+), 107 deletions(-) diff --git a/.drone.yml b/.drone.yml index 7084efa..7a57909 100644 --- a/.drone.yml +++ b/.drone.yml @@ -10,7 +10,7 @@ steps: # way to share system packages between steps commands: # install dependencies - - apt update && apt install -y libkrb5-dev python3-dev + - apt update && apt install -y libkrb5-dev libpq-dev python3-dev - python3 -m venv venv - . venv/bin/activate - pip install -r dev-requirements.txt diff --git a/ceo_common/interfaces/IDatabaseService.py b/ceo_common/interfaces/IDatabaseService.py index 1062df2..973d21d 100644 --- a/ceo_common/interfaces/IDatabaseService.py +++ b/ceo_common/interfaces/IDatabaseService.py @@ -9,7 +9,10 @@ class IDatabaseService(Interface): auth_password = Attribute('password to a privileged user on the database host') def create_db(username: str) -> str: - """try to create a database and user and return its password""" + """create a user and database and return the password""" + + def reset_passwd(username: str) -> str: + """reset user password and return it""" def delete_db(username: str): """remove user and delete their database""" diff --git a/ceod/api/database.py b/ceod/api/database.py index 3001ee6..1a2d169 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -1,50 +1,58 @@ from flask import Blueprint, request from zope import component +from functools import wraps + from ceod.api.utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ user_is_in_group, requires_authentication_no_realm, \ create_streaming_response, development_only -from ceo_common.errors import UserNotFoundError, DatabaseConnectionError, DatabasePermissionError, InvalidUsernameError +from ceo_common.errors import UserNotFoundError, DatabaseConnectionError, DatabasePermissionError, \ + InvalidUsernameError, UserAlreadyExistsError from ceo_common.interfaces import ILDAPService, IDatabaseService bp = Blueprint('db', __name__) +def db_exception_handler(func): + @wraps(func) + def function(db_type: str, username: str): + try: + if not username.isalnum(): # username should not contain symbols + raise InvalidUsernameError() + ldap_srv = component.getUtility(ILDAPService) + ldap_srv.get_user(username) # make sure user exists + return func(db_type, username) + except UserNotFoundError: + return {'error': 'user not found'}, 404 + except UserAlreadyExistsError: + return {'error': 'database user is already created'}, 409 + except InvalidUsernameError: + return {'error': 'username contains invalid characters'}, 400 + except DatabaseConnectionError: + return {'error': 'unable to connect or authenticate to sql server'}, 500 + except DatabasePermissionError: + return {'error': 'unable to perform action due to permissions'}, 500 + return function + + +@db_exception_handler def create_db_from_type(db_type: str, username: str): - try: - if not username.isalnum(): # username should not contain symbols - raise InvalidUsernameError() - ldap_srv = component.getUtility(ILDAPService) - ldap_srv.get_user(username) # make sure user exists - db_srv = component.getUtility(IDatabaseService, db_type) - password = db_srv.create_db(username) - return {'password': password} - except UserNotFoundError: - return {'error': 'user not found'}, 404 - except InvalidUsernameError: - return {'error': 'username contains invalid characters'}, 400 - except DatabaseConnectionError: - return {'error': 'unable to connect or authenticate to sql server'}, 500 - except DatabasePermissionError: - return {'error': 'unable to perform action due to permissions'}, 500 + db_srv = component.getUtility(IDatabaseService, db_type) + password = db_srv.create_db(username) + return {'password': password} +@db_exception_handler +def reset_db_passwd_from_type(db_type: str, username: str): + db_srv = component.getUtility(IDatabaseService, db_type) + password = db_srv.reset_passwd(username) + return {'password': password} + + +@db_exception_handler def delete_db_from_type(db_type: str, username: str): - try: - if not username.isalnum(): # username should not contain symbols - raise InvalidUsernameError() - ldap_srv = component.getUtility(ILDAPService) - ldap_srv.get_user(username) # make sure user exists - db_srv = component.getUtility(IDatabaseService, db_type) - db_srv.delete_db(username) - except UserNotFoundError: - return {'error': 'user not found'}, 404 - except InvalidUsernameError: - return {'error': 'username contains invalid characters'}, 400 - except DatabaseConnectionError: - return {'error': 'unable to connect or authenticate to sql server'}, 500 - except DatabasePermissionError: - return {'error': 'unable to perform action due to permissions'}, 500 + db_srv = component.getUtility(IDatabaseService, db_type) + db_srv.delete_db(username) @bp.route('/mysql/', methods=['POST']) @@ -63,6 +71,22 @@ def create_postgresql_db(auth_user: str, username: str): return create_db_from_type('postgresql', username) +@bp.route('/mysql//pwreset', methods=['POST']) +@requires_authentication_no_realm +def reset_mysql_db_passwd(auth_user: str, username: str): + if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): + return {'error': "not authorized to request password reset for others"}, 403 + return reset_db_passwd_from_type('mysql', username) + + +@bp.route('/postgresql//pwreset', methods=['POST']) +@requires_authentication_no_realm +def reset_postgresql_db_passwd(auth_user: str, username: str): + if not (auth_user == username or user_is_in_group(auth_user, 'syscom')): + return {'error': "not authorized to request password reset for others"}, 403 + return reset_db_passwd_from_type('postgresql', username) + + @bp.route('/mysql/', methods=['DELETE']) @authz_restrict_to_syscom @development_only diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index bd41a9f..8e7c8cb 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -1,9 +1,13 @@ from zope.interface import implementer from zope import component +from contextlib import contextmanager + from ceo_common.interfaces import IDatabaseService, IConfig -from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError +from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError, UserAlreadyExistsError, \ + UserNotFoundError from ceod.utils import gen_password from ceod.db.utils import response_is_empty + from mysql.connector import connect from mysql.connector.errors import InterfaceError, ProgrammingError @@ -18,48 +22,58 @@ class MySQLService: self.auth_username = config.get('mysql_username') self.auth_password = config.get('mysql_password') - def create_db(self, username: str) -> str: - password = gen_password() - search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" - search_for_db = f"SHOW DATABASES LIKE '{username}'" - create_user = f"CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s" - reset_password = f"ALTER USER '{username}'@'localhost' IDENTIFIED BY %(password)s" - create_database = f""" - CREATE DATABASE {username}; - GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'localhost'; - """ - try: - with connect( - host='localhost', - user=self.auth_username, - password=self.auth_password, - ) as con: - with con.cursor() as cursor: - if response_is_empty(search_for_user, con): - cursor.execute(create_user, {'password': password}) - else: - cursor.execute(reset_password, {'password': password}) - if response_is_empty(search_for_db, con): - cursor.execute(create_database) - return password - except InterfaceError: - raise DatabaseConnectionError() - except ProgrammingError: - raise DatabasePermissionError() - - def delete_db(self, username: str): - drop_user = f"DROP USER IF EXISTS '{username}'@'localhost'" - drop_db = f"DROP DATABASE IF EXISTS {username}" + @contextmanager + def mysql_connection(self): try: with connect( host='localhost', user=self.auth_username, password=self.auth_password, ) as con: - with con.cursor() as cursor: - cursor.execute(drop_db) - cursor.execute(drop_user) + yield con except InterfaceError: raise DatabaseConnectionError() except ProgrammingError: raise DatabasePermissionError() + + def create_db(self, username: str) -> str: + password = gen_password() + search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" + search_for_db = f"SHOW DATABASES LIKE '{username}'" + create_user = f"CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s" + create_database = f""" + CREATE DATABASE {username}; + GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'localhost'; + """ + + with self.mysql_connection() as con: + with con.cursor() as cursor: + if response_is_empty(search_for_user, con): + cursor.execute(create_user, {'password': password}) + if response_is_empty(search_for_db, con): + cursor.execute(create_database) + else: + raise UserAlreadyExistsError() + return password + + def reset_db_passwd(self, username: str) -> str: + password = gen_password() + search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" + reset_password = f"ALTER USER '{username}'@'localhost' IDENTIFIED BY %(password)s" + + with self.mysql_connection() as con: + with con.cursor() as cursor: + if not response_is_empty(search_for_user, con): + cursor.execute(reset_password, {'password': password}) + else: + raise UserNotFoundError(username) + return password + + def delete_db(self, username: str): + drop_user = f"DROP USER IF EXISTS '{username}'@'localhost'" + drop_db = f"DROP DATABASE IF EXISTS {username}" + + with self.mysql_connection() as con: + with con.cursor() as cursor: + cursor.execute(drop_db) + cursor.execute(drop_user) diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 009542d..1ddef9e 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -1,9 +1,13 @@ from zope.interface import implementer from zope import component +from contextlib import contextmanager + from ceo_common.interfaces import IDatabaseService, IConfig -from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError +from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError, UserAlreadyExistsError, \ + UserNotFoundError from ceod.utils import gen_password from ceod.db.utils import response_is_empty + from psycopg2 import connect, OperationalError, ProgrammingError from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT @@ -18,16 +22,8 @@ class PostgreSQLService: self.auth_username = config.get('postgresql_username') self.auth_password = config.get('postgresql_password') - def create_db(self, username: str) -> str: - password = gen_password() - search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" - search_for_db = f"SELECT FROM pg_database WHERE datname='{username}'" - create_user = f"CREATE USER {username} WITH PASSWORD %(password)s" - reset_password = f"ALTER USER {username} WITH PASSWORD %(password)s" - create_database = f""" - CREATE DATABASE {username} OWNER {username}; - REVOKE ALL ON DATABASE {username} FROM PUBLIC; - """ + @contextmanager + def psql_connection(self): try: with connect( host='localhost', @@ -35,35 +31,50 @@ class PostgreSQLService: password=self.auth_password, ) as con: con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) - with con.cursor() as cursor: - if response_is_empty(search_for_user, con): - cursor.execute(create_user, {'password': password}) - else: - cursor.execute(reset_password, {'password': password}) - if response_is_empty(search_for_db, con): - cursor.execute(create_database) - return password + yield con except OperationalError: raise DatabaseConnectionError() except ProgrammingError: raise DatabasePermissionError() + def create_db(self, username: str) -> str: + password = gen_password() + search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" + search_for_db = f"SELECT FROM pg_database WHERE datname='{username}'" + create_user = f"CREATE USER {username} WITH PASSWORD %(password)s" + create_database = f""" + CREATE DATABASE {username} OWNER {username}; + REVOKE ALL ON DATABASE {username} FROM PUBLIC; + """ + + with self.psql_connection() as con: + with con.cursor() as cursor: + if response_is_empty(search_for_user, con): + cursor.execute(create_user, {'password': password}) + if response_is_empty(search_for_db, con): + cursor.execute(create_database) + else: + raise UserAlreadyExistsError() + return password + + def reset_db_passwd(self, username: str) -> str: + password = gen_password() + search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" + reset_password = f"ALTER USER {username} WITH PASSWORD %(password)s" + + with self.psql_connection() as con: + with con.cursor() as cursor: + if not response_is_empty(search_for_user, con): + cursor.execute(reset_password, {'password': password}) + else: + raise UserNotFoundError(username) + return password + def delete_db(self, username: str): drop_user = f"DROP USER IF EXISTS {username}" drop_db = f"DROP DATABASE IF EXISTS {username}" - try: - with connect( - host='localhost', - user=self.auth_username, - password=self.auth_password, - ) as con: - con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) - with con.cursor() as cursor: - cursor.execute(drop_db) - cursor.execute(drop_user) - except OperationalError: - raise DatabaseConnectionError() - except ProgrammingError: - raise DatabasePermissionError() - + with self.psql_connection() as con: + with con.cursor() as cursor: + cursor.execute(drop_db) + cursor.execute(drop_user) diff --git a/tests/ceod/db/test_mysql.py b/tests/ceod/db/test_mysql.py index 4c7da26..2d14544 100644 --- a/tests/ceod/db/test_mysql.py +++ b/tests/ceod/db/test_mysql.py @@ -30,6 +30,23 @@ def test_mysql_db_create(cfg): password=password, ) + +def test_mysql_passwd_reset(): + pass + + +# test with curl +# test with invalid perms for curl + +# test perms + +# test with dup user + +# test with invalid perms for db + +# test with invalid host for db + + # except InterfaceError: # raise DatabaseConnectionError() # except ProgrammingError: @@ -41,6 +58,3 @@ def test_mysql_db_create(cfg): # each test should not require anything before or change anything # this means you should delete user and databases created after done -# attempt to create database -# password reset -# recreate database (user dropped database) -- 2.39.2 From 29305168c35b1ee8ffa4768b459a8444043f7b9e Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Thu, 26 Aug 2021 16:45:24 -0400 Subject: [PATCH 11/18] allow db users to login remotely --- README.md | 12 +++++++++++- ceod/db/MySQLService.py | 16 +++++++++++++--- ceod/db/PostgreSQLService.py | 2 +- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 8bf9710..f2df9f3 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ ALTER USER postgres WITH PASSWORD 'postgres'; REVOKE ALL ON SCHEMA public FROM public; GRANT ALL ON SCHEMA public TO postgres; ``` -create a new `pg_hba.conf` to force password authentication and reject non local +create a new `pg_hba.conf` to force password authentication ``` cd /etc/postgresql/// mv pg_hba.conf pg_hba.conf.old @@ -58,12 +58,22 @@ mv pg_hba.conf pg_hba.conf.old # new pg_hba.conf # TYPE DATABASE USER ADDRESS METHOD local all postgres md5 +host all postgres localhost md5 +host all postgres 0.0.0.0/0 reject +host all postgres ::/0 reject local sameuser all md5 host sameuser all 0.0.0.0/0 md5 +host sameuser all ::/0 md5 +``` +``` +# modified postgresql.conf +# listen_addresses = 'localhost' +listen_address = '*' ``` ``` systemctl restart postgresql ``` +users can login remotely but superusers (`postgres` and `mysql`) are only allowed to login from the database host #### Mailman You should create the following mailing lists from the mail container: diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 8e7c8cb..4a73e5c 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -40,10 +40,14 @@ class MySQLService: password = gen_password() search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" search_for_db = f"SHOW DATABASES LIKE '{username}'" - create_user = f"CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s" + create_user = f""" + CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s; + CREATE USER '{username}'@'%' IDENTIFIED BY %(password)s; + """ create_database = f""" CREATE DATABASE {username}; GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'localhost'; + GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'%'; """ with self.mysql_connection() as con: @@ -59,7 +63,10 @@ class MySQLService: def reset_db_passwd(self, username: str) -> str: password = gen_password() search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" - reset_password = f"ALTER USER '{username}'@'localhost' IDENTIFIED BY %(password)s" + reset_password = f""" + ALTER USER '{username}'@'localhost' IDENTIFIED BY %(password)s + ALTER USER '{username}'@'%' IDENTIFIED BY %(password)s + """ with self.mysql_connection() as con: with con.cursor() as cursor: @@ -70,8 +77,11 @@ class MySQLService: return password def delete_db(self, username: str): - drop_user = f"DROP USER IF EXISTS '{username}'@'localhost'" drop_db = f"DROP DATABASE IF EXISTS {username}" + drop_user = f""" + DROP USER IF EXISTS '{username}'@'localhost'; + DROP USER IF EXISTS '{username}'@'%'; + """ with self.mysql_connection() as con: with con.cursor() as cursor: diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 1ddef9e..6ab3c49 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -71,8 +71,8 @@ class PostgreSQLService: return password def delete_db(self, username: str): - drop_user = f"DROP USER IF EXISTS {username}" drop_db = f"DROP DATABASE IF EXISTS {username}" + drop_user = f"DROP USER IF EXISTS {username}" with self.psql_connection() as con: with con.cursor() as cursor: -- 2.39.2 From d6dbfd5d3fec285c0c48af0ca1e47de826ed2471 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Fri, 27 Aug 2021 21:01:36 -0400 Subject: [PATCH 12/18] add tests --- tests/ceod/api/test_db_mysql.py | 121 ++++++++++++++++++++++++++++++++ tests/ceod/api/test_db_psql.py | 120 +++++++++++++++++++++++++++++++ tests/ceod/db/__init__.py | 0 tests/ceod/db/test_mysql.py | 60 ---------------- tests/ceod/db/test_postgres.py | 1 - 5 files changed, 241 insertions(+), 61 deletions(-) create mode 100644 tests/ceod/api/test_db_mysql.py create mode 100644 tests/ceod/api/test_db_psql.py delete mode 100644 tests/ceod/db/__init__.py delete mode 100644 tests/ceod/db/test_mysql.py delete mode 100644 tests/ceod/db/test_postgres.py diff --git a/tests/ceod/api/test_db_mysql.py b/tests/ceod/api/test_db_mysql.py new file mode 100644 index 0000000..0c3f9a7 --- /dev/null +++ b/tests/ceod/api/test_db_mysql.py @@ -0,0 +1,121 @@ +import pytest + +from ceod.model import User +from mysql.connector import connect +from mysql.connector.errors import InterfaceError, ProgrammingError + + +def test_api_create_mysql_db(cfg, client, g_admin_ctx, create_user_result): + uid = create_user_result['uid'] + with g_admin_ctx(): + user = User(uid='someone_else', cn='Some Name', terms=['s2021']) + user.add_to_ldap() + + # user should be able to create db for themselves + status, data = client.post(f"/api/mysql/{uid}", json={}, principal=uid) + assert status == 200 + assert 'password' in data + passwd = data['password'] + + # conflict if attempting to create db when already has one + status, data = client.post(f"/api/mysql/{uid}", json={}, principal=uid) + assert status == 409 + + # normal user cannot create db for others + status, data = client.post(f"/api/mysql/someone_else", json={}, principal=uid) + assert status == 403 + + # cannot create db for user not in ldap + status, data = client.post("/api/mysql/user_not_found", json={}) + assert status == 404 + + # cannot create db when username contains symbols + status, data = client.post("/api/mysql/#invalid", json={}) + assert status == 400 + + with connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=passwd, + ) as con: + with con.cursor() as cur: + cur.execute("SHOW DATABASES") + response = cur.fetchall() + assert len(response) == 2 + + with pytest.raises(ProgrammingError): + cur.execute("CREATE DATABASE new_db") + + status, data = client.delete(f"/api/mysql/{uid}", json={}) + assert status == 200 + + # user should be deleted + with pytest.raises(InterfaceError): + con = connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=passwd, + ) + + # db should be deleted + with connect( + host=cfg.get('ceod_database_host'), + user=cfg.get('mysql_username'), + password=cfg.get('mysql_password'), + ) as con: + with con.cursor() as cur: + cur.execute(f"SHOW DATABASES LIKE '{uid}'") + response = cur.fetchall() + assert len(response) == 0 + + with g_admin_ctx(): + user.remove_from_ldap() + + +def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, create_user_result): + with g_admin_ctx(): + user = User(uid='someone_else', cn='Some Name', terms=['s2021']) + user.add_to_ldap() + + uid = create_user_result['uid'] + + status, data = client.post(f"/api/mysql/{uid}", json={}) + assert status == 200 + assert 'password' in data + old_passwd = data['password'] + + con = connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=old_passwd, + ) + con.close() + + # normal user can get a password reset for themselves + status, data = client.post(f"/api/mysql/{uid}/pwreset", json={}, principal=uid) + assert status == 200 + assert 'password' in data + new_passwd = data['password'] + + assert old_passwd != new_passwd + + # normal user cannot reset password for others + status, data = client.post(f"/api/mysql/{uid}/pwreset", json={}, principal='someone_else') + assert status == 403 + + # cannot password reset a user that does not have a database + status, data = client.post(f"/api/mysql/someone_else/pwreset", json={}) + assert status == 404 + + con = connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=new_passwd, + ) + con.close() + + status, data = client.delete(f"/api/mysql/{uid}", json={}) + assert status == 200 + + with g_admin_ctx(): + user.remove_from_ldap() diff --git a/tests/ceod/api/test_db_psql.py b/tests/ceod/api/test_db_psql.py new file mode 100644 index 0000000..ac5671a --- /dev/null +++ b/tests/ceod/api/test_db_psql.py @@ -0,0 +1,120 @@ +import pytest + +from ceod.model import User +from psycopg2 import connect, OperationalError, ProgrammingError + + +def test_api_create_psql_db(cfg, client, g_admin_ctx, create_user_result): + uid = create_user_result['uid'] + with g_admin_ctx(): + user = User(uid='someone_else', cn='Some Name', terms=['s2021']) + user.add_to_ldap() + + # user should be able to create db for themselves + status, data = client.post(f"/api/postgresql/{uid}", json={}, principal=uid) + assert status == 200 + assert 'password' in data + passwd = data['password'] + + # conflict if attempting to create db when already has one + status, data = client.post(f"/api/postgresql/{uid}", json={}, principal=uid) + assert status == 409 + + # normal user cannot create db for others + status, data = client.post(f"/api/postgresql/someone_else", json={}, principal=uid) + assert status == 403 + + # cannot create db for user not in ldap + status, data = client.post("/api/postgresql/user_not_found", json={}) + assert status == 404 + + # cannot create db when username contains symbols + status, data = client.post("/api/postgresql/#invalid", json={}) + assert status == 400 + + with connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=passwd, + ) as con: + with con.cursor() as cur: + cur.execute("SHOW DATABASES") + response = cur.fetchall() + assert len(response) == 2 + + with pytest.raises(ProgrammingError): + cur.execute("CREATE DATABASE new_db") + + status, data = client.delete(f"/api/postgresql/{uid}", json={}) + assert status == 200 + + # user should be deleted + with pytest.raises(OperationalError): + con = connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=passwd, + ) + + # db should be deleted + with connect( + host=cfg.get('ceod_database_host'), + user=cfg.get('postgresql_username'), + password=cfg.get('postgresql_password'), + ) as con: + with con.cursor() as cur: + cur.execute(f"SHOW DATABASES LIKE '{uid}'") + response = cur.fetchall() + assert len(response) == 0 + + with g_admin_ctx(): + user.remove_from_ldap() + + +def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, create_user_result): + with g_admin_ctx(): + user = User(uid='someone_else', cn='Some Name', terms=['s2021']) + user.add_to_ldap() + + uid = create_user_result['uid'] + + status, data = client.post(f"/api/postgresql/{uid}", json={}) + assert status == 200 + assert 'password' in data + old_passwd = data['password'] + + con = connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=old_passwd, + ) + con.close() + + # normal user can get a password reset for themselves + status, data = client.post(f"/api/postgresql/{uid}/pwreset", json={}, principal=uid) + assert status == 200 + assert 'password' in data + new_passwd = data['password'] + + assert old_passwd != new_passwd + + # normal user cannot reset password for others + status, data = client.post(f"/api/postgresql/{uid}/pwreset", json={}, principal='someone_else') + assert status == 403 + + # cannot password reset a user that does not have a database + status, data = client.post(f"/api/postgresql/someone_else/pwreset", json={}) + assert status == 404 + + con = connect( + host=cfg.get('ceod_database_host'), + user=uid, + password=new_passwd, + ) + con.close() + + status, data = client.delete(f"/api/postgresql/{uid}", json={}) + assert status == 200 + + with g_admin_ctx(): + user.remove_from_ldap() diff --git a/tests/ceod/db/__init__.py b/tests/ceod/db/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/ceod/db/test_mysql.py b/tests/ceod/db/test_mysql.py deleted file mode 100644 index 2d14544..0000000 --- a/tests/ceod/db/test_mysql.py +++ /dev/null @@ -1,60 +0,0 @@ -import pytest - -from ceod.db.MySQLService import MySQLService -from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError -from mysql.connector import connect -from mysql.connector.errors import InterfaceError, ProgrammingError - - -def test_mysql_db_create(cfg): - mysql_srv = MySQLService() - password = mysql_srv.create_db('test_jdoe') - - with connect( - host=cfg.get('ceod_database_host'), - user='test_jdoe', - password=password, - ) as con: - with con.cursor() as cur: - cur.execute("SHOW DATABASES") - response = cur.fetchall() - assert len(response) == 2 - - mysql_srv.delete_db('test_jdoe') - - # user should be deleted - with pytest.raises(InterfaceError): - con = connect( - host=cfg.get('ceod_database_host'), - user='test_jdoe', - password=password, - ) - - -def test_mysql_passwd_reset(): - pass - - -# test with curl -# test with invalid perms for curl - -# test perms - -# test with dup user - -# test with invalid perms for db - -# test with invalid host for db - - -# except InterfaceError: -# raise DatabaseConnectionError() -# except ProgrammingError: -# raise DatabasePermissionError() - -# ask for mysql and postgres with proper postgres configs and no public schema - -# tests are stateless -# each test should not require anything before or change anything -# this means you should delete user and databases created after done - diff --git a/tests/ceod/db/test_postgres.py b/tests/ceod/db/test_postgres.py deleted file mode 100644 index 8b13789..0000000 --- a/tests/ceod/db/test_postgres.py +++ /dev/null @@ -1 +0,0 @@ - -- 2.39.2 From 562c6aca96a60be2c0c99eec71cc775ebe06dd18 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Fri, 27 Aug 2021 23:01:35 -0400 Subject: [PATCH 13/18] fixes --- ceod/api/database.py | 7 +++---- tests/ceod/api/test_db_mysql.py | 10 +++++----- tests/ceod/api/test_db_psql.py | 10 +++++----- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/ceod/api/database.py b/ceod/api/database.py index 1a2d169..e2ef131 100644 --- a/ceod/api/database.py +++ b/ceod/api/database.py @@ -1,10 +1,9 @@ -from flask import Blueprint, request +from flask import Blueprint from zope import component from functools import wraps -from ceod.api.utils import authz_restrict_to_staff, authz_restrict_to_syscom, \ - user_is_in_group, requires_authentication_no_realm, \ - create_streaming_response, development_only +from ceod.api.utils import authz_restrict_to_syscom, user_is_in_group, requires_authentication_no_realm, \ + development_only from ceo_common.errors import UserNotFoundError, DatabaseConnectionError, DatabasePermissionError, \ InvalidUsernameError, UserAlreadyExistsError from ceo_common.interfaces import ILDAPService, IDatabaseService diff --git a/tests/ceod/api/test_db_mysql.py b/tests/ceod/api/test_db_mysql.py index 0c3f9a7..c956a95 100644 --- a/tests/ceod/api/test_db_mysql.py +++ b/tests/ceod/api/test_db_mysql.py @@ -22,7 +22,7 @@ def test_api_create_mysql_db(cfg, client, g_admin_ctx, create_user_result): assert status == 409 # normal user cannot create db for others - status, data = client.post(f"/api/mysql/someone_else", json={}, principal=uid) + status, data = client.post("/api/mysql/someone_else", json={}, principal=uid) assert status == 403 # cannot create db for user not in ldap @@ -85,9 +85,9 @@ def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, create_user_result): old_passwd = data['password'] con = connect( - host=cfg.get('ceod_database_host'), - user=uid, - password=old_passwd, + host=cfg.get('ceod_database_host'), + user=uid, + password=old_passwd, ) con.close() @@ -104,7 +104,7 @@ def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, create_user_result): assert status == 403 # cannot password reset a user that does not have a database - status, data = client.post(f"/api/mysql/someone_else/pwreset", json={}) + status, data = client.post("/api/mysql/someone_else/pwreset", json={}) assert status == 404 con = connect( diff --git a/tests/ceod/api/test_db_psql.py b/tests/ceod/api/test_db_psql.py index ac5671a..944aa65 100644 --- a/tests/ceod/api/test_db_psql.py +++ b/tests/ceod/api/test_db_psql.py @@ -21,7 +21,7 @@ def test_api_create_psql_db(cfg, client, g_admin_ctx, create_user_result): assert status == 409 # normal user cannot create db for others - status, data = client.post(f"/api/postgresql/someone_else", json={}, principal=uid) + status, data = client.post("/api/postgresql/someone_else", json={}, principal=uid) assert status == 403 # cannot create db for user not in ldap @@ -84,9 +84,9 @@ def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, create_user_result): old_passwd = data['password'] con = connect( - host=cfg.get('ceod_database_host'), - user=uid, - password=old_passwd, + host=cfg.get('ceod_database_host'), + user=uid, + password=old_passwd, ) con.close() @@ -103,7 +103,7 @@ def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, create_user_result): assert status == 403 # cannot password reset a user that does not have a database - status, data = client.post(f"/api/postgresql/someone_else/pwreset", json={}) + status, data = client.post("/api/postgresql/someone_else/pwreset", json={}) assert status == 404 con = connect( -- 2.39.2 From 409894a07d3f0975ef033b921047fb168104e8d8 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Sat, 28 Aug 2021 00:01:56 -0400 Subject: [PATCH 14/18] check db config on startup --- ceod/db/MySQLService.py | 20 ++++++++++++++++++++ ceod/db/PostgreSQLService.py | 20 ++++++++++++++++++++ tests/ceod/api/test_db_mysql.py | 11 ++++++----- tests/ceod/api/test_db_psql.py | 11 ++++++----- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 4a73e5c..a011da3 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -21,6 +21,26 @@ class MySQLService: config = component.getUtility(IConfig) self.auth_username = config.get('mysql_username') self.auth_password = config.get('mysql_password') + try: + test_user = "test_user_64559" + test_perms = f""" + CREATE USER '{test_user}'@'localhost'; + CREATE DATABASE {test_user}; + GRANT ALL PRIVILEGES ON {test_user}.* TO '{test_user}'@'localhost'; + DROP DATABASE {test_user}; + DROP USER '{test_user}'@'localhost'; + """ + with connect( + host='localhost', + user=self.auth_username, + password=self.auth_password, + ) as con: + with con.cursor() as cursor: + cursor.execute(test_perms) + except InterfaceError: + raise Exception('unable to connect or authenticate to sql server') + except ProgrammingError: + raise Exception('insufficient permissions to create users and databases') @contextmanager def mysql_connection(self): diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 6ab3c49..b9f79db 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -21,6 +21,26 @@ class PostgreSQLService: config = component.getUtility(IConfig) self.auth_username = config.get('postgresql_username') self.auth_password = config.get('postgresql_password') + try: + test_user = "test_user_64559" + test_perms = f""" + CREATE USER {test_user}; + CREATE DATABASE {test_user} OWNER {test_user}; + REVOKE ALL ON DATABASE {test_user} FROM PUBLIC; + DROP DATABASE {test_user}; + DROP USER {test_user}; + """ + with connect( + host='localhost', + user=self.auth_username, + password=self.auth_password, + ) as con: + with con.cursor() as cursor: + cursor.execute(test_perms) + except OperationalError: + raise Exception('unable to connect or authenticate to sql server') + except ProgrammingError: + raise Exception('insufficient permissions to create users and databases') @contextmanager def psql_connection(self): diff --git a/tests/ceod/api/test_db_mysql.py b/tests/ceod/api/test_db_mysql.py index c956a95..512bb4b 100644 --- a/tests/ceod/api/test_db_mysql.py +++ b/tests/ceod/api/test_db_mysql.py @@ -5,8 +5,9 @@ from mysql.connector import connect from mysql.connector.errors import InterfaceError, ProgrammingError -def test_api_create_mysql_db(cfg, client, g_admin_ctx, create_user_result): - uid = create_user_result['uid'] +def test_api_create_mysql_db(cfg, client, g_admin_ctx, ldap_user): + uid = ldap_user.uid + with g_admin_ctx(): user = User(uid='someone_else', cn='Some Name', terms=['s2021']) user.add_to_ldap() @@ -72,13 +73,13 @@ def test_api_create_mysql_db(cfg, client, g_admin_ctx, create_user_result): user.remove_from_ldap() -def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, create_user_result): +def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, ldap_user): + uid = ldap_user.uid + with g_admin_ctx(): user = User(uid='someone_else', cn='Some Name', terms=['s2021']) user.add_to_ldap() - uid = create_user_result['uid'] - status, data = client.post(f"/api/mysql/{uid}", json={}) assert status == 200 assert 'password' in data diff --git a/tests/ceod/api/test_db_psql.py b/tests/ceod/api/test_db_psql.py index 944aa65..51a82ee 100644 --- a/tests/ceod/api/test_db_psql.py +++ b/tests/ceod/api/test_db_psql.py @@ -4,8 +4,9 @@ from ceod.model import User from psycopg2 import connect, OperationalError, ProgrammingError -def test_api_create_psql_db(cfg, client, g_admin_ctx, create_user_result): - uid = create_user_result['uid'] +def test_api_create_psql_db(cfg, client, g_admin_ctx, ldap_user): + uid = ldap_user.uid + with g_admin_ctx(): user = User(uid='someone_else', cn='Some Name', terms=['s2021']) user.add_to_ldap() @@ -71,13 +72,13 @@ def test_api_create_psql_db(cfg, client, g_admin_ctx, create_user_result): user.remove_from_ldap() -def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, create_user_result): +def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, ldap_user): + uid = ldap_user.uid + with g_admin_ctx(): user = User(uid='someone_else', cn='Some Name', terms=['s2021']) user.add_to_ldap() - uid = create_user_result['uid'] - status, data = client.post(f"/api/postgresql/{uid}", json={}) assert status == 200 assert 'password' in data -- 2.39.2 From 01e3bef9cadf5178aa45df4b9df29643dc48fb5c Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Sun, 29 Aug 2021 16:31:43 +0000 Subject: [PATCH 15/18] fix tests --- .drone.yml | 5 ++ .drone/auth1-setup.sh | 18 +----- .drone/coffee-setup.sh | 48 ++++++++++++++++ .drone/common.sh | 17 ++++++ .drone/phosphoric-acid-setup.sh | 44 ++++++--------- ceod/api/database.py | 18 +++--- ceod/db/MySQLService.py | 78 +++++++++++--------------- ceod/db/PostgreSQLService.py | 97 +++++++++++++++------------------ tests/ceod/api/test_db_mysql.py | 60 ++++++++++---------- tests/ceod/api/test_db_psql.py | 66 +++++++++++----------- tests/ceod_dev.ini | 3 +- tests/ceod_test_local.ini | 3 +- tests/conftest.py | 27 ++++++++- 13 files changed, 267 insertions(+), 217 deletions(-) create mode 100755 .drone/coffee-setup.sh create mode 100644 .drone/common.sh diff --git a/.drone.yml b/.drone.yml index 7a57909..0dee1cc 100644 --- a/.drone.yml +++ b/.drone.yml @@ -29,6 +29,11 @@ services: commands: - .drone/auth1-setup.sh - sleep infinity + - name: coffee + image: debian:buster + commands: + - .drone/coffee-setup.sh + - sleep infinity trigger: branch: diff --git a/.drone/auth1-setup.sh b/.drone/auth1-setup.sh index bad9a17..115d382 100755 --- a/.drone/auth1-setup.sh +++ b/.drone/auth1-setup.sh @@ -2,23 +2,7 @@ set -ex -# don't resolve container names to *real* CSC machines -sed -E '/^(domain|search)[[:space:]]+csclub.uwaterloo.ca/d' /etc/resolv.conf > /tmp/resolv.conf -cat /tmp/resolv.conf > /etc/resolv.conf -rm /tmp/resolv.conf - -get_ip_addr() { - getent hosts $1 | cut -d' ' -f1 -} - -add_fqdn_to_hosts() { - ip_addr=$1 - hostname=$2 - sed -E "/${ip_addr}.*\\b${hostname}\\b/d" /etc/hosts > /tmp/hosts - cat /tmp/hosts > /etc/hosts - rm /tmp/hosts - echo "$ip_addr $hostname.csclub.internal $hostname" >> /etc/hosts -} +. .drone/common.sh # set FQDN in /etc/hosts add_fqdn_to_hosts $(get_ip_addr $(hostname)) auth1 diff --git a/.drone/coffee-setup.sh b/.drone/coffee-setup.sh new file mode 100755 index 0000000..e84d137 --- /dev/null +++ b/.drone/coffee-setup.sh @@ -0,0 +1,48 @@ +#!/bin/bash + +set -ex + +. .drone/common.sh + +# set FQDN in /etc/hosts +add_fqdn_to_hosts $(get_ip_addr $(hostname)) coffee + +export DEBIAN_FRONTEND=noninteractive +apt update + +apt install --no-install-recommends -y default-mysql-server postgresql + +service mysql stop +sed -E -i 's/^(bind-address[[:space:]]+= 127.0.0.1)$/#\1/' /etc/mysql/mariadb.conf.d/50-server.cnf +service mysql start +cat < $POSTGRES_DIR/pg_hba.conf +# TYPE DATABASE USER ADDRESS METHOD +local all postgres peer +host all postgres 0.0.0.0/0 md5 + +local all all peer +host all all localhost md5 + +local sameuser all md5 +host sameuser all 0.0.0.0/0 md5 +EOF +grep -Eq "^listen_addresses = '*'$" $POSTGRES_DIR/postgresql.conf || \ + echo "listen_addresses = '*'" >> $POSTGRES_DIR/postgresql.conf +service postgresql start +su -c " +cat < /tmp/resolv.conf +cp /tmp/resolv.conf /etc/resolv.conf +rm /tmp/resolv.conf + +get_ip_addr() { + getent hosts $1 | cut -d' ' -f1 +} + +add_fqdn_to_hosts() { + ip_addr=$1 + hostname=$2 + sed -E "/${ip_addr}.*\\b${hostname}\\b/d" /etc/hosts > /tmp/hosts + cp /tmp/hosts /etc/hosts + rm /tmp/hosts + echo "$ip_addr $hostname.csclub.internal $hostname" >> /etc/hosts +} diff --git a/.drone/phosphoric-acid-setup.sh b/.drone/phosphoric-acid-setup.sh index f1d0969..1977dcd 100755 --- a/.drone/phosphoric-acid-setup.sh +++ b/.drone/phosphoric-acid-setup.sh @@ -2,27 +2,26 @@ set -ex -# don't resolve container names to *real* CSC machines -sed -E '/^(domain|search)[[:space:]]+csclub.uwaterloo.ca/d' /etc/resolv.conf > /tmp/resolv.conf -cat /tmp/resolv.conf > /etc/resolv.conf -rm /tmp/resolv.conf +. .drone/common.sh -get_ip_addr() { - getent hosts $1 | cut -d' ' -f1 -} - -add_fqdn_to_hosts() { - ip_addr=$1 - hostname=$2 - sed -E "/${ip_addr}.*\\b${hostname}\\b/d" /etc/hosts > /tmp/hosts - cat /tmp/hosts > /etc/hosts - rm /tmp/hosts - echo "$ip_addr $hostname.csclub.internal $hostname" >> /etc/hosts +sync_with() { + host=$1 + synced=false + # give it 5 minutes + for i in {1..60}; do + if nc -vz $host 9000 ; then + synced=true + break + fi + sleep 5 + done + test $synced = true } # set FQDN in /etc/hosts add_fqdn_to_hosts $(get_ip_addr $(hostname)) phosphoric-acid add_fqdn_to_hosts $(get_ip_addr auth1) auth1 +add_fqdn_to_hosts $(get_ip_addr coffee) coffee export DEBIAN_FRONTEND=noninteractive apt update @@ -41,18 +40,9 @@ cp .drone/nsswitch.conf /etc/nsswitch.conf apt install -y krb5-user libpam-krb5 libsasl2-modules-gssapi-mit cp .drone/krb5.conf /etc/krb5.conf -# sync with auth1 apt install -y netcat-openbsd -synced=false -# give it 5 minutes -for i in {1..60}; do - if nc -vz auth1 9000 ; then - synced=true - break - fi - sleep 5 -done -test $synced = true + +sync_with auth1 rm -f /etc/krb5.keytab cat <', methods=['POST']) @@ -90,11 +94,11 @@ def reset_postgresql_db_passwd(auth_user: str, username: str): @authz_restrict_to_syscom @development_only def delete_mysql_db(username: str): - delete_db_from_type('mysql', username) + return delete_db_from_type('mysql', username) @bp.route('/postgresql/', methods=['DELETE']) @authz_restrict_to_syscom @development_only def delete_postgresql_db(username: str): - delete_db_from_type('postgresl', username) + return delete_db_from_type('postgresql', username) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index a011da3..6e16fb2 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -5,12 +5,15 @@ from contextlib import contextmanager from ceo_common.interfaces import IDatabaseService, IConfig from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError, UserAlreadyExistsError, \ UserNotFoundError +from ceo_common.logger_factory import logger_factory from ceod.utils import gen_password from ceod.db.utils import response_is_empty from mysql.connector import connect from mysql.connector.errors import InterfaceError, ProgrammingError +logger = logger_factory(__name__) + @implementer(IDatabaseService) class MySQLService: @@ -21,39 +24,27 @@ class MySQLService: config = component.getUtility(IConfig) self.auth_username = config.get('mysql_username') self.auth_password = config.get('mysql_password') - try: - test_user = "test_user_64559" - test_perms = f""" - CREATE USER '{test_user}'@'localhost'; - CREATE DATABASE {test_user}; - GRANT ALL PRIVILEGES ON {test_user}.* TO '{test_user}'@'localhost'; - DROP DATABASE {test_user}; - DROP USER '{test_user}'@'localhost'; - """ - with connect( - host='localhost', - user=self.auth_username, - password=self.auth_password, - ) as con: - with con.cursor() as cursor: - cursor.execute(test_perms) - except InterfaceError: - raise Exception('unable to connect or authenticate to sql server') - except ProgrammingError: - raise Exception('insufficient permissions to create users and databases') + self.host = config.get('mysql_host') + + # check that database is up and that we have admin rights + test_user = "test_user_64559" + self.create_db(test_user) + self.delete_db(test_user) @contextmanager def mysql_connection(self): try: with connect( - host='localhost', + host=self.host, user=self.auth_username, password=self.auth_password, ) as con: yield con - except InterfaceError: + except InterfaceError as e: + logger.error(e) raise DatabaseConnectionError() - except ProgrammingError: + except ProgrammingError as e: + logger.error(e) raise DatabasePermissionError() def create_db(self, username: str) -> str: @@ -61,49 +52,42 @@ class MySQLService: search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" search_for_db = f"SHOW DATABASES LIKE '{username}'" create_user = f""" - CREATE USER '{username}'@'localhost' IDENTIFIED BY %(password)s; CREATE USER '{username}'@'%' IDENTIFIED BY %(password)s; """ create_database = f""" CREATE DATABASE {username}; - GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'localhost'; GRANT ALL PRIVILEGES ON {username}.* TO '{username}'@'%'; """ - with self.mysql_connection() as con: - with con.cursor() as cursor: - if response_is_empty(search_for_user, con): - cursor.execute(create_user, {'password': password}) - if response_is_empty(search_for_db, con): - cursor.execute(create_database) - else: - raise UserAlreadyExistsError() - return password + with self.mysql_connection() as con, con.cursor() as cursor: + if response_is_empty(search_for_user, con): + cursor.execute(create_user, {'password': password}) + if response_is_empty(search_for_db, con): + cursor.execute(create_database) + else: + raise UserAlreadyExistsError() + return password def reset_db_passwd(self, username: str) -> str: password = gen_password() search_for_user = f"SELECT user FROM mysql.user WHERE user='{username}'" reset_password = f""" - ALTER USER '{username}'@'localhost' IDENTIFIED BY %(password)s ALTER USER '{username}'@'%' IDENTIFIED BY %(password)s """ - with self.mysql_connection() as con: - with con.cursor() as cursor: - if not response_is_empty(search_for_user, con): - cursor.execute(reset_password, {'password': password}) - else: - raise UserNotFoundError(username) - return password + with self.mysql_connection() as con, con.cursor() as cursor: + if not response_is_empty(search_for_user, con): + cursor.execute(reset_password, {'password': password}) + else: + raise UserNotFoundError(username) + return password def delete_db(self, username: str): drop_db = f"DROP DATABASE IF EXISTS {username}" drop_user = f""" - DROP USER IF EXISTS '{username}'@'localhost'; DROP USER IF EXISTS '{username}'@'%'; """ - with self.mysql_connection() as con: - with con.cursor() as cursor: - cursor.execute(drop_db) - cursor.execute(drop_user) + with self.mysql_connection() as con, con.cursor() as cursor: + cursor.execute(drop_db) + cursor.execute(drop_user) diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index b9f79db..4fe3a1a 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -3,14 +3,17 @@ from zope import component from contextlib import contextmanager from ceo_common.interfaces import IDatabaseService, IConfig -from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError, UserAlreadyExistsError, \ - UserNotFoundError +from ceo_common.errors import DatabaseConnectionError, DatabasePermissionError, \ + UserAlreadyExistsError, UserNotFoundError +from ceo_common.logger_factory import logger_factory from ceod.utils import gen_password from ceod.db.utils import response_is_empty from psycopg2 import connect, OperationalError, ProgrammingError from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT +logger = logger_factory(__name__) + @implementer(IDatabaseService) class PostgreSQLService: @@ -21,80 +24,68 @@ class PostgreSQLService: config = component.getUtility(IConfig) self.auth_username = config.get('postgresql_username') self.auth_password = config.get('postgresql_password') - try: - test_user = "test_user_64559" - test_perms = f""" - CREATE USER {test_user}; - CREATE DATABASE {test_user} OWNER {test_user}; - REVOKE ALL ON DATABASE {test_user} FROM PUBLIC; - DROP DATABASE {test_user}; - DROP USER {test_user}; - """ - with connect( - host='localhost', - user=self.auth_username, - password=self.auth_password, - ) as con: - with con.cursor() as cursor: - cursor.execute(test_perms) - except OperationalError: - raise Exception('unable to connect or authenticate to sql server') - except ProgrammingError: - raise Exception('insufficient permissions to create users and databases') + self.host = config.get('postgresql_host') + + # check that database is up and that we have admin rights + test_user = "test_user_64559" + self.create_db(test_user) + self.delete_db(test_user) @contextmanager def psql_connection(self): + con = None try: - with connect( - host='localhost', + # Don't use the connection as a context manager, because that + # creates a new transaction. + con = connect( + host=self.host, user=self.auth_username, password=self.auth_password, - ) as con: - con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) - yield con - except OperationalError: + ) + con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) + yield con + except OperationalError as e: + logger.error(e) raise DatabaseConnectionError() - except ProgrammingError: + except ProgrammingError as e: + logger.error(e) raise DatabasePermissionError() + finally: + if con is not None: + con.close() def create_db(self, username: str) -> str: password = gen_password() search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" search_for_db = f"SELECT FROM pg_database WHERE datname='{username}'" create_user = f"CREATE USER {username} WITH PASSWORD %(password)s" - create_database = f""" - CREATE DATABASE {username} OWNER {username}; - REVOKE ALL ON DATABASE {username} FROM PUBLIC; - """ + create_database = f"CREATE DATABASE {username} OWNER {username}" + revoke_perms = f"REVOKE ALL ON DATABASE {username} FROM PUBLIC" - with self.psql_connection() as con: - with con.cursor() as cursor: - if response_is_empty(search_for_user, con): - cursor.execute(create_user, {'password': password}) - if response_is_empty(search_for_db, con): - cursor.execute(create_database) - else: - raise UserAlreadyExistsError() - return password + with self.psql_connection() as con, con.cursor() as cursor: + if not response_is_empty(search_for_user, con): + raise UserAlreadyExistsError() + cursor.execute(create_user, {'password': password}) + if response_is_empty(search_for_db, con): + cursor.execute(create_database) + cursor.execute(revoke_perms) + return password def reset_db_passwd(self, username: str) -> str: password = gen_password() search_for_user = f"SELECT FROM pg_roles WHERE rolname='{username}'" reset_password = f"ALTER USER {username} WITH PASSWORD %(password)s" - with self.psql_connection() as con: - with con.cursor() as cursor: - if not response_is_empty(search_for_user, con): - cursor.execute(reset_password, {'password': password}) - else: - raise UserNotFoundError(username) - return password + with self.psql_connection() as con, con.cursor() as cursor: + if response_is_empty(search_for_user, con): + raise UserNotFoundError(username) + cursor.execute(reset_password, {'password': password}) + return password def delete_db(self, username: str): drop_db = f"DROP DATABASE IF EXISTS {username}" drop_user = f"DROP USER IF EXISTS {username}" - with self.psql_connection() as con: - with con.cursor() as cursor: - cursor.execute(drop_db) - cursor.execute(drop_user) + with self.psql_connection() as con, con.cursor() as cursor: + cursor.execute(drop_db) + cursor.execute(drop_user) diff --git a/tests/ceod/api/test_db_mysql.py b/tests/ceod/api/test_db_mysql.py index 512bb4b..345e8b4 100644 --- a/tests/ceod/api/test_db_mysql.py +++ b/tests/ceod/api/test_db_mysql.py @@ -5,7 +5,7 @@ from mysql.connector import connect from mysql.connector.errors import InterfaceError, ProgrammingError -def test_api_create_mysql_db(cfg, client, g_admin_ctx, ldap_user): +def test_api_create_mysql_db(cfg, client, g_admin_ctx, ldap_user, krb_user): uid = ldap_user.uid with g_admin_ctx(): @@ -13,87 +13,85 @@ def test_api_create_mysql_db(cfg, client, g_admin_ctx, ldap_user): user.add_to_ldap() # user should be able to create db for themselves - status, data = client.post(f"/api/mysql/{uid}", json={}, principal=uid) + status, data = client.post(f"/api/db/mysql/{uid}", json={}, principal=uid) assert status == 200 assert 'password' in data passwd = data['password'] # conflict if attempting to create db when already has one - status, data = client.post(f"/api/mysql/{uid}", json={}, principal=uid) + status, data = client.post(f"/api/db/mysql/{uid}", json={}, principal=uid) assert status == 409 # normal user cannot create db for others - status, data = client.post("/api/mysql/someone_else", json={}, principal=uid) + status, data = client.post("/api/db/mysql/someone_else", json={}, principal=uid) assert status == 403 # cannot create db for user not in ldap - status, data = client.post("/api/mysql/user_not_found", json={}) + status, data = client.post("/api/db/mysql/user_not_found", json={}) assert status == 404 # cannot create db when username contains symbols - status, data = client.post("/api/mysql/#invalid", json={}) + status, data = client.post("/api/db/mysql/!invalid", json={}) assert status == 400 with connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('mysql_host'), user=uid, password=passwd, - ) as con: - with con.cursor() as cur: - cur.execute("SHOW DATABASES") - response = cur.fetchall() - assert len(response) == 2 + ) as con, con.cursor() as cur: + cur.execute("SHOW DATABASES") + response = cur.fetchall() + assert len(response) == 2 - with pytest.raises(ProgrammingError): - cur.execute("CREATE DATABASE new_db") + with pytest.raises(ProgrammingError): + cur.execute("CREATE DATABASE new_db") - status, data = client.delete(f"/api/mysql/{uid}", json={}) + status, data = client.delete(f"/api/db/mysql/{uid}", json={}) assert status == 200 # user should be deleted - with pytest.raises(InterfaceError): + with pytest.raises(ProgrammingError): con = connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('mysql_host'), user=uid, password=passwd, ) # db should be deleted with connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('mysql_host'), user=cfg.get('mysql_username'), password=cfg.get('mysql_password'), - ) as con: - with con.cursor() as cur: - cur.execute(f"SHOW DATABASES LIKE '{uid}'") - response = cur.fetchall() - assert len(response) == 0 + ) as con, con.cursor() as cur: + cur.execute(f"SHOW DATABASES LIKE '{uid}'") + response = cur.fetchall() + assert len(response) == 0 with g_admin_ctx(): user.remove_from_ldap() -def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, ldap_user): +def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, ldap_user, krb_user): uid = ldap_user.uid with g_admin_ctx(): user = User(uid='someone_else', cn='Some Name', terms=['s2021']) user.add_to_ldap() - status, data = client.post(f"/api/mysql/{uid}", json={}) + status, data = client.post(f"/api/db/mysql/{uid}", json={}) assert status == 200 assert 'password' in data old_passwd = data['password'] con = connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('mysql_host'), user=uid, password=old_passwd, ) con.close() # normal user can get a password reset for themselves - status, data = client.post(f"/api/mysql/{uid}/pwreset", json={}, principal=uid) + status, data = client.post(f"/api/db/mysql/{uid}/pwreset", json={}, principal=uid) assert status == 200 assert 'password' in data new_passwd = data['password'] @@ -101,21 +99,21 @@ def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, ldap_user): assert old_passwd != new_passwd # normal user cannot reset password for others - status, data = client.post(f"/api/mysql/{uid}/pwreset", json={}, principal='someone_else') + status, data = client.post(f"/api/db/mysql/someone_else/pwreset", json={}, principal=uid) assert status == 403 # cannot password reset a user that does not have a database - status, data = client.post("/api/mysql/someone_else/pwreset", json={}) + status, data = client.post("/api/db/mysql/someone_else/pwreset", json={}) assert status == 404 con = connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('mysql_host'), user=uid, password=new_passwd, ) con.close() - status, data = client.delete(f"/api/mysql/{uid}", json={}) + status, data = client.delete(f"/api/db/mysql/{uid}", json={}) assert status == 200 with g_admin_ctx(): diff --git a/tests/ceod/api/test_db_psql.py b/tests/ceod/api/test_db_psql.py index 51a82ee..8070fcf 100644 --- a/tests/ceod/api/test_db_psql.py +++ b/tests/ceod/api/test_db_psql.py @@ -4,7 +4,7 @@ from ceod.model import User from psycopg2 import connect, OperationalError, ProgrammingError -def test_api_create_psql_db(cfg, client, g_admin_ctx, ldap_user): +def test_api_create_psql_db(cfg, client, g_admin_ctx, ldap_user, krb_user): uid = ldap_user.uid with g_admin_ctx(): @@ -12,87 +12,88 @@ def test_api_create_psql_db(cfg, client, g_admin_ctx, ldap_user): user.add_to_ldap() # user should be able to create db for themselves - status, data = client.post(f"/api/postgresql/{uid}", json={}, principal=uid) + status, data = client.post(f"/api/db/postgresql/{uid}", json={}, principal=uid) assert status == 200 assert 'password' in data passwd = data['password'] # conflict if attempting to create db when already has one - status, data = client.post(f"/api/postgresql/{uid}", json={}, principal=uid) + status, data = client.post(f"/api/db/postgresql/{uid}", json={}, principal=uid) assert status == 409 # normal user cannot create db for others - status, data = client.post("/api/postgresql/someone_else", json={}, principal=uid) + status, data = client.post("/api/db/postgresql/someone_else", json={}, principal=uid) assert status == 403 # cannot create db for user not in ldap - status, data = client.post("/api/postgresql/user_not_found", json={}) + status, data = client.post("/api/db/postgresql/user_not_found", json={}) assert status == 404 # cannot create db when username contains symbols - status, data = client.post("/api/postgresql/#invalid", json={}) + status, data = client.post("/api/db/postgresql/!invalid", json={}) assert status == 400 - with connect( - host=cfg.get('ceod_database_host'), + con = connect( + host=cfg.get('postgresql_host'), user=uid, password=passwd, - ) as con: - with con.cursor() as cur: - cur.execute("SHOW DATABASES") - response = cur.fetchall() - assert len(response) == 2 + ) + con.autocommit = True + with con.cursor() as cur: + cur.execute("SELECT datname FROM pg_database") + response = cur.fetchall() + # 3 of the 4 are postgres, template0, template1 + assert len(response) == 4 + with pytest.raises(ProgrammingError): + cur.execute("CREATE DATABASE new_db") + con.close() - with pytest.raises(ProgrammingError): - cur.execute("CREATE DATABASE new_db") - - status, data = client.delete(f"/api/postgresql/{uid}", json={}) + status, data = client.delete(f"/api/db/postgresql/{uid}", json={}) assert status == 200 # user should be deleted with pytest.raises(OperationalError): con = connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('postgresql_host'), user=uid, password=passwd, ) # db should be deleted with connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('postgresql_host'), user=cfg.get('postgresql_username'), password=cfg.get('postgresql_password'), - ) as con: - with con.cursor() as cur: - cur.execute(f"SHOW DATABASES LIKE '{uid}'") - response = cur.fetchall() - assert len(response) == 0 + ) as con, con.cursor() as cur: + cur.execute(f"SELECT datname FROM pg_database WHERE datname = '{uid}'") + response = cur.fetchall() + assert len(response) == 0 with g_admin_ctx(): user.remove_from_ldap() -def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, ldap_user): +def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, ldap_user, krb_user): uid = ldap_user.uid with g_admin_ctx(): user = User(uid='someone_else', cn='Some Name', terms=['s2021']) user.add_to_ldap() - status, data = client.post(f"/api/postgresql/{uid}", json={}) + status, data = client.post(f"/api/db/postgresql/{uid}", json={}) assert status == 200 assert 'password' in data old_passwd = data['password'] con = connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('postgresql_host'), user=uid, password=old_passwd, ) con.close() # normal user can get a password reset for themselves - status, data = client.post(f"/api/postgresql/{uid}/pwreset", json={}, principal=uid) + status, data = client.post(f"/api/db/postgresql/{uid}/pwreset", json={}, principal=uid) assert status == 200 assert 'password' in data new_passwd = data['password'] @@ -100,21 +101,22 @@ def test_api_passwd_reset_psql(cfg, client, g_admin_ctx, ldap_user): assert old_passwd != new_passwd # normal user cannot reset password for others - status, data = client.post(f"/api/postgresql/{uid}/pwreset", json={}, principal='someone_else') + status, data = client.post("/api/db/postgresql/someone_else/pwreset", + json={}, principal=uid) assert status == 403 # cannot password reset a user that does not have a database - status, data = client.post("/api/postgresql/someone_else/pwreset", json={}) + status, data = client.post("/api/db/postgresql/someone_else/pwreset", json={}) assert status == 404 con = connect( - host=cfg.get('ceod_database_host'), + host=cfg.get('postgresql_host'), user=uid, password=new_passwd, ) con.close() - status, data = client.delete(f"/api/postgresql/{uid}", json={}) + status, data = client.delete(f"/api/db/postgresql/{uid}", json={}) assert status == 200 with g_admin_ctx(): diff --git a/tests/ceod_dev.ini b/tests/ceod_dev.ini index 542d826..f577120 100644 --- a/tests/ceod_dev.ini +++ b/tests/ceod_dev.ini @@ -61,8 +61,9 @@ available = president,vice-president,treasurer,secretary, [mysql] username = mysql password = mysql +host = localhost [postgresql] username = postgres password = postgres - +host = localhost diff --git a/tests/ceod_test_local.ini b/tests/ceod_test_local.ini index 111215d..25d9dae 100644 --- a/tests/ceod_test_local.ini +++ b/tests/ceod_test_local.ini @@ -60,8 +60,9 @@ available = president,vice-president,treasurer,secretary, [mysql] username = mysql password = mysql +host = coffee [postgresql] username = postgres password = postgres - +host = coffee diff --git a/tests/conftest.py b/tests/conftest.py index 4fefa46..209629c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,6 +6,7 @@ import os import pwd import shutil import subprocess +from subprocess import DEVNULL import sys import time from unittest.mock import patch, Mock @@ -19,9 +20,11 @@ from zope import component from .utils import gssapi_creds_ctx, ccache_cleanup # noqa: F401 from ceo_common.interfaces import IConfig, IKerberosService, ILDAPService, \ - IFileService, IMailmanService, IHTTPClient, IUWLDAPService, IMailService + IFileService, IMailmanService, IHTTPClient, IUWLDAPService, IMailService, \ + IDatabaseService from ceo_common.model import Config, HTTPClient from ceod.api import create_app +from ceod.db import MySQLService, PostgreSQLService from ceod.model import KerberosService, LDAPService, FileService, User, \ MailmanService, Group, UWLDAPService, UWLDAPRecord, MailService import ceod.utils as utils @@ -239,6 +242,20 @@ def mail_srv(cfg, mock_mail_server): return _mail_srv +@pytest.fixture(scope='session') +def mysql_srv(cfg): + mysql_srv = MySQLService() + component.provideUtility(mysql_srv, IDatabaseService, 'mysql') + return mysql_srv + + +@pytest.fixture(scope='session') +def postgresql_srv(cfg): + psql_srv = PostgreSQLService() + component.provideUtility(psql_srv, IDatabaseService, 'postgresql') + return psql_srv + + @pytest.fixture(autouse=True, scope='session') def app( cfg, @@ -248,6 +265,8 @@ def app( mailman_srv, uwldap_srv, mail_srv, + mysql_srv, + postgresql_srv, ): app = create_app({'TESTING': True}) return app @@ -297,7 +316,11 @@ def ldap_user(simple_user, g_admin_ctx): @pytest.fixture def krb_user(simple_user): - simple_user.add_to_kerberos('krb5') + # We don't want to use add_to_kerberos() here because that expires the + # user's password, which we don't want for testing + subprocess.run( + ['kadmin', '-k', '-p', 'ceod/admin', 'addprinc', '-pw', 'krb5', + simple_user.uid], stdout=DEVNULL, check=True) yield simple_user simple_user.remove_from_kerberos() -- 2.39.2 From a812ba94cd740ca759e8266cb89c377ee25577d6 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Sun, 29 Aug 2021 16:34:23 +0000 Subject: [PATCH 16/18] remove DB connection check at startup --- ceod/db/MySQLService.py | 5 ----- ceod/db/PostgreSQLService.py | 5 ----- 2 files changed, 10 deletions(-) diff --git a/ceod/db/MySQLService.py b/ceod/db/MySQLService.py index 6e16fb2..043a906 100644 --- a/ceod/db/MySQLService.py +++ b/ceod/db/MySQLService.py @@ -26,11 +26,6 @@ class MySQLService: self.auth_password = config.get('mysql_password') self.host = config.get('mysql_host') - # check that database is up and that we have admin rights - test_user = "test_user_64559" - self.create_db(test_user) - self.delete_db(test_user) - @contextmanager def mysql_connection(self): try: diff --git a/ceod/db/PostgreSQLService.py b/ceod/db/PostgreSQLService.py index 4fe3a1a..3f3cbb8 100644 --- a/ceod/db/PostgreSQLService.py +++ b/ceod/db/PostgreSQLService.py @@ -26,11 +26,6 @@ class PostgreSQLService: self.auth_password = config.get('postgresql_password') self.host = config.get('postgresql_host') - # check that database is up and that we have admin rights - test_user = "test_user_64559" - self.create_db(test_user) - self.delete_db(test_user) - @contextmanager def psql_connection(self): con = None -- 2.39.2 From 56a80a7567fb7011d8ca629f87fa246a7d0ad794 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Sun, 29 Aug 2021 16:38:20 +0000 Subject: [PATCH 17/18] fix lint errors --- tests/ceod/api/test_db_mysql.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ceod/api/test_db_mysql.py b/tests/ceod/api/test_db_mysql.py index 345e8b4..ff7c26c 100644 --- a/tests/ceod/api/test_db_mysql.py +++ b/tests/ceod/api/test_db_mysql.py @@ -2,7 +2,7 @@ import pytest from ceod.model import User from mysql.connector import connect -from mysql.connector.errors import InterfaceError, ProgrammingError +from mysql.connector.errors import ProgrammingError def test_api_create_mysql_db(cfg, client, g_admin_ctx, ldap_user, krb_user): @@ -99,7 +99,7 @@ def test_api_passwd_reset_mysql(cfg, client, g_admin_ctx, ldap_user, krb_user): assert old_passwd != new_passwd # normal user cannot reset password for others - status, data = client.post(f"/api/db/mysql/someone_else/pwreset", json={}, principal=uid) + status, data = client.post("/api/db/mysql/someone_else/pwreset", json={}, principal=uid) assert status == 403 # cannot password reset a user that does not have a database -- 2.39.2 From 76c1082d4c4bb00216be65d863acc40e8e641549 Mon Sep 17 00:00:00 2001 From: Max Erenberg Date: Sun, 29 Aug 2021 16:58:55 +0000 Subject: [PATCH 18/18] update DB instructions in README --- README.md | 56 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index f2df9f3..2a8e1b9 100644 --- a/README.md +++ b/README.md @@ -33,14 +33,32 @@ On phosphoric-acid, you will additionally need to create a principal called `ceod/admin` (remember to addprinc **and** ktadd). #### Database -create superuser `mysql` with password `mysql` -``` -mysql -u root +**Note**: The instructions below apply to the dev environment only; in +production, the DB superusers should be restricted to the host where +the DB is running. + +Attach to the coffee container, run `mysql`, and run the following: -CREATE USER 'mysql'@'localhost' IDENTIFIED BY 'mysql'; -GRANT ALL PRIVILEGES ON *.* TO 'mysql'@'localhost' WITH GRANT OPTION; ``` -modify superuser `postgres` for password authentication and restrict new users +CREATE USER 'mysql' IDENTIFIED BY 'mysql'; +GRANT ALL PRIVILEGES ON *.* TO 'mysql' WITH GRANT OPTION; +``` +(In prod, the superuser should have '@localhost' appended to its name.) + +Now open /etc/mysql/mariadb.conf.d/50-server.cnf and comment out the following line: +``` +bind-address = 127.0.0.1 +``` +Then restart MariaDB: +``` +systemctl restart mariadb +``` + +Install PostgreSQL in the container: +``` +apt install -y postgresql +``` +Modify the superuser `postgres` for password authentication and restrict new users: ``` su postgres psql @@ -49,7 +67,7 @@ ALTER USER postgres WITH PASSWORD 'postgres'; REVOKE ALL ON SCHEMA public FROM public; GRANT ALL ON SCHEMA public TO postgres; ``` -create a new `pg_hba.conf` to force password authentication +Create a new `pg_hba.conf`: ``` cd /etc/postgresql/// mv pg_hba.conf pg_hba.conf.old @@ -57,23 +75,33 @@ mv pg_hba.conf pg_hba.conf.old ``` # new pg_hba.conf # TYPE DATABASE USER ADDRESS METHOD +local all postgres peer +host all postgres 0.0.0.0/0 md5 + +local all all peer +host all all localhost md5 + +local sameuser all md5 +host sameuser all 0.0.0.0/0 md5 +``` +**Warning**: in prod, the postgres user should only be allowed to connect locally, +so the relevant snippet in pg_hba.conf should look something like +``` local all postgres md5 host all postgres localhost md5 host all postgres 0.0.0.0/0 reject host all postgres ::/0 reject -local sameuser all md5 -host sameuser all 0.0.0.0/0 md5 -host sameuser all ::/0 md5 ``` +Add the following to postgresql.conf: ``` -# modified postgresql.conf -# listen_addresses = 'localhost' -listen_address = '*' +listen_addresses = '*' ``` +Now restart PostgreSQL: ``` systemctl restart postgresql ``` -users can login remotely but superusers (`postgres` and `mysql`) are only allowed to login from the database host +**In prod**, users can login remotely but superusers (`postgres` and `mysql`) are only +allowed to login from the database host. #### Mailman You should create the following mailing lists from the mail container: -- 2.39.2