Fixed a couple of bugs, added a simple test script, cleaner error messages

and compile-time support for unauthorized row updates behaviour (abort, skip).

git-svn-id: http://svn.osgeo.org/postgis/trunk@2390 b70326c6-7e19-0410-871a-916f4a2858ee

diff --git a/extras/WFS_locks/Makefile b/extras/WFS_locks/Makefile
index 882a14979ac9037a0c7076342a0396ff44993ef6..80f36ccb08c1d438f5f3df34af75ce30de6e8d2c 100644 (file)
--- a/extras/WFS_locks/Makefile
+++ b/extras/WFS_locks/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -I$(shell pg_config --includedir-server)
+CFLAGS += -W -Wall -I$(shell pg_config --includedir-server)
 CC = gcc
 MODULE_FILENAME = $(PWD)/libWFS_locks.so
 

diff --git a/extras/WFS_locks/WFS_locks.c b/extras/WFS_locks/WFS_locks.c
index 69c85b9e07f7e3e245d9a4ea74f3c81a4a74098e..32343f9d47bcbf87df2e05e010bbfe3a30bbaeed 100644 (file)
--- a/extras/WFS_locks/WFS_locks.c
+++ b/extras/WFS_locks/WFS_locks.c
@@ -3,7 +3,9 @@
 #include "commands/trigger.h"   /* ... and triggers */
 #include "utils/lsyscache.h"   /* for get_namespace_name() */
 
-//#define PGIS_DEBUG 1
+#define PGIS_DEBUG 1
+
+#define ABORT_ON_AUTH_FAILURE 1
 
 Datum check_authorization(PG_FUNCTION_ARGS);
 
@@ -20,7 +22,8 @@ Datum check_authorization(PG_FUNCTION_ARGS)
 {
        TriggerData *trigdata = (TriggerData *) fcinfo->context;
        char *colname;
-       HeapTuple rettuple;
+       HeapTuple rettuple_ok;
+       HeapTuple rettuple_fail;
        TupleDesc tupdesc;
        int SPIcode;
        char query[1024];
@@ -29,17 +32,40 @@ Datum check_authorization(PG_FUNCTION_ARGS)
        HeapTuple tuple;
        char *lockcode;
        char *authtable = "authorization_table";
+       const char *op;
+#define ERRMSGLEN 256
+       char errmsg[ERRMSGLEN];
 
 
        /* Make sure trigdata is pointing at what I expect */
-       if (!CALLED_AS_TRIGGER(fcinfo))
+       if ( ! CALLED_AS_TRIGGER(fcinfo) )
+       {
                elog(ERROR,"check_authorization: not fired by trigger manager");
+       }
+
+       if ( ! TRIGGER_FIRED_BEFORE(trigdata->tg_event) )
+       {
+               elog(ERROR,"check_authorization: not fired *before* event");
+       }
 
-       if (! (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event) ||
-               TRIGGER_FIRED_BY_DELETE(trigdata->tg_event)) )
+       if ( TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event) )
+       {
+               rettuple_ok = trigdata->tg_newtuple;
+               rettuple_fail = NULL;
+               op = "UPDATE";
+       }
+       else if ( TRIGGER_FIRED_BY_DELETE(trigdata->tg_event) )
+       {
+               rettuple_ok = trigdata->tg_trigtuple;
+               rettuple_fail = NULL;
+               op = "DELETE";
+       }
+       else
+       {
                elog(ERROR,"check_authorization: not fired by update or delete");
+       }
+
 
-       rettuple = trigdata->tg_newtuple;
        tupdesc = trigdata->tg_relation->rd_att;
 
        /* Connect to SPI manager */
@@ -56,8 +82,7 @@ Datum check_authorization(PG_FUNCTION_ARGS)
                SPI_fnumber(tupdesc, colname));
 
 #if PGIS_DEBUG
