BUG whoami returning "admin_for" and "ta_for" which do not match for superusers about anubis

[AlpacaMax]

I see. Looks like a pretty straight forward task. I'll take this one and work on it this weekend.

[AlpacaMax]

Are these two suppose to always match for superusers? I just checked the User model. If you're not the TA for OOP then sure you won't be listed while as a superuser you are an admin for every course.

[wabscale]

There is just a bit of a disconnect in logic with that section of the User model.

The problem starts with the fact that until like a month ago, Anubis was only designed to really handle a single Course. We could add extra courses, and have different assignments for them, but the admin permissions were global. Basically if you were a TA for one course, you were a TA for all. Obviously this was not a good permission model especially since we are trying to expand this to other courses. There is now course specific TA and Professor permissions.

When I implemented "course awareness" to Anubis, it was a really really large rewrite. Adding the course awareness was when I started to call Anubis an LMS since it finally had all the necessary permission models. I basically needed to augment "course aware" logic to the entire admin API. Every single call that lists or modifies resources needed extra checks to make sure that the user making the request was an admin for the course in question, and that their permission level was sufficient. There are a couple of things that a course Professor can do that a TA cannot (like change the course title).

BTW, the main function that is used for checking if a admin is allowed to modify a resource is anubis.utils.lms.courses.assert_course_context.

The TL;DR here is that we basically just need to simplify the logic a bit in the User.data function for superusers. Like if you are a superuser the professor_for and ta_for fields should be what is in super_for. Then for courses that you are a professor for, then you should also be a ta for those courses too. I think we should move that logic into a separate function (maybe in anubis/utils/lms/course.py) that will take the user object and return a dict {"admin_for": [...], "ta_for": [...], "professor_for": [...]}. We can then call that from the User.data function and add the results to the response.

Do those changes make sense? I can further explain the admin permission model in Anubis if needed.

[AlpacaMax]

I'm confused. Can you elaborate more on the changes you want to make? Some examples would help.

[wabscale]

So let me explain the permission model in Anubis first. There are four levels of permissions. They are (in order of permissions) student, ta, professor, and superuser.

• A student can only do student things (like viewing their own submissions).
• TAs can do most admin things for their course (like view autograde results, etc...).
• Professors can do everything a TA can plus a couple of extra things (like change the course title).
• Superusers can do everything a professor can + a few things (like creating a new course).

The TA and professor permissions are specific to courses. So you are only a TA or a professor for the courses that you have been set as a ta or professor for. Then with superusers, they do not need to be added to courses. Superuser is a global permission. What that means is that if you are a superuser, its is like you are automatically a professor for all courses.

In the User.data property we need to calculate which courses that user is a professor or ta for. Then in the special case of superusers, you should just be a TA and professor for all courses.

So then we need to calculate which courses go in the "ta_for" and "professor_for" field. If you are a professor for a course, then it should also be in the "ta_for". Basically the "ta_for" should always include the things that are in "professor_for" field. Then the "admin_for" should basically be a union of "ta_for" and "professor_for" (you could calculate that with a simple set operation in python). Lets consider an example:

from anubis.models import User

# Say that Jeff is a ta for Intro to OS (ITOS), and a Professor for Mining Massive Data Sets (MMDS).
# We would expect that the ta_for field would have both courses, and professor_for would have only MMDS.
# Jeff is NOT a superuser.

jeff = User.query.filter(User.name == "mynameisjeff").first()
jeff_data = jeff.data

print(jeff_data['is_superuser'])
# False

print(jeff_data['ta_for'])  
# [ {"id": "...", "name": "ITOS"}, {"id": "...", "name": "MMDS"} ]

print(jeff_data['professor_for'])
# [{"id": "...", "name": "MMDS"} ]

print(jeff_data['professor_for'])
# [ {"id": "...", "name": "ITOS"}, {"id": "...", "name": "MMDS"} ]


# Now say that John (me) is a superuser. I do not need to have TA or Professor permissions for any courses.
# I automatically am just a admin for everything:

john = User.query.filter(User.name == "mynameisjohn").first()
john_data = john.data

print(john_data['is_superuser'])
# True

print(john_data['ta_for'])  
# [ {"id": "...", "name": "ITOS"}, {"id": "...", "name": "MMDS"} ]

print(john_data['professor_for'])
# [ {"id": "...", "name": "ITOS"}, {"id": "...", "name": "MMDS"} ]

print(john_data['professor_for'])
# [ {"id": "...", "name": "ITOS"}, {"id": "...", "name": "MMDS"} ]

Ok so I think I have explained what should go into those fields. The purpose of this card is to move that logic to a separate file, and have it work the way I described above. The file where the models are defined is already over 1K lines. We should keep as much moving pieces out of them as possible.

Also I don't think I have said this anywhere, but these values are used by the website when loading to determine if the admin panel should be added to the side and which admin course contexts you can switch between. Like here in the navbar where it reads what the is_admin field from User.data to see if it should add the nav items for the admin panel. Then here in the top header it uses the admin_for value to figure out which courses to offer as an admin context. The ta_for and professor_for fields are given so the frontend can render components differently depending on if a permission is allowed or not for the set course context.

[AlpacaMax]

John I have a proposal. So I kinda disagree with you on putting this logic to a separate file as I think it is a User specific thing. However I do agree with you that the models/__init__.py file simply has too many lines of code to be easily maintainable. My proposal is that we separate all these models into its own file under models directory. This way we can ensure the maintainability of these models in the future.

[AlpacaMax]

Never mind. Apparently sqlalchemy doesn't have a workaround on cyclic association. I misremembered that with Django.