DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
8 stars 2 forks source link

Default privileges for Instructors are not removed if the user is also a member of other roles (W) #117

Closed afig closed 7 years ago

afig commented 7 years ago

If an Instructor that is currently being dropped through dropInstructor is also a member of another role, then the default privileges on the public schema are not removed, due to the statement's location within the procedure's control structure. This creates an issue when attempting to drop the user at a later point, since they still own those default privileges.

The fix is to move the statement that revokes those default privileges, Currently it is located before the DROP SCHEMA statement, however, it should be located after the DELETE FROM statement in the block one level above. Currently, the default privileges are only being revoked if ClassDB_Instructor is the user's only role.

To replicate:

sql0708=# SELECT classdb.createInstructor('multitest', 'noname');
 createinstructor
------------------

(1 row)

sql0708=# SELECT classdb.createStudent('multitest', 'noname');
NOTICE:  User "multitest" already exists, password not modified
NOTICE:  Schema "multitest" already exists
NOTICE:  role "classdb" is already a member of role "multitest"
 createstudent
---------------

(1 row)

sql0708=# SELECT classdb.dropInstructor('multitest');
NOTICE:  User "multitest" remains a member of one or more additional roles
 dropinstructor
----------------

(1 row)

sql0708=# SELECT classdb.dropStudent('multitest');
ERROR:  role "multitest" cannot be dropped because some objects depend on it
DETAIL:  owner of default privileges on new relations belonging to role multitest in schema public
CONTEXT:  SQL statement "DROP ROLE multitest"
PL/pgSQL function dropstudent(character varying) line 16 at EXECUTE
sql0708=#

Sorry for having to add another issue to M1, but I noticed this while updating the tests in Pull Request #115

afig commented 7 years ago

Example of a potential fix: https://gist.github.com/afig/2b6b6b79bc9861c79312d25f82a40291