-       elog(NOTICE,"check_authorization(%s.%s.%s=%s)",
-               nspname, relname, colname, pk_id);
+       elog(NOTICE,"check_authorization called");
 #endif
 
        sprintf(query,"SELECT authid FROM \"%s\" WHERE expires >= now() AND toid = '%d' AND rid = '%s'", authtable, trigdata->tg_relation->rd_id, pk_id);
@@ -73,21 +98,21 @@ Datum check_authorization(PG_FUNCTION_ARGS)
        if (!SPI_processed )
        {
 #if PGIS_DEBUG
-               elog(NOTICE,"there is NOT a lock on row '%s'", pk_id);
+               elog(NOTICE,"there is NO lock on row '%s'", pk_id);
 #endif
                SPI_finish();
-               return PointerGetDatum(trigdata->tg_trigtuple);
+               return PointerGetDatum(rettuple_ok);
        }
 
        // there is a lock - check to see if I have rights to it!
 
        tuptable = SPI_tuptable;
-       tupdesc = SPI_tuptable->tupdesc;
+       tupdesc = tuptable->tupdesc;
        tuple = tuptable->vals[0];
        lockcode = SPI_getvalue(tuple, tupdesc, 1);
 
 #if PGIS_DEBUG
-       elog(NOTICE,"there is a lock on this row!");
+       elog(NOTICE, "there is a lock on row '%s' (auth: '%s').", pk_id, lockcode);
 #endif
 
        // check to see if temp_lock_have_table table exists
@@ -98,14 +123,7 @@ Datum check_authorization(PG_FUNCTION_ARGS)
                elog(ERROR,"couldnt execute to test for lockkey temp table :%s",query);
        if (SPI_processed==0)
        {
-               SPI_finish();
-
-               elog(ERROR,"Unauthorized modification (requires auth: '%s')",
-                       lockcode);
-               elog(NOTICE,"Modificaton of row '%s' requires authorization '%s'",
-                       pk_id, lockcode);
-               // ignore requested delete or update
-               return PointerGetDatum(trigdata->tg_trigtuple);
+               goto fail;
        }
 
        sprintf(query, "SELECT * FROM temp_lock_have_table WHERE lockcode ='%s'",lockcode);
@@ -116,7 +134,7 @@ Datum check_authorization(PG_FUNCTION_ARGS)
 
        SPIcode = SPI_exec(query,0);
        if (SPIcode != SPI_OK_SELECT )
-               elog(ERROR,"couldnt execute to test for lock aquire: %s", query);
+               elog(ERROR, "couldnt execute to test for lock aquire: %s", query);
 
        if (SPI_processed >0)
        {
@@ -124,16 +142,23 @@ Datum check_authorization(PG_FUNCTION_ARGS)
                elog(NOTICE,"I own the lock - I can modify the row");
 #endif
                SPI_finish();
-               return PointerGetDatum(rettuple);
+               return PointerGetDatum(rettuple_ok);
        }
 
+       fail:
+
+       snprintf(errmsg, ERRMSGLEN, "%s where \"%s\" = '%s' requires authorization '%s'",
+               op, colname, pk_id, lockcode);
+       errmsg[ERRMSGLEN-1] = '\0';
+
+#ifdef ABORT_ON_AUTH_FAILURE
+       elog(ERROR, "%s", errmsg);
+#else
+       elog(NOTICE, "%s", errmsg);
+#endif
+
        SPI_finish();
+       return PointerGetDatum(rettuple_fail);
 
-       elog(ERROR,"Unauthorized modification (requires auth: '%s')",
-               lockcode);
-       elog(NOTICE,"Modificaton of row '%s' requires authorization '%s'",
-               pk_id, lockcode);
-       // ignore the command
-       return PointerGetDatum(trigdata->tg_trigtuple);
 
 }

diff --git a/extras/WFS_locks/WFS_locks.sql.in b/extras/WFS_locks/WFS_locks.sql.in
index dc8a913409414bc9f943a8317669f7659cec9021..6fb966fccbb61749df02d94c3969037e3dd736f7 100644 (file)
--- a/extras/WFS_locks/WFS_locks.sql.in
+++ b/extras/WFS_locks/WFS_locks.sql.in
@@ -31,6 +31,10 @@
 # define _STRICT with(isstrict)
 #endif 
 
+#if USE_VERSION >= 73
+# define HAS_SCHEMAS 1
+#endif
+
 
 -----------------------------------------------------------------------
 -- LONG TERM LOCKING
@@ -71,12 +75,18 @@ DECLARE
        myrec RECORD;
        
 BEGIN
+
        EXECUTE ''DELETE FROM authorization_table WHERE expires < now()''; 
 
+#if HAS_SCHEMAS
        SELECT c.oid INTO mytoid FROM pg_class c, pg_namespace n
                WHERE c.relname = mytable
                AND c.relnamespace = n.oid
                AND n.nspname = myschema;
+#else
+       SELECT c.oid INTO mytoid FROM pg_class c
+               WHERE c.relname = mytable;
+#endif
 
        -- RAISE NOTICE ''toid: %'', mytoid;
 
@@ -164,8 +174,10 @@ LANGUAGE PLPGSQL;
 CREATEFUNCTION CheckAuth(text, text, text)
        RETURNS INT
        AS '
+#if USE_VERSION >= 73
 DECLARE
        schema text;
+#endif
 BEGIN
 
 #if USE_VERSION >= 73
@@ -201,6 +213,12 @@ CREATEFUNCTION CheckAuthTrigger()
        '@MODULE_FILENAME@', 'check_authorization'
        LANGUAGE C;
 
+CREATE TABLE authorization_table (
+       toid oid, -- table oid
+       rid text, -- row id
+       expires timestamp,
+       authid text
+);
 
 ---------------------------------------------------------------
 -- END

diff --git a/extras/WFS_locks/test.sql b/extras/WFS_locks/test.sql
new file mode 100644 (file)
index 0000000..ec5ce05
--- /dev/null
+++ b/extras/WFS_locks/test.sql
@@ -0,0 +1,61 @@
+--BEGIN;
+
+CREATE TABLE test_locks (id serial, b char(16), c char(16) );
+INSERT INTO test_locks(b) VALUES ('one');
+INSERT INTO test_locks(b) VALUES ('two');
+INSERT INTO test_locks(b) VALUES ('three');
+
+-- Enable locks checking on the table
+SELECT CheckAuth('test_locks', 'id');
+
+-- this has no lock
+UPDATE test_locks SET c = 'nolocks';
+
+-- place the lock
+SELECT LockRow('test_locks', '1', 'auth1', now()::timestamp+'00:01');
+SELECT LockRow('test_locks', '2', 'auth2', now()::timestamp+'00:01');
+
+-- this should fail due to missing auth
+UPDATE test_locks SET c = 'unauthorized' where id = 1;
+
+BEGIN;
+
+       -- Add authorization for row 1
+       SELECT AddAuth('auth1');
+
+       -- we're authorized for row 1
+       UPDATE test_locks SET c = 'authorized' where id = 1;
+
+END;
+
+-- Out of transaction we're no more authorized for row 1
+UPDATE test_locks SET c = 'unauthorized' where id = 1;
+
+BEGIN;
+
+       -- Add authorization for row 2
+       SELECT AddAuth('auth2');
+
+       -- we're authorized for row 2
+       UPDATE test_locks SET c = 'authorized' where id = 2;
+
+END;
+
+
+BEGIN;
+
+       -- Add authorization for row 2
+       SELECT AddAuth('auth2');
+
+       -- we're *not* authorized for row 1
+       UPDATE test_locks SET c = 'unauthorized' where id = 1;
+
+END;
+
+UPDATE test_locks SET c = 'unauthorized' where id = 2;
+UPDATE test_locks SET c = 'unauthorized' where id = 1;
+
+SELECT * from test_locks;
+DROP TABLE test_locks;
+
+END;