-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dropping a user as student from a DB removes the user from student role in all DBs on the server #277
Comments
If I understand the situation correctly, in the example/scenario provided, the student should still be in the The main issue is that the I believe related behavior has been informally discussed in-person before, but not in the context of being an issue. |
I believe you are right about what is happening, especially after doing a lot of tests surrounding the issue. Maybe we could add a function to remove a student from just a single database? I think it would be relatively simple, the only problem I see with that is testing that functionality in the test scripts, since you would have to be testing in multiple databases. |
Ideally function An easy solution is to add an optional parameter such as Testing with multiple DBs should not be an issue because a psql script can change connection using the EDIT: Finishing the apparently unfinished sentence from many moon ago. Apologies for the delay in noticing it. Long term however, this problem can be addressed by using db-specific names for ClassDB roles instead of the fixed names The approach I suggest has some interesting consequences and implementation alternatives which need to be carefully evaluated, but I feel confident it can work. |
I think the solution that @smurthys provides will work well for our needs. While the described behavior could be implemented with a separate function, there is no need to since all we need to do is guard one action in the current drop scripts, which is easiest done with a parameter. I agree that testing with multiple DBs is possible, with the test script creating, connecting to, and dropping a test database (only after asserting that the database did not previously exist). As an aside, it seems like @smurthys' comment has an unfinished idea at the end. It would be interesting to hear about long-term solutions and considerations about this Issue. I can see how a future change to the way ClassDB manages roles could change the necessity of such a parameter. |
After some testing I have noticed a problem related to this issue. It most likely will be solved along with the above issue but I wanted to make the problem known. If there is a user in two ClassDB databases and the user is dropped from one database but not the other the above problem is encountered. However, another related problem also occurs if the user is dropped from database 1 and their objects remain owned by them. As it would happen if you called the drop function like: This would make the objects remain in that database 1, as expected. However, if an admin/teacher in database 2 wants to drop the same student from the server using
This error is caused by the objects that were left in database 1 still owned by This can be easily fixed by a superuser by going into every database that still has In any case, we need to keep this problem in mind while trying to solve this issue. |
Thanks for the comprehensive report @KevinKelly25. I do not believe that it will be solved alongside the above issue, since they are relatively distinct as one involves permissions while the other involves user management. In Postgres, these end up using the same DBMS feature, (roles) but the two uses serve different purposes. Overall, the scenario described could be considered intended behavior. I do not think we should blindly drop data that is contained in other databases, since, in general, we do not want to drop data that was not created through the current instance of ClassDB. This is important in situations where an instance of ClassDB is running on a server cluster shared with other applications (or other instances of ClassDB that are operated by different instructors). If we were to implement the fix provided, then in such situations, an instructor or DB manager may accidentally (or even maliciously) drop data in other databases that they were not supposed to drop. In the description provided, it is correctly said
This is true, and ClassDB should not provide a way to circumvent those restrictions. I agree that this behavior is important to keep in mind when managing related ClassDB functionality. Thanks again @KevinKelly25 for bringing this up. Although tangential from the original issue, I wish to provide some possible solutions to this secondary issue: One way to mitigate problems caused by this is by updating the documentation on the Removing Users page to better explain the consequences of each object disposition option. We can also give a better error message than the one provided by default. Another (somewhat complex) solution is to add a "smart drop" option for
|
If you drop a student from one ClassDB database it drops that student from all ClassDB databases. It makes sense why it does that looking at
dropStudent
function but it seems inconsistent withcreateStudent
. For example:If I have two databases cs305 and cs205. If I add a student to cs205 that student will not show up in cs305. However, if I add a student to both cs205 and cs305 then drop the student from either of the classes it will remove the student from both classes. At the very least not letting them connect to either and also removing them from student activity table of both.
The text was updated successfully, but these errors were encountered: