From 710ceacd7c0b6a4ba608d8185fba7d389fff0e06 Mon Sep 17 00:00:00 2001 From: Darksider3 Date: Sat, 19 Oct 2019 21:56:46 +0200 Subject: [PATCH] Breaking up the code smell regarding the CFG.py! It began smelling already but having some duplicate code across the interfaces is still better than having all of it all over the place. It enables to write specific flags which are nice to have. For example, Import.py requires the --Import flag because it WANTS the user to read the whole Help before it acts actually as an importer. When the user supplies something they should know what's currently happening. Also removes the hardcoded dependency on lib.CFG-Calls from most calls which was already embarassingly present. Introduced some db and cfg-variables which doesnt clutter anything but suck much less. In future we provide a set of default arguments and a bare minimum - config_ui as the bare minimum, default as the full blown storm. This is rather big because it also patches several other smells including a bug where a user from the db wouldnt be reported as existent --- Dockerfile | 13 +++---- private/Backup.py | 20 ++++++---- private/Import.py | 28 +++++++++----- private/ListUsers.py | 18 ++++++--- private/editUsers.py | 7 ++++ private/lib/UserExceptions.py | 1 - private/lib/uis/config_ui.py | 2 +- private/lib/uis/default.py | 5 --- private/lib/validator.py | 69 +++++++++++++++++------------------ 9 files changed, 90 insertions(+), 73 deletions(-) create mode 100644 private/editUsers.py diff --git a/Dockerfile b/Dockerfile index 0e3e6ad..8b73220 100644 --- a/Dockerfile +++ b/Dockerfile @@ -26,13 +26,6 @@ ENV TILDE_CONF="/app/data/applicationsconfig.ini" # private/{scripts, administrate.py}, public/{scripts, userapplications.py}, config/userapplicatonsconfig.ini #configs, logs, db COPY config/applicationsconfig.ini /app/data/applicationsconfig.ini - -# admin scripts -COPY private/ /app/admin/ - -# user accessible scripts -# Make TILDE_ENV -COPY public/ /app/user/ #SSH config into /etc :) COPY config/etc /etc @@ -41,5 +34,11 @@ RUN touch /app/data/applications.log # Doesnt work, @TODO why #RUN setfacl -R -m u:tilde:rwx /app/data/ RUN chown -R tilde /app/data +# admin scripts +COPY private/ /app/admin/ + +# user accessible scripts +# Make TILDE_ENV +COPY public/ /app/user/ RUN mkdir /app/user/.ssh CMD ["sh", "-c", " echo TILDE_CONF=$TILDE_CONF > /app/user/.ssh/environment && exec /usr/sbin/sshd -D"] diff --git a/private/Backup.py b/private/Backup.py index faa1063..bd324e2 100644 --- a/private/Backup.py +++ b/private/Backup.py @@ -3,7 +3,14 @@ import ListUsers import csv import io -import lib.CFG +import configparser +import lib.uis.default as default_cmd # Follows -u, -a, -f flags + + +default_cmd.argparser.description += " - Backups Tilde Users to stdout or a file." +args = default_cmd.argparser.parse_args() +config = configparser.ConfigParser() +config.read(args.config) class Backup: @@ -12,7 +19,7 @@ class Backup: dialect: str field_names: tuple - def __init__(self, fname: str = lib.CFG.args.file, quoting: int = csv.QUOTE_NONNUMERIC, dialect: str = "excel"): + def __init__(self, fname: str, quoting: int = csv.QUOTE_NONNUMERIC, dialect: str = "excel"): self.setFilename(fname) self.setQuoting(quoting) self.setDialect(dialect) @@ -46,13 +53,10 @@ class Backup: if __name__ == "__main__": try: - L = ListUsers.ListUsers() + L = ListUsers.ListUsers(config['DEFAULT']['applications_db'], uap=args.unapproved, app=args.approved) fetch = L.getFetch() - B = Backup() - if lib.CFG.args.Import: - print("For importing please call the ./Import.py file with the --Import flag") - else: - B.BackupToFile(fetch) + B = Backup(args.file) + B.BackupToFile(fetch) exit(0) except KeyboardInterrupt as e: pass diff --git a/private/Import.py b/private/Import.py index 0686db2..20a7a7c 100644 --- a/private/Import.py +++ b/private/Import.py @@ -1,11 +1,21 @@ -import lib.CFG import csv import os +import configparser import lib.UserExceptions +import lib.uis.config_ui # dont go to default, just following -c flag +ArgParser = lib.uis.config_ui.argparser +ArgParser.description += "- Imports a CSV file consisting of user specific details to the database" +ArgParser.add_argument('-f', '--file', default="stdout", + type=str, help='Import from CSV file', required=True) +ArgParser.add_argument('--Import', default=False, action="store_true", + help="Import Users.", required=True) +args = ArgParser.parse_args() +config = configparser.ConfigParser() +config.read(args.config) -def ImportFromFile(fname: str = lib.CFG.args.file, db: str = lib.CFG.config['DEFAULT']['applications_db'], - userids: tuple = tuple([])): + +def ImportFromFile(fname: str, db: str, userids: tuple = tuple([])): if not os.path.isfile(fname): print(f"File {fname} don't exist") return None @@ -18,14 +28,14 @@ def ImportFromFile(fname: str = lib.CFG.args.file, db: str = lib.CFG.config['DEF try: with open(fname, 'r', newline='') as f: import lib.validator - err = lib.validator.checkImportFile(f) + sql = lib.sqlitedb.SQLitedb(db) + err = lib.validator.checkImportFile(fname, db) if err is not True: print(err) exit(0) import lib.sqlitedb import lib.System sysctl = lib.System.System() - sql = lib.sqlitedb.SQLitedb(lib.CFG.config['DEFAULT']['applications_db']) reader = csv.DictReader(f) # @TODO csv.Sniffer to compare? When yes, give force-accept option for row in reader: if row["status"] == "1": @@ -64,14 +74,14 @@ def ImportFromFile(fname: str = lib.CFG.args.file, db: str = lib.CFG.config['DEF if __name__ == "__main__": try: - if not lib.CFG.args.Import: + if not args.Import: print("Error, need the import flag") - if not lib.CFG.args.file: + if not args.file: print("Error, need the import file") - if not lib.CFG.args.file: + if not args.file: print("You MUST set a CSV-file with the -f/--file flag that already exist") exit(1) - ImportFromFile() + ImportFromFile(args.file, config['DEFAULT']['applications_db']) exit(0) except KeyboardInterrupt as e: pass diff --git a/private/ListUsers.py b/private/ListUsers.py index 2851073..48fcdc8 100644 --- a/private/ListUsers.py +++ b/private/ListUsers.py @@ -1,15 +1,21 @@ #!/usr/bin/env python3 from lib.sqlitedb import SQLitedb -import lib.CFG +import configparser +import lib.uis.default as default_cmd # Follows -u, -a, -f flags + +default_cmd.argparser.description += " - Lists Users from the Tilde database." +args = default_cmd.argparser.parse_args() +config = configparser.ConfigParser() +config.read(args.config) class ListUsers: db = None usersFetch = None - def __init__(self, uap: bool = lib.CFG.args.unapproved, app: bool = lib.CFG.args.approved): - self.db = SQLitedb(lib.CFG.config['DEFAULT']['applications_db']) + def __init__(self, db: str, uap: bool = False, app: bool = True): + self.db = SQLitedb(db) if uap: # only unapproved users query = "SELECT * FROM `applications` WHERE status = '0'" elif app: # Approved users @@ -33,7 +39,7 @@ class ListUsers: if __name__ == "__main__": try: ret = "" - L = ListUsers() + L = ListUsers(config['DEFAULT']['applications_db'], uap=args.unapproved, app=args.approved) fetch = L.getFetch() # @TODO MAYBE best solution: https://pypi.org/project/texttable/ # examle: @@ -64,8 +70,8 @@ print(t.draw()) ret += "%-4i| %-14s| %-25s| %-22s| %-8s | %-5i |\n" % ( user["id"], user["username"], user["email"], user["name"], user["timestamp"], user["status"] ) - if lib.CFG.args.file != "stdout": - with open(lib.CFG.args.file, 'w') as f: + if args.file != "stdout": + with open(args.file, 'w') as f: print(ret, file=f) else: print(ret) diff --git a/private/editUsers.py b/private/editUsers.py new file mode 100644 index 0000000..b38341f --- /dev/null +++ b/private/editUsers.py @@ -0,0 +1,7 @@ +import lib.uis.default + +argparser = lib.uis.default.argparser + +args = argparser.parse_args() +config = argparser.ConfigParser() +config.read(args.config) diff --git a/private/lib/UserExceptions.py b/private/lib/UserExceptions.py index 926c8e5..a3e4637 100644 --- a/private/lib/UserExceptions.py +++ b/private/lib/UserExceptions.py @@ -44,4 +44,3 @@ class UsernameTooLong(User): class UsernameInvalidCharacters(User): pass - diff --git a/private/lib/uis/config_ui.py b/private/lib/uis/config_ui.py index 28fb370..6f70e82 100644 --- a/private/lib/uis/config_ui.py +++ b/private/lib/uis/config_ui.py @@ -1,6 +1,6 @@ import argparse import lib.cwd -argparser = argparse.ArgumentParser(description='Tilde administration tools') +argparser = argparse.ArgumentParser(description='Tilde administration tools ', conflict_handler="resolve") argparser.add_argument('-c', '--config', default=lib.cwd.cwd, type=str, help='Path to configuration file', required=False) diff --git a/private/lib/uis/default.py b/private/lib/uis/default.py index 38f9b8f..6435e73 100644 --- a/private/lib/uis/default.py +++ b/private/lib/uis/default.py @@ -9,8 +9,3 @@ argparser.add_argument('-a', '--approved', default=False, action="store_true", help="Only approved Users.", required=False) argparser.add_argument('-f', '--file', default="stdout", type=str, help='write to file instead of stdout', required=False) -argparser.add_argument('--Import', default=False, action="store_true", - help="Import Users from file. Affects currently only Import.py.\n" - "Setting this to true will result in -f being interpreted as the input file to import " - "users from. The file MUST be a comma separated CSV file being readable having it's " - "defined columns written in the first line.") diff --git a/private/lib/validator.py b/private/lib/validator.py index 6019048..a5c3be2 100644 --- a/private/lib/validator.py +++ b/private/lib/validator.py @@ -1,7 +1,6 @@ import re import pwd import lib.sqlitedb -import lib.CFG def checkUsernameCharacters(username: str): @@ -21,20 +20,18 @@ def checkUsernameLength(username: str): return True -def checkUserExists(username: str): +def checkUserExists(username: str, db: str): try: pwd.getpwnam(username) except KeyError: return True # User already exists else: - if checkUserInDB(username): - return True return False -def checkUserInDB(username: str): +def checkUserInDB(username: str, db: str): try: - ldb = lib.sqlitedb.SQLitedb(lib.CFG.config['DEFAULT']['applications_db']) + ldb = lib.sqlitedb.SQLitedb(db) fetched = ldb.safequery("SELECT * FROM 'applications' WHERE username = ?", tuple([username])) if fetched: return True @@ -78,39 +75,39 @@ def checkDatetimeFormat(form: str): return True -def checkImportFile(f): - import csv - reador = csv.DictReader(f) +def checkImportFile(fname: str, db: str): error_list = str() valid = True ln = 1 # line number - - for row in reador: - # if any of this fails move on to the next user, just print a relatively helpful message lel - if not lib.validator.checkUsernameCharacters(row["username"]): - error_list += (f"Line {ln}: Username contains unsupported characters or starts with a number: '" - f"{row['username']}'.\n") - valid = False - if not lib.validator.checkUsernameLength(row["username"]): - error_list += f"Line {ln}: Username '{row['username']}' is either too long(>16) or short(<3)\n" - valid = False - if not lib.validator.checkSSHKey(row["pubkey"]): - error_list += f"Line {ln}: Following SSH-Key of user '{row['username']}' isn't valid: '{row['pubkey']}'."\ - f"\n" - valid = False - if not lib.validator.checkEmail(row["email"]): - error_list += f"Line {ln}: E-Mail address of user '{row['username']}' '{row['email']}' is not valid.\n" - valid = False - if not lib.validator.checkUserExists(row["username"]): - error_list += f"Line {ln}: User '{row['username']}' already exists.\n" - valid = False - if not lib.validator.checkDatetimeFormat(row["timestamp"]): - error_list += f"Line {ln}: Timestamp '{row['timestamp']}' from user '{row['username']}' is invalid.\n" - valid = False - if int(row["status"]) > 1 or int(row["status"]) < 0: - error_list += f"Line {ln}: Status '{row['status']}' MUST be either 0 or 1.\n" - valid = False - ln += 1 + with open(fname, 'r', newline='') as f: + import csv + reador = csv.DictReader(f) + for row in reador: + # if any of this fails move on to the next user, just print a relatively helpful message lel + if not lib.validator.checkUsernameCharacters(row["username"]): + error_list += (f"Line {ln}: Username contains unsupported characters or starts with a number: '" + f"{row['username']}'.\n") + valid = False + if not lib.validator.checkUsernameLength(row["username"]): + error_list += f"Line {ln}: Username '{row['username']}' is either too long(>16) or short(<3)\n" + valid = False + if not lib.validator.checkSSHKey(row["pubkey"]): + error_list += f"Line {ln}: Following SSH-Key of user '{row['username']}' isn't valid: '{row['pubkey']}'."\ + f"\n" + valid = False + if not lib.validator.checkEmail(row["email"]): + error_list += f"Line {ln}: E-Mail address of user '{row['username']}' '{row['email']}' is not valid.\n" + valid = False + if not lib.validator.checkUserExists(row["username"], db) or checkUserInDB(row["username"], db): + error_list += f"Line {ln}: User '{row['username']}' already exists.\n" + valid = False + if not lib.validator.checkDatetimeFormat(row["timestamp"]): + error_list += f"Line {ln}: Timestamp '{row['timestamp']}' from user '{row['username']}' is invalid.\n" + valid = False + if int(row["status"]) > 1 or int(row["status"]) < 0: + error_list += f"Line {ln}: Status '{row['status']}' MUST be either 0 or 1.\n" + valid = False + ln += 1 if valid: return True else: