From 2778fd584201cafe5120900f34fdab9c7604c753 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:29:20 -0400 Subject: [PATCH 01/28] orbit: hyperspace: do_bcrypt_hash: remove get parameter Given that almost all the callers are using get=True, why even have a get parameter at all. Callers can just wrap a call to this function in a call to print to get the behavior of get=False. --- orbit/hyperspace.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index f8f39260..3a97e69f 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -80,7 +80,7 @@ def do_validate_creds(args): def do_change_password(args): need(args, u=True, p=True) - new_hash = do_bcrypt_hash(args, get=True) + new_hash = do_bcrypt_hash(args) query = (db.User .update({db.User.pwdhash: new_hash}) .where(db.User.username == args.username)) @@ -99,19 +99,15 @@ def do_delete_user(args): print(args.username) -def do_bcrypt_hash(args, get=False): +def do_bcrypt_hash(args): need(args, p=True) - res = str(bcrypt.hashpw(bytes(args.password, "UTF-8"), - bcrypt.gensalt()), "UTF-8") - if get: - return res - else: - print(res) + return bcrypt.hashpw(args.password.encode('utf-8'), + bcrypt.gensalt()).decode('utf-8') def do_newuser(args): need(args, u=True, p=True) - new_hash = do_bcrypt_hash(args, get=True) + new_hash = do_bcrypt_hash(args) try: db.User.create(username=args.username, pwdhash=new_hash, student_id=args.studentid) @@ -176,7 +172,7 @@ def hyperspace_main(raw_args): dest='do', const=do_delete_user) actions.add_argument('-b', '--bcrypthash', action='store_const', help='Generate bcrypt hash from supplied password', - dest='do', const=do_bcrypt_hash) + dest='do', const=lambda args: print(do_bcrypt_hash(args))) # NOQA: E501 actions.add_argument('-l', '--listsessions', action='store_const', help='List of all known sessions (some could be invalid)', # NOQA: E501 dest='do', const=do_list_sessions) From 25991211ac1e7dcdffe6c68b3c77d87453f27217 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:30:43 -0400 Subject: [PATCH 02/28] orbit: hyperspace: remove ability to perform bcrypt hash Unclear when this would ever be useful (maybe it was added because we used to need to manually edit the synapse db to change passwords?) --- orbit/hyperspace.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 3a97e69f..0daf67fb 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -170,9 +170,6 @@ def hyperspace_main(raw_args): actions.add_argument('-w', '--withdrawuser', action='store_const', help='Delete ("withdraw") the supplied username', dest='do', const=do_delete_user) - actions.add_argument('-b', '--bcrypthash', action='store_const', - help='Generate bcrypt hash from supplied password', - dest='do', const=lambda args: print(do_bcrypt_hash(args))) # NOQA: E501 actions.add_argument('-l', '--listsessions', action='store_const', help='List of all known sessions (some could be invalid)', # NOQA: E501 dest='do', const=do_list_sessions) From c6ce16f1305601ee67db0906a6eacb95ef6fbbac Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:32:12 -0400 Subject: [PATCH 03/28] orbit: hyperspace: remove unused -e option A relic of a previous attempt at a dashboard that never was. --- orbit/hyperspace.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 0daf67fb..39db8d38 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -142,8 +142,6 @@ def hyperspace_main(raw_args): parser.add_argument('-p', '--password', help='Password to operate with') parser.add_argument('-i', '--studentid', help='Student ID to operate with') parser.add_argument('-t', '--token', help='Token to operate with') - parser.add_argument('-e', '--exercise', - help='Assignment/Exercise to operate with') actions = parser.add_mutually_exclusive_group() actions.add_argument('-r', '--roster', action='store_const', From f0536e712cb8a1ca6174c967f223b34c8a8d1680 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:33:56 -0400 Subject: [PATCH 04/28] orbit: hyperspace: remove ability to query for session by token unclear when this ever was or will be useful. --- orbit/hyperspace.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 39db8d38..c29687e0 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -16,13 +16,11 @@ def errx(msg): exit(1) -def need(a, u=False, p=False, t=False): +def need(a, u=False, p=False): if u and a.username is None: errx("Need username. Bye.") if p and a.password is None: errx("Need password. Bye.") - if t and a.token is None: - errx("Need token. Bye.") def nou(u): @@ -38,16 +36,6 @@ def do_query_username(args): f'Student ID : {user.student_id}') -def do_validate_token(args): - need(args, t=True) - - ses = db.Session.get_or_none(db.Session.token == args.token) - if ses: - print(ses.username) - else: - print('null') - - def do_drop_session(args): need(args, u=True) query = (db.Session @@ -141,7 +129,6 @@ def hyperspace_main(raw_args): parser.add_argument('-u', '--username', help='Username to operate with') parser.add_argument('-p', '--password', help='Password to operate with') parser.add_argument('-i', '--studentid', help='Student ID to operate with') - parser.add_argument('-t', '--token', help='Token to operate with') actions = parser.add_mutually_exclusive_group() actions.add_argument('-r', '--roster', action='store_const', @@ -150,9 +137,6 @@ def hyperspace_main(raw_args): actions.add_argument('-n', '--newuser', action='store_const', help='Create a new user from supplied credentials', dest='do', const=do_newuser) - actions.add_argument('-s', '--session', action='store_const', - help='Check valitity of supplied token', - dest='do', const=do_validate_token) actions.add_argument('-d', '--dropsession', action='store_const', help='Drop any existing valid session for supplied username', # NOQA: E501 dest='do', const=do_drop_session) From d057efa28d81c919bd5adb8881ca878ad2f33a7f Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:34:29 -0400 Subject: [PATCH 05/28] orbit: hyperspace: remove ability to query specific user It is unclear what this option offers that cannot be achieved by piping the output of roster through grep. --- orbit/hyperspace.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index c29687e0..92ca0d5a 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -27,15 +27,6 @@ def nou(u): errx(f'no such user "{u}". Bye.') -def do_query_username(args): - need(args, u=True) - if not (user := db.User.get_or_none(db.User.username == args.username)): - nou(args.username) - print(f'Username : {user.username}\n' - f'Hashed Password : {user.pwdhash}\n' - f'Student ID : {user.student_id}') - - def do_drop_session(args): need(args, u=True) query = (db.Session @@ -155,9 +146,6 @@ def hyperspace_main(raw_args): actions.add_argument('-l', '--listsessions', action='store_const', help='List of all known sessions (some could be invalid)', # NOQA: E501 dest='do', const=do_list_sessions) - actions.add_argument('-q', '--queryuser', action='store_const', - help='Get information about supplied username if valid', # NOQA: E501 - dest='do', const=do_query_username) args = parser.parse_args(raw_args) if (args.do): From ee7d9fb258bea2e12675c48225aa8a20e3003a86 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:34:53 -0400 Subject: [PATCH 06/28] orbit: hyperspace: remove ability to create sessions unclear when this would be useful. --- orbit/hyperspace.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 92ca0d5a..75aeb0af 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -8,7 +8,6 @@ # internal imports import config -from radius import Session def errx(msg): @@ -40,12 +39,6 @@ def do_drop_session(args): print('null') -def do_create_session(args): - need(args, u=True) - ses = Session(username=args.username) - print(ses.token) - - def do_validate_creds(args): need(args, u=True, p=True) if not (user := db.User.get_or_none(db.User.username == args.username)): @@ -131,9 +124,6 @@ def hyperspace_main(raw_args): actions.add_argument('-d', '--dropsession', action='store_const', help='Drop any existing valid session for supplied username', # NOQA: E501 dest='do', const=do_drop_session) - actions.add_argument('-c', '--createsession', action='store_const', - help='Create session for supplied username', - dest='do', const=do_create_session) actions.add_argument('-v', '--validatecreds', action='store_const', help='Create session for supplied username', dest='do', const=do_validate_creds) From 957023f8ff6766db40abd48b2a6df69f6c8d9a07 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:35:24 -0400 Subject: [PATCH 07/28] orbit: hyperspace: remove ability to validate credentials unclear when this would be useful when you can just try logging into the website to test the credentials. --- orbit/hyperspace.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 75aeb0af..34f0eae3 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -39,17 +39,6 @@ def do_drop_session(args): print('null') -def do_validate_creds(args): - need(args, u=True, p=True) - if not (user := db.User.get_or_none(db.User.username == args.username)): - nou(args.username) - if not bcrypt.checkpw(args.password.encode('utf-8'), - user.pwdhash.encode('utf-8')): - print('null') - return - print(f'credentials(username: {args.username}, password:{args.password})') - - def do_change_password(args): need(args, u=True, p=True) new_hash = do_bcrypt_hash(args) @@ -87,7 +76,6 @@ def do_newuser(args): db.Registration.create(username=args.username, password=args.password, student_id=args.studentid) - do_validate_creds(args) except db.peewee.IntegrityError as e: errx(f'cannot create user with duplicate field: "{e}"') @@ -124,9 +112,6 @@ def hyperspace_main(raw_args): actions.add_argument('-d', '--dropsession', action='store_const', help='Drop any existing valid session for supplied username', # NOQA: E501 dest='do', const=do_drop_session) - actions.add_argument('-v', '--validatecreds', action='store_const', - help='Create session for supplied username', - dest='do', const=do_validate_creds) actions.add_argument('-m', '--mutatepassword', action='store_const', help='Change password for supplied username to supplied password', # NOQA: E501 dest='do', const=do_change_password) From 409d7afcf39e03ba52e6d89a9d0b6b24c4098e2d Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:37:17 -0400 Subject: [PATCH 08/28] orbit: hyperspace: only print output on error for action commands The previous approach of printing 'null' or equivalent under error conditions was fallible (consider a user with username null), and not friendly for use in scripts (should exit with nonzero code if error occurred) Instead, for queries that take an action, nothing is printed and the script exits with zero in the happy case, or an error is printed to stderr and the script exits with a nonzero exit code. --- orbit/hyperspace.py | 11 +++-------- test.sh | 12 +++--------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 34f0eae3..6159dfbd 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -30,13 +30,10 @@ def do_drop_session(args): need(args, u=True) query = (db.Session .delete() - .where(db.Session.username == args.username) - .returning(db.Session)) + .where(db.Session.username == args.username)) - if ses := next(iter(query.execute()), None): - print(ses.username) - else: - print('null') + if query.execute() < 1: + errx('No session belonging to that user found') def do_change_password(args): @@ -47,7 +44,6 @@ def do_change_password(args): .where(db.User.username == args.username)) if query.execute() < 1: nou(args.username) - print(f'credentials(username: {args.username}, password:{args.password})') def do_delete_user(args): @@ -57,7 +53,6 @@ def do_delete_user(args): .where(db.User.username == args.username)) if query.execute() < 1: nou(args.username) - print(args.username) def do_bcrypt_hash(args): diff --git a/test.sh b/test.sh index 7fb34037..eb951888 100755 --- a/test.sh +++ b/test.sh @@ -146,15 +146,9 @@ curl --url "https://$SINGULARITY_HOSTNAME/login" \ | grep "msg = authentication failure" # Check that we can create a user -orbit/warpdrive.sh \ - -u user -p pass -i 1234 -n \ - | tee test/create_user \ - | grep "credentials(username: user, password:pass)" - -add_cleanup "orbit/warpdrive.sh \ - -u user -w \ - | tee test/delete_user \ - | grep 'user'" +orbit/warpdrive.sh -u user -p pass -i 1234 -n + +add_cleanup "orbit/warpdrive.sh -u user -w" # Check that registration fails with incorrect student id curl --url "https://$SINGULARITY_HOSTNAME/register" \ From dc15d1e1a337961b1b5858798f0ee4bf4367d684 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:38:15 -0400 Subject: [PATCH 09/28] orbit: hyperspace: reorder args to group session and user commands This makes the help output a bit nicer --- orbit/hyperspace.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 6159dfbd..3d92e2d5 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -104,9 +104,6 @@ def hyperspace_main(raw_args): actions.add_argument('-n', '--newuser', action='store_const', help='Create a new user from supplied credentials', dest='do', const=do_newuser) - actions.add_argument('-d', '--dropsession', action='store_const', - help='Drop any existing valid session for supplied username', # NOQA: E501 - dest='do', const=do_drop_session) actions.add_argument('-m', '--mutatepassword', action='store_const', help='Change password for supplied username to supplied password', # NOQA: E501 dest='do', const=do_change_password) @@ -116,6 +113,9 @@ def hyperspace_main(raw_args): actions.add_argument('-l', '--listsessions', action='store_const', help='List of all known sessions (some could be invalid)', # NOQA: E501 dest='do', const=do_list_sessions) + actions.add_argument('-d', '--dropsession', action='store_const', + help='Drop any existing valid session for supplied username', # NOQA: E501 + dest='do', const=do_drop_session) args = parser.parse_args(raw_args) if (args.do): From e4324680f91d9bcf53e38b240b9831854ace7b76 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:38:35 -0400 Subject: [PATCH 10/28] orbit: hyperspace: improve help output when no action is specified We can just directly show the help output from the arg parser instead of prompting the user to do it themselves with `-h`. --- orbit/hyperspace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 3d92e2d5..4cb3536c 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -121,7 +121,7 @@ def hyperspace_main(raw_args): if (args.do): args.do(args) else: - print("Nothing to do. Tip: -h") + parser.print_help() if __name__ == "__main__": From 9f11d30fcb3527b59a58d790847e50fc5d4975fc Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:40:53 -0400 Subject: [PATCH 11/28] orbit: hyperspace: improve need If a command requires more than one argument, list all of the required arguments that are missing instead of only the first one. --- orbit/hyperspace.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/orbit/hyperspace.py b/orbit/hyperspace.py index 4cb3536c..6c750901 100755 --- a/orbit/hyperspace.py +++ b/orbit/hyperspace.py @@ -16,10 +16,13 @@ def errx(msg): def need(a, u=False, p=False): + needed = [] if u and a.username is None: - errx("Need username. Bye.") + needed.append('username') if p and a.password is None: - errx("Need password. Bye.") + needed.append('password') + if needed: + errx(f"Need {' and '.join(needed)}. Bye.") def nou(u): From ac5d7eb5b28291d61531c34521e4447640e3e6f8 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:50:39 -0400 Subject: [PATCH 12/28] orbit: radius: replace ad-hoc generation of tokens the standard library secrets package contains useful functions that can replace the logic we were using for generating tokens. This approach is more secure too because the tokens generated by the previous approach could be brute forced (given a username, and knowing that the token is only good for 3 hours there are finitely many inputs to hash function, the one we were using (SHA256) is designed to be fast to compute which is not what you want for a cryptographically secure hash function i.e. you want slow). A spoofed token could be tested against the website and if a match was found, used to hijack a logged in session. --- orbit/radius.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index 5c54cb8f..c165347e 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -3,13 +3,13 @@ # it's all one things now import bcrypt -import hashlib import html import markdown import os import re import subprocess import sys +import secrets from http import HTTPStatus, cookies from datetime import datetime, timedelta from urllib.parse import parse_qs, urlparse @@ -141,8 +141,7 @@ def valid(self): return self.token def mk_hash(self, username): - hash_input = username + str(datetime.now()) - return hashlib.sha256(encode(hash_input)).hexdigest() + return secrets.token_hex() def expired(self): if (expiry := self.expiry) is None or datetime.utcnow() > expiry: From aeb668c6edda55634047ffe6c7c8270246e72dd7 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:51:06 -0400 Subject: [PATCH 13/28] orbit: radius: remove encode and decode helpers string already supports `.encode` and bytes already support `.decode`. Introducing new helpers only makes things more confusing. --- orbit/radius.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index c165347e..6bb1d1c6 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -27,10 +27,6 @@ # === utilities === -def encode(dat): return bytes(dat, "UTF-8") -def decode(dat): return str(dat, "UTF-8") - - def mk_table(row_list, indentation_level=0): # Create elements in first row, and elements afterwards first_row = True @@ -54,7 +50,7 @@ def indenter(adjustment): return '\t' * (indentation_level + adjustment) def check_credentials(username, password): if not (user := db.User.get_or_none(db.User.username == username)): return False - return bcrypt.checkpw(encode(password), encode(user.pwdhash)) + return bcrypt.checkpw(password.encode(), user.pwdhash.encode()) # === user session handling === @@ -275,7 +271,7 @@ def expiry(self): def body_args_query(self, key): return html.escape( - decode(self.body_args.get(encode(key), [b''])[0])) + self.body_args.get(key.encode(), [b''])[0].decode()) def queries_query(self, key): return self.queries.get(key, [''])[0] @@ -319,7 +315,7 @@ def raw_respond(self, response_code, body=b''): def respond(self, response_document): self.headers += [('Content-Type', 'text/html')] response_document = self.format_html(response_document) - return self.raw_respond(HTTPStatus.OK, encode(response_document)) + return self.raw_respond(HTTPStatus.OK, response_document.encode()) form_welcome_template = """ From 8b58352066b38d5429a1a000c90b4a5fb8ee5348 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:51:46 -0400 Subject: [PATCH 14/28] orbit: radius: cgit: prefer bytes.decode over str constructor The decode method is shorter and consistent with the rest of the code. utf-8 is the default option for encoding and decoding, so mentioning it only makes the code longer without subtantially improving the clarity. --- orbit/radius.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orbit/radius.py b/orbit/radius.py index 6bb1d1c6..a30e804e 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -492,7 +492,7 @@ def handle_cgit(rocket): env=cgit_env) so, se = proc.communicate() try: - outstring = str(so, 'UTF-8') + outstring = so.decode() begin = outstring.index('\n\n') return rocket.respond(outstring[begin+2:]) except (UnicodeDecodeError, ValueError) as ex: From ec901b59f7603f722d418370739e9ce93b8579a5 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:55:06 -0400 Subject: [PATCH 15/28] orbit: radius: inline form welcome buttons This variable is only used one place and just inserted into a different string unconditionally so its inclusion is just more confusing than helpful. --- orbit/radius.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index a30e804e..83c1e37b 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -328,16 +328,12 @@ def respond(self, response_document):
- {} +
+ +
""".strip() -form_welcome_buttons = """ -
- -
-""".strip() # NOQA: E501 - form_login = """
@@ -362,8 +358,7 @@ def cookie_info_table(session): def mk_form_welcome(session): - return form_welcome_template.format(cookie_info_table(session), - form_welcome_buttons) + return form_welcome_template.format(cookie_info_table(session)) def handle_login(rocket): From d756be78c6edae8c2994fd669c1b2f78e3badf5d Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:55:51 -0400 Subject: [PATCH 16/28] orbit: radius: upgrade login form to function That way, we can use f-strings instead of the old and unpleasent `%` string formatting. --- orbit/radius.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index 83c1e37b..78f3b84a 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -332,10 +332,16 @@ def respond(self, response_document):
-""".strip() +""".strip() # NOQA: 501 + -form_login = """ -
+def login_form(target_location=None): + if target_location is not None: + target_redir = f'?target={target_location}' + else: + target_redir = '' + return f''' +
@@ -344,8 +350,7 @@ def respond(self, response_document):
-

Need an account? Register here


-""".strip() +

Need an account? Register here


''' def cookie_info_table(session): @@ -374,12 +379,11 @@ def respond(welcome): rocket.headers += [('Location', target)] return rocket.raw_respond(HTTPStatus.SEE_OTHER) elif target: - return rocket.respond(form_login % ({'target_redir': - f'?target={target}'})) + return rocket.respond(login_form(target_location=target)) elif welcome: return rocket.respond(mk_form_welcome(rocket.session)) else: - return rocket.respond(form_login % ({'target_redir': ''})) + return rocket.respond(login_form()) if rocket.session: rocket.msg(f'{rocket.username} authenticated by token') From 827541f7a56535d1d6ef428ca513114345821400 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:56:46 -0400 Subject: [PATCH 17/28] orbit: radius: inline register response The string is only used one place and putting it in the function means that an f-string can be used which is nicer. Also fix the spacing (wasn't indented), and the closing tags (h1 vs h3 mismatch). --- orbit/radius.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index 78f3b84a..08d5d561 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -445,13 +445,6 @@ def handle_dashboard(rocket): """.strip() -register_response = """ -

Save these credentials, you will not be able to access them again


-

Username: %(username)s


-

Password: %(password)s


-""".strip() - - def handle_register(rocket): def form_respond(): return rocket.respond(form_register) @@ -468,10 +461,10 @@ def form_respond(): rocket.msg('no such student') return form_respond() rocket.msg('welcome to the classroom') - return rocket.respond((register_response % { - 'username': registration.username, - 'password': registration.password, - })) + return rocket.respond(f''' +

Save these credentials, you will not be able to access them again


+

Username: {registration.username}


+

Password: {registration.password}


''') def handle_cgit(rocket): From f9c3dfb2cb98c1d4b0e1e5fb2ff1ae86e189e562 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:58:41 -0400 Subject: [PATCH 18/28] orbit: radius: inline form register It is only used in one place so just putting it there is nicer. --- orbit/radius.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index 08d5d561..a4747a44 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -436,18 +436,15 @@ def handle_dashboard(rocket): return handle_stub(rocket, ['dashboard in development, check back later']) -form_register = """ +def handle_register(rocket): + def form_respond(): + return rocket.respond('''

-
-""".strip() + ''') - -def handle_register(rocket): - def form_respond(): - return rocket.respond(form_register) if rocket.method != 'POST': return form_respond() if not (student_id := rocket.body_args_query('student_id')): From d83e5efcce76b4e813f528443777e2f78f318a5f Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 16:59:05 -0400 Subject: [PATCH 19/28] orbit: radius: remove mk_table helper It is only used in one place and it doesn't even work correctly (missing newlines after each row). We can just write the single table that we want to generate manually and use an f-string too. --- orbit/radius.py | 46 +++++++++------------------------------------- 1 file changed, 9 insertions(+), 37 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index a4747a44..74af8d90 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -27,26 +27,6 @@ # === utilities === -def mk_table(row_list, indentation_level=0): - # Create elements in first row, and elements afterwards - first_row = True - def indenter(adjustment): return '\t' * (indentation_level + adjustment) - - output = f'{indenter(0)}' - for row in row_list: - output += f'{indenter(1)}' - for column in row: - if first_row: - output += f'{indenter(2)}' - first_row = False - else: - output += f'{indenter(2)}' - output += f'{indenter(1)}' - output += f'{indenter(0)}
{column}{column}
' - - return output - - def check_credentials(username, password): if not (user := db.User.get_or_none(db.User.username == username)): return False @@ -318,10 +298,16 @@ def respond(self, response_document): return self.raw_respond(HTTPStatus.OK, response_document.encode()) -form_welcome_template = """ +def mk_form_welcome(session): + return f'''
- {} + + + + + +
Cookie KeyValue
Token{session.token}
User{session.username}
Expiry{session.expiry_fmt()}
Welcome!
@@ -331,8 +317,7 @@ def respond(self, response_document):
-
-""".strip() # NOQA: 501 +
''' def login_form(target_location=None): @@ -353,19 +338,6 @@ def login_form(target_location=None):

Need an account? Register here


''' -def cookie_info_table(session): - return mk_table([ - ('Cookie Key', 'Value'), - ('Token', session.token), - ('User', session.username), - ('Expiry', session.expiry_fmt()), - ('Remaining Validity', str(session.expiry - datetime.utcnow()))]) - - -def mk_form_welcome(session): - return form_welcome_template.format(cookie_info_table(session)) - - def handle_login(rocket): target = rocket.queries_query('target') From da8ab4d8d3aba93a4d3759d3e9a5be9d7968d15c Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:02:11 -0400 Subject: [PATCH 20/28] orbit: radius: remove unused str and repr functions --- orbit/radius.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index 74af8d90..dbd469e6 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -141,12 +141,6 @@ def mk_cookie_header(self): return [('Set-Cookie', cookie_val)] - def __repr__(self): - return f'Session({self.token},{self.username},{self.expiry})' - - def __str__(self): - return repr(self) - class Rocket: """ @@ -201,16 +195,6 @@ def __init__(self, env, start_response): self.headers = [] self.body_args = self.read_body_args_wsgi() - def __repr__(self): - return ( - f'Rocket({self.method},{self.path_info},{self.queries},' - f'{str(self.headers)},{self._msg},{str(self.session)},' - f'{self.body_args})' - ) - - def __str__(self): - return repr(self) - def msg(self, msg): self._msg = msg From 7e0afe2322208da677917f237e4ef8847f09dc5a Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:04:20 -0400 Subject: [PATCH 21/28] orbit: radius: rocket: remove unused properties `token` and `expiry` were not used anywhere. Accessing their values only really makes sense if you already know that session is valid, in which case you can just do .session.{token,expiry} which is probably clearer. --- orbit/radius.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/orbit/radius.py b/orbit/radius.py index dbd469e6..1de2f79b 100644 --- a/orbit/radius.py +++ b/orbit/radius.py @@ -223,16 +223,6 @@ def username(self): if session := self.session: return session.username - @property - def token(self): - if session := self.session: - return session.token - - @property - def expiry(self): - if session := self.session: - return session.expiry - def body_args_query(self, key): return html.escape( self.body_args.get(key.encode(), [b''])[0].decode()) From e8e0ca0526cda706c33beafef5f8377404cf2e37 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:05:27 -0400 Subject: [PATCH 22/28] orbit: get dependencies from `apk` instead of `pip` We aren't using any special versions or exotic packages. Just installing the appropriate global python packages from `apk` is easily adequate to get the dependencies for orbit, and it makes all of the invocations more simple because we do not need to enter a virtual environment anymore since the packages are global. --- orbit/Containerfile | 23 ++++++++--------------- orbit/warpdrive.sh | 5 +---- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/orbit/Containerfile b/orbit/Containerfile index 856ff967..9dca995c 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -1,24 +1,14 @@ FROM alpine:3.19 AS build RUN apk update && apk upgrade && apk add \ - python3-dev \ - py3-pip \ - build-base \ - libffi-dev \ + py3-peewee \ envsubst \ ; -COPY requirements.txt /requirements.txt -RUN python3 -m venv /radius-venv && \ - source /radius-venv/bin/activate && \ - pip install -r requirements.txt && \ - : - COPY . /orbit WORKDIR /orbit RUN mkdir -p /var/orbit/ && \ - source /radius-venv/bin/activate && \ ./db.py \ : @@ -42,22 +32,25 @@ COPY --from=orbit_repos_source . /etc/cgit FROM alpine:3.19 AS orbit RUN apk update && apk upgrade && apk add \ - python3 \ + py3-bcrypt \ + py3-peewee \ + py3-markdown \ + uwsgi-python3 \ + uwsgi-http \ cgit \ ; COPY --from=build /orbit /orbit -COPY --from=build /radius-venv /radius-venv COPY --from=build /var/orbit /var/orbit COPY --from=build /var/git /var/git COPY --from=build /etc/cgit /etc/cgit COPY cgitrc /etc/cgitrc -RUN chown -R 100:100 /orbit /radius-venv /var/orbit /var/git +RUN chown -R 100:100 /orbit /var/orbit /var/git USER 100:100 EXPOSE 9098 -CMD /bin/sh -c "source /radius-venv/bin/activate && uwsgi /orbit/radius.ini" +CMD uwsgi --plugin python3,http /orbit/radius.ini diff --git a/orbit/warpdrive.sh b/orbit/warpdrive.sh index ed4ef082..be658446 100755 --- a/orbit/warpdrive.sh +++ b/orbit/warpdrive.sh @@ -12,7 +12,4 @@ require "${DOCKER}" CONTAINER=${CONTAINER:-singularity_orbit_1} -cat < Date: Wed, 1 May 2024 17:07:22 -0400 Subject: [PATCH 23/28] orbit: remove old files Since we don't have a venv anymore, we don't need {,dev-}requirements.txt While we are at it, we can remove the old orbit readme which was an artifact of it being a separate repo. The instructions are now out of date and they have been superseded by the main readme. We also do not need the dockerignore file since the orbit db has not been stored in the orbit folder in a long time. --- orbit/.dockerignore | 1 - orbit/README.md | 6 ------ orbit/dev-requirements.txt | 4 ---- orbit/requirements.txt | 7 ------- 4 files changed, 18 deletions(-) delete mode 100644 orbit/.dockerignore delete mode 100644 orbit/README.md delete mode 100644 orbit/dev-requirements.txt delete mode 100644 orbit/requirements.txt diff --git a/orbit/.dockerignore b/orbit/.dockerignore deleted file mode 100644 index 1b30444a..00000000 --- a/orbit/.dockerignore +++ /dev/null @@ -1 +0,0 @@ -orbit.db diff --git a/orbit/README.md b/orbit/README.md deleted file mode 100644 index 8ebcc009..00000000 --- a/orbit/README.md +++ /dev/null @@ -1,6 +0,0 @@ -# orbit - -For development, setup and enter a python virtualenv for -(e.g. `python -m venv orbit-dev && source orbit-dev/bin/activate`) -and install the development dependencies: `pip install -r dev-requirements.txt` -to enable style checking via `test-style.sh` utilizing flake8. diff --git a/orbit/dev-requirements.txt b/orbit/dev-requirements.txt deleted file mode 100644 index 2c9e2a97..00000000 --- a/orbit/dev-requirements.txt +++ /dev/null @@ -1,4 +0,0 @@ -flake8 ~=7.0.0 -mccabe ~= 0.7.0 -pycodestyle ~= 2.11.1 -pyflakes ~= 3.2.0 diff --git a/orbit/requirements.txt b/orbit/requirements.txt deleted file mode 100644 index a1543266..00000000 --- a/orbit/requirements.txt +++ /dev/null @@ -1,7 +0,0 @@ -bcrypt ~= 3.1.6 -certbot ~= 2.6.0 -Markdown ~= 3.3.7 -urllib3 ~= 1.26.16 -uWSGI ~= 2.0.21 -uwsgi-tools ~= 1.1.1 -peewee ~= 3.17.1 From 57f2e4aec3b45222a07c91d48e32b9b94d3da068 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:10:19 -0400 Subject: [PATCH 24/28] orbit: Containerfile: set workdir and prefer relative paths There are lots of absolute paths scattered around the orbit codebase. By just setting the workdir to the location of the orbit source folder within the Containerfile most of those paths can be converted into relative paths which makes moving the orbit source code easier. --- orbit/Containerfile | 4 +++- orbit/config.py | 5 ++--- orbit/radius.ini | 1 - orbit/warpdrive.sh | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/orbit/Containerfile b/orbit/Containerfile index 9dca995c..2c2e2e08 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -40,6 +40,8 @@ RUN apk update && apk upgrade && apk add \ cgit \ ; +WORKDIR /orbit + COPY --from=build /orbit /orbit COPY --from=build /var/orbit /var/orbit COPY --from=build /var/git /var/git @@ -53,4 +55,4 @@ USER 100:100 EXPOSE 9098 -CMD uwsgi --plugin python3,http /orbit/radius.ini +CMD uwsgi --plugin python3,http ./radius.ini diff --git a/orbit/config.py b/orbit/config.py index 67b781d1..07fe92a3 100644 --- a/orbit/config.py +++ b/orbit/config.py @@ -1,9 +1,8 @@ version_info = '${orbit_version_info}' # read these documents from a filesystem path -orbit_root = '/orbit' -doc_root = f'{orbit_root}/docs' -doc_header = f'{orbit_root}/header.html' +doc_root = './docs' +doc_header = './header.html' database = '/var/orbit/orbit.db' # duration of authentication token validity period diff --git a/orbit/radius.ini b/orbit/radius.ini index bb303375..5d4696c0 100644 --- a/orbit/radius.ini +++ b/orbit/radius.ini @@ -1,6 +1,5 @@ [uwsgi] http = 0.0.0.0:9098 -chdir = /orbit wsgi-file = radius.py disable-logging diff --git a/orbit/warpdrive.sh b/orbit/warpdrive.sh index be658446..19344730 100755 --- a/orbit/warpdrive.sh +++ b/orbit/warpdrive.sh @@ -12,4 +12,4 @@ require "${DOCKER}" CONTAINER=${CONTAINER:-singularity_orbit_1} -$DOCKER exec -i "$CONTAINER" /orbit/hyperspace.py "$@" +$DOCKER exec -i "$CONTAINER" ./hyperspace.py "$@" From 0f3e606af6113646141e68ee2f8930123b574f87 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:11:42 -0400 Subject: [PATCH 25/28] orbit/submatrix: Containerfile: prefer array form of CMD instruction This makes them consistent with the other `Containerfile`s. --- orbit/Containerfile | 2 +- submatrix/Containerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/orbit/Containerfile b/orbit/Containerfile index 2c2e2e08..fddd9360 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -55,4 +55,4 @@ USER 100:100 EXPOSE 9098 -CMD uwsgi --plugin python3,http ./radius.ini +CMD ["uwsgi", "--plugin", "python,http", "./radius.ini"] diff --git a/submatrix/Containerfile b/submatrix/Containerfile index 903c8301..2fdecfcc 100644 --- a/submatrix/Containerfile +++ b/submatrix/Containerfile @@ -16,5 +16,5 @@ VOLUME /var/synapse USER 100:100 -CMD /usr/bin/synapse_homeserver -c /etc/synapse/homeserver.yaml +CMD ["synapse_homeserver", "-c", "/etc/synapse/homeserver.yaml"] From 9d311d87f21e01fab86fa093a10827b3b7c0ab00 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:13:21 -0400 Subject: [PATCH 26/28] orbit: Containerfile: only `chown` files that orbit needs to write The source code and configuration files can be owned by root because user 100 can still read them. Only the orbit db which needs to be modified by the running container ought to be owned by user 100. This adds an additional layer of security since the code on disk cannot be modified without a privilege escalation to the container root user. Fixes: acf767b ("orbit: switch to non-root user within container") --- orbit/Containerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orbit/Containerfile b/orbit/Containerfile index fddd9360..4b6da033 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -49,7 +49,7 @@ COPY --from=build /etc/cgit /etc/cgit COPY cgitrc /etc/cgitrc -RUN chown -R 100:100 /orbit /var/orbit /var/git +RUN chown -R 100:100 /var/orbit USER 100:100 From c06da56490114f4f9a3b0e9aa614bb834cd36885 Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:14:41 -0400 Subject: [PATCH 27/28] orbit: Copy from additional contexts directly into final stage Instead of copying a bunch of files into the first build stage that are otherwise unused except to copy them from build stage into the final image, just copy directly into the final stage. --- orbit/Containerfile | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/orbit/Containerfile b/orbit/Containerfile index 4b6da033..40449cb4 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -19,16 +19,6 @@ RUN test -n "$orbit_version_info" || (echo 'version info is not set' && false) & rm config.py.template \ ; -RUN mkdir \ - /var/git \ - /orbit/docs \ - /etc/cgit \ - ; - -COPY --from=orbit_singularity_git_dir . /var/git/singularity -COPY --from=orbit_docs_source . /orbit/docs -COPY --from=orbit_repos_source . /etc/cgit - FROM alpine:3.19 AS orbit RUN apk update && apk upgrade && apk add \ @@ -43,9 +33,10 @@ RUN apk update && apk upgrade && apk add \ WORKDIR /orbit COPY --from=build /orbit /orbit +COPY --from=orbit_docs_source . ./docs COPY --from=build /var/orbit /var/orbit -COPY --from=build /var/git /var/git -COPY --from=build /etc/cgit /etc/cgit +COPY --from=orbit_singularity_git_dir . /var/git/singularity +COPY --from=orbit_repos_source . /etc/cgit COPY cgitrc /etc/cgitrc From f1515af44647e04dd9c8fbd10635492ef84cc0eb Mon Sep 17 00:00:00 2001 From: charliemirabile <46761267+charliemirabile@users.noreply.github.com> Date: Wed, 1 May 2024 17:15:43 -0400 Subject: [PATCH 28/28] orbit: Containerfile: put orbit code in `/usr/local/share` Instead of creating a nonstandard orbit directory in the filesystem root, we can put the orbit folder within `/usr/local/share`, a standard location for architecture independent data (e.g. python source code) as specified in the FHS. --- container-compose-dev.yml | 2 +- orbit/Containerfile | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/container-compose-dev.yml b/container-compose-dev.yml index 9b48fd88..b0307857 100644 --- a/container-compose-dev.yml +++ b/container-compose-dev.yml @@ -17,5 +17,5 @@ services: volumes: - type: bind source: ./kdlp.underground.software - target: /orbit/docs + target: /usr/local/share/orbit/docs read_only: true diff --git a/orbit/Containerfile b/orbit/Containerfile index 40449cb4..a31ef9ff 100644 --- a/orbit/Containerfile +++ b/orbit/Containerfile @@ -5,8 +5,8 @@ RUN apk update && apk upgrade && apk add \ envsubst \ ; -COPY . /orbit -WORKDIR /orbit +COPY . /usr/local/share/orbit +WORKDIR /usr/local/share/orbit RUN mkdir -p /var/orbit/ && \ ./db.py \ @@ -30,9 +30,9 @@ RUN apk update && apk upgrade && apk add \ cgit \ ; -WORKDIR /orbit +WORKDIR /usr/local/share/orbit -COPY --from=build /orbit /orbit +COPY --from=build /usr/local/share/orbit /usr/local/share/orbit COPY --from=orbit_docs_source . ./docs COPY --from=build /var/orbit /var/orbit COPY --from=orbit_singularity_git_dir . /var/git/singularity