moralcode / classclockapi Goto Github PK

currently the log messages arent the most descriptive as to what is happening.

this should be improved

This would be intended as a way to solve the "its hard to tell how many users you have in open source" problem

This might pair well with features like built-in first-party analytics/usage graphs, and could potentially leverage the data from the heartbeat HEAD requests from the frontend (/v0/ping endpoint) to estimate number of active clients

it seems as though this ClassClock API may be useful on its own, independent of the client app as a way for applications to have their own generic (non-individualized) way to store school bell schedule data that provides a bonus in terms of interoperability with other apps that are based on a ClassClock-formatted API

Since Auth0 is already fairly standards-based, it shouldn't be too crazy to try and abstract it and allow for other auth providers, such as google (see this tutorial).

This would likely require:

• documenting the required roles and permissions the API needs
• decoupling the role and permission strings into some kind of constants file
• creating a function in each auth class to identify the role of a user since each provider may do it differently
• TBD

See https://brandur.org/soft-deletion for the reasons why.

The main reason i added soft deletion was in case users accidentally deleted something important and missed the timeout window provided by react-admin.

this can be useful for upgrades, migrations, backups .etc to avoid requiring complete downtime since limiting write access will only affect a small portion of users (admins) who likely aren't doing admin tasks that frequently anyway.

• allow for read-only mode to be toggled without restarting the API
• allow the API to proactively communicate whether it is in read only mode so the frontend can display a notice to users so they don't attempt to make any new changes

Similar to #7 and #9, it might be useful to maintain a log file for all changes to data made against the db to at least make manual data recovery possible in the event something is corrupted by a change (edit from a user that wasnt undone in time) rather than a deletion per the related issue regarding soft delete.

it would be nice to automate the deployment process for the main branch just like the main classclock repo.

the best way to do this would probably be best to automatically publish as a docker image to docker hub

This results in the Admin panel not showing any data.

This is likely due to the api trying to hold a single connection to the management API (which is used for checking roles and other access things) instead of using a connection pool or some other way of doing it.

That said this can also be reproduced by omitting the auth0 client ID and or secret environment variables

for added security

when accessing protected data from the admin panel, it fails with the following error

[2022-11-01 15:27:57 +0000] [9] [ERROR] Error handling request /v0/bellschedules
Traceback (most recent call last):
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/gunicorn/workers/sync.py", line 135, in handle
    self.handle_request(listener, req, client, addr)
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/gunicorn/workers/sync.py", line 176, in handle_request
    respiter = self.wsgi(environ, resp.start_response)
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/flask/app.py", line 2091, in __call__
    return self.wsgi_app(environ, start_response)
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/flask/app.py", line 2076, in wsgi_app
    response = self.handle_exception(e)
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/flask/app.py", line 2073, in wsgi_app
    response = self.full_dispatch_request()
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/flask/app.py", line 1518, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/flask/app.py", line 1516, in full_dispatch_request
    rv = self.dispatch_request()
  File "/root/.local/share/virtualenvs/classclock-api-8K7EYNFV/lib/python3.8/site-packages/flask/app.py", line 1502, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
  File "/classclock-api/common/helpers.py", line 433, in wrapper
    return func(*args, **kwargs)
  File "/classclock-api/common/helpers.py", line 378, in decorated
    return func(*args, **kwargs)
  File "/classclock-api/common/helpers.py", line 396, in decorated
    is_admin = check_for_role("admin")
  File "/classclock-api/common/helpers.py", line 292, in check_for_role
    role_names = [r["name"].lower() for r in roles_json]
  File "/classclock-api/common/helpers.py", line 292, in <listcomp>
    role_names = [r["name"].lower() for r in roles_json]
TypeError: string indices must be integers

when performing lots of actions in the backend, its pretty easy to hit the API's existing rate limits

Error seems to have occurred when trying to rename a previously-created schedule (possibly to something that was the same as an existing schedule???)

Schedule name change saved, but the frontend was showing negative numbers.
User tried editing the entire schedule, but it didn’t work (presumably the cause of this issue).

after clicking refresh on classclock (possibly frontend? not sure) it seemed to work after that

needs replication

see sentry issue CLASSCLOCK-API-A

(mysql.connector.errors.DataError) 1292 (22007): Incorrect time value: '' for column `classclock`.`bellschedulemeetingtimes`.`end_time` at row 1

this can be reproduced by creating a schedule without selecting a school in the admin UI.

This should return at LEAST a sane error message on the backend.
The frontend should also include validation to ensure the form cant be submitted without specifying a school.

when sending an authenticated PATCH request to update the bell schedule dates from the admin panel, the server logs show a crash that seems to be caused by:

• a lack of school ID for each individual date in the "dates" block in the body of the PATCH request
• an extra, leftover school_id column in the database that shouldnt be there but is set up as NOT NULL

the specific exception causing this issue was something like this:

[2022-11-20 00:24:01 +0000] [11] [ERROR] Error handling request /v0/bellschedule/<schedule id omitted>
...a pretty large stack trace consisting mostly of SQLAlchemy internals omitted because it is useless...
(mysql.connector.errors.DatabaseError) 1364 (HY000): Field 'school_id' doesn't have a default value
[SQL: INSERT INTO bellscheduledates (bell_schedule_id, date, creation_date) VALUES (%(bell_schedule_id)s, %(date)s, %(creation_date)s)]

idea: have the app automatically run a database dump and save it to a specific folder that can be mounted in docker.

May also be fun to allow superadmins to query and download a list of these files via the API.

this may be hard to do from within python though unless SQLAlchemy supports it. There also may not be demand for this to be worth maintaining

it seems this is currently being enforced/caught at the sqlalchemy layer by as a DataError. which is probably a dumb way to do it

pending further evaluation and root-causing as the affected column ((mysql.connector.errors.DataError) 1406 (22001): Data too long for column 'bell_schedule_display_name' at row 1) wasnt being given a string that was too long (only 27 ASCII characters). This will also likely require a schema change

Due to an issue with closing connections from a connection pool with the latest version of mysql-connector-python, the hosting provider seemingly only supporting virtualenv and not pipenv (or at least without more time and effort than its worth at the moment), and my limited time and patience for dealing with issues, I think it would be best to migrate the ClassClock database to SQLite in preparation for possibly finding a new host (since hosting that is not tied to a DB is often cheaper IIRC).

I have attempted the migration and already have the database in sqlite3 format (thanks to https://github.com/dumblob/mysql2sqlite), but it seems as though I have hit some encoding error, presumably due to the weird way that I was storing UUID's in the database as 16-byte BINARY so that it would be faster than retrieving records with VARCHAR primary keys (based on my research at the time it was made). Here is the error for the curious:

[2021-02-10 20:57:33 -0800] [545386] [ERROR] Error handling request /v0/schools/
Traceback (most recent call last):
  File "/home/ace/.local/share/virtualenvs/API-pA0gWSxv/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1276, in _execute_context
    self.dialect.do_execute(
  File "/home/ace/.local/share/virtualenvs/API-pA0gWSxv/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 608, in do_execute
    cursor.execute(statement, parameters)
sqlite3.OperationalError: Could not decode to UTF-8 column 'schools_school_id' with text 'R,!��`���Bt'

that horrible mess of bytes is supposed to represent the bytes-format UUID of 52 2c 21 a0 ce 60 11 e9 b6 83 02 42 ac 1f 08 74 if it helps.

A big transition like this is probably not a super good use of time, so another possible solution to this would be to find and pay for new web hosting that can host the API and database in a way that works better with the dependencies of this API.

While these issues are present on the main branch, I have been making the changes on dev since theres quite a few unreleased changes there and if there's going to be a change this major to the hosting of the API, it might as well also be a new release.

it might be helpful to have the api check the current database version (i.e. the alembic version stamp) when it starts up to make sure that the database schema it is running against is compatible with that version

Currently whether the user is an admin is determined by the string name of their role. This is reasonably secure since these are created in Auth0 by the super admin, so it would be very hard for an attacker to somehow mess with that, however, this should Ideally not be done this way because it is certifiably jank ™️.

a better way may be to have the super admin provide the Role ID values from auth0 as an environment variable to be used for comparisons. i.e. there could be env vars such as CLASSCLOCK_SUPERADMIN_ROLE_ID and CLASSCLOCK_SCHOOL_ADMIN_ROLE_ID. This would allow for future UI's to be created for the superadmin to be able to assign admins to schools on their instance and may be required for an open source release