DASSL / ClassDB

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

Users can drop their own schema (WM) #175

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

Users can drop their own schema.

This problem is due to users now owning their own schema (which is better than in v1.0), but ownership also permits users to drop their own schema which means students can inadvertently drop their own schema.

smurthys commented 6 years ago

This is a known requirement and the implementation was intentionally delayed due to time constraint.

smurthys commented 6 years ago

Addressing this issue requires the use of event triggers. The solution involves the following steps:

My research so far shows the following impediments:

Alternatives:

Modified version of the example "snitch" trigger function showing TG_TABLE_SCHEMA does not work:

CREATE OR REPLACE FUNCTION snitch() RETURNS event_trigger AS $$
BEGIN
    IF LOWER(TG_TAG) = 'drop schema' THEN
       --error: column "tg_table_schema" does not exist
       RAISE NOTICE 'snitch: % % %', TG_EVENT, TG_TAG, TG_TABLE_SCHEMA; 
    END IF;
END
$$ LANGUAGE plpgsql;
wildtayne commented 6 years ago

Just wanted to quickly comment on this because these event trigger issues are similar to ones I had when writing the DDL activity triggers. The Postgres documentation on event triggers is pretty fragmented. For example, the page linked in @smurthys's mostly concerns DML triggers - TG_TABLE_NAME and TG_TABLE_SCHEMA are only available in DML triggers.

You can get similar information from sql_drop and ddl_command_end using the two functions described here (I forget how I even found this page in the first place). This is what the DDL activity triggers use. I also whipped up the following example of using them with DROP SCHEMA. This example is intended for sql_drop triggers.

CREATE OR REPLACE FUNCTION ClassDB.expDDLTrigger()
RETURNS event_trigger AS
$$
BEGIN
   IF TG_TAG = 'DROP SCHEMA' THEN
      RAISE NOTICE 'Schema dropped by %', SESSION_USER;
      RAISE NOTICE 'Schema % dropped!!', (SELECT object_name
                                          FROM pg_event_trigger_dropped_objects());
   END IF;
END;
$$ LANGUAGE plpgsql;

Output:

classdbv200=# CREATE SCHEMA aSchema;
CREATE SCHEMA
classdbv200=# DROP SCHEMA aSchema;
NOTICE:  Schema dropped by postgres
NOTICE:  Schema aschema dropped!!
DROP SCHEMA
smurthys commented 6 years ago

Unfortunately, the problem is that a catalog look up is not viable in both sql_drop and ddl_command_end. We need to look up the catalog to find schema owner. EDIT: link in my first comment

afig commented 6 years ago

Although it does not provide the same ownership guarantee, wouldn't it be possible to query the RoleBase table to see if the schema being dropped matches SchemaName for the row corresponding to the Student? After all, ClassDB only needs to ensure that students do not drop their assigned schema. At least in this situation, we do not need to manage other schemas that they may have ownership of.

smurthys commented 6 years ago

I worry the stored schema name is unreliable, but perhaps we could use that and prevent drop if the student still owns that schema. Sadly, that does not address DROP OWNED, because we need to know the role name whose objects will be dropped.

Also, we need to think about which user is involved: current_user or session_user?

smurthys commented 6 years ago

It seems both current_user and session_user should be tested: current_user for obvious reasons. Also session_user because there is no circumstance under which a student should be able to drop their own schema.