Role based Access - New Teacher/Student Login & Reg - STILL needs fixing/testing

[angadk4]

Created a new system for RBAC. Essentially, individuals can now use a toggle at the top of the registration page to log in as either a teacher or student, which assigns the role of either "User" or "Teacher." Upon login, the system now uses an email query to verify whether the user is a teacher or student, and proceeds accordingly.

Changes Made:

• Added radio switch at the top of register.html page: can be on either the student or teacher position, but not both at once.
• Added relevant css styling for the role-switch in register.css.
• Updated relevant register.js code to add event listeners for the new role switch.
• Updated relevant register.js code to be able to retain position of switch upon failed attempt/redirect.
• Updated register.js code so that when in 'teacher' position, the grade field disappears and input for it is no longer needed for form submission.
• Updated register route in auth.py so the system now requests the role attribute, and based on "teacher" or "student" assigns a new User or Teacher model.
• Updated login route in auth.py so the system now uses the inputted email to determine whether the user is a teacher or student, and redirects accordingly.
• Added temporary teacher redirect page (we will decide what teachers should be able to do later).

Notes/Issues:

• IMPORTANT: Under models.py, the Teacher class had yrs_specialization set to Nullable=False. This was causing issues as the registration for teachers does not ask for this field, so it should be nullable in reality. I am not sure of how to migrate the database, so I would appreciate if someone could migrate the database with my new changes or show me how.
• IMPORTANT: For some reason, line 68 in auth.py (role = request.form.get("role")) is not functioning as intended. I am completely unsure why this issue exists, but it needs to be fixed for the RBAC system to work and be tested.

[alexchen2]

Overall, there are two general things I wanted to ask you regarding the implementation of your RBAC model here: the mutual exclusivity of consumers being either Users or Teachers, and the design similarities of the Users & Teachers database tables.

For the former, how are you setting up the distinction between consumers within these roles? Can a consumer be both a teacher and a student, or is it an "either-or", one-or-the-other situation? Looking at your auth.py login code at lines 26 to 31, it appears as if you're going for the latter option. You attempt to query one entry in both User and Teacher with the given login email and expect one of them to be None:

# Check both User and Teacher tables
user = User.query.filter_by(email=email).first()
teacher = Teacher.query.filter_by(email=email).first()

# Determine if the email belongs to a Teacher or a User
account = teacher if teacher else user

However, if we take a look at lines 64 to 84:

role = request.form.get("role")
...
if role == "teacher":
    user_email = Teacher.query.filter_by(email=email).first()
    user_name = Teacher.query.filter_by(username=username).first()
else:
    user_email = User.query.filter_by(email=email).first()
    user_name = User.query.filter_by(username=username).first()
...
if user_name:
    flash("This username already exists.", "register_error")
    return redirect(url_for("auth.register"))
...
if user_email:
    flash("This email already exists.", "register_error")
    return redirect(url_for("auth.register"))   

This code shows that when a consumer signs up under some arbitrary username A and email B, the auth system checks for duplicate usernames/emails among only other teachers when signing up as a teacher, or only other students when signing up as a student. Thus, an actor/two separate actors could accidentally sign up with the same exact email A and password B under two accounts—one student, and one teacher. Problem is, given the way the if statement at line 31 is structured, the auth system will always choose only one account role over the other, and it always has a bias towards choosing teachers. As such, when logging in with email A and password B, the system will never log into the student's account and always access the teacher's.

If you're going for a "can be both" approach and want the same email to have a teacher and student role, then we should probably add a similar radio role select button/use separate pages for login. Otherwise, you should probably check for duplicate emails/usernames within both the User and Teacher tables in lines 64 to 84, no matter the role.

[alexchen2]

For the second topic, this initially came to mind due to bugs with logging in as a Teacher; Flask-Login requires any entity inheriting UserMixin to specifically have an id attribute as its primary key (not tid), and Teacher was also missing an extra failed_signin_attempts field from User that was needed in auth.py. These fixes are pretty simple to fix by altering the Teacher table, but the User and Teacher tables have so many similarities between each other at this point that it feels inefficient and a waste of space to have them as separate tables. Wouldn't it just be better to have a main User table with shared attributes (name, hashed_password, email, etc.), then create ISA specializations for each role that inherit from the parent User table (Student having grade, points, etc; Teacher having yrs_experience, specialization, etc.)? This way, not only is our code more concise, but we wouldn't have to constantly create and manage duplicate columns between both tables taking up valuable storage.

I do understand, however, that changing this DB structure this late into the project would most likely take a bit more time in fixing any resulting bugs with queries dependent on the old User model (with Student-specific attributes), so I understand if you aren't too keen on the idea.

[angadk4]

I have implemented all of the relevant changes recommended by Alex:

• Changes to register.html, register.css, and register.js have been made exactly as recommended, with a few minor modifications.
• Changes to models.py have been implemented so that teacher.tid and all of its occurrrences are replaced by teacher.id to better fit the Flask model.
• In terms of auth.py, I have chosen to go with the approach of only allowing a user to either be a teacher or student. I modified registration logic to check for email and username in both Teacher and User tables to ensure no individuals have registered with these credentials in the past. I also modified the login logic to ensure there is no bias when checking for account type based on email queries.

Here are some other important things to note:

• I definitely agree that creating an ISA specialization to simplify these roles would simply the logic, however I have been told that RBAC is not yet a major feature of our application, and thus it's probably best to continue as is for now.
• IMPORTANT: If it is deemed that these new changes have been correctly implemented and this PR is ready for merge, please ensure to migrate the database after doing so. At the moment, changing the teacher tid to id is causing some issues with the login/registration logic. I am not well aware of the alembic system being used for migrations, so this will likely be better handled by someone else on the team.

[angadk4]

Added changes to properly account for an error when email associated with both teacher & user.