gdal icon indicating copy to clipboard operation
gdal copied to clipboard

3.9.0 PostgreSQL ogr_system_tables.metadata is a breaking change when multiple roles and different privileges are present

Open Meibes opened this issue 1 year ago • 1 comments

What is the bug?

when i import a shapefile into postgresql with admin privileges, it automatically creates ogr_system_tables.metadata. when another user who has restricted privileges for a specific schema tries to import a shapefile into the restricted schema, it fails because gdal is trying to access ogr_system_tables.metadata, which the user does not have access to.

It would be good to check if ogr_system_tables.metadata exists and then also check if the user has access or not and display a warning instead of throwing an error and aborting the whole process.

My workaround is to add -nomd to all my scripts where a privileged user would import files into the db so that ogr_system_tables.metadata is never created in the first place, but I'm not sure if this is enough.

(with corrections from ai)

Steps to reproduce the issue

create and setup database and user

--CREATE DATABASE test_db;
CREATE EXTENSION IF NOT EXISTS postgis;
CREATE SCHEMA IF NOT EXISTS test_schema;

CREATE ROLE test_role WITH LOGIN ENCRYPTED PASSWORD 'test_password';
GRANT CONNECT ON DATABASE test_db to test_role;
GRANT ALL PRIVILEGES ON SCHEMA test_schema, public TO test_role;
GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA test_schema, public TO test_role;
GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA test_schema, public TO test_role;

test_shape.zip (only has a single point)

import creates ogr_system_tables (insert your own password)

ogr2ogr -f PostgreSQL PG:"host=localhost port=5432 dbname=test_db user=postgres password=my_super_secret_password" test_shape.shp

import failes because ogr_system_tables exists but test_role has no access

ogr2ogr -f PostgreSQL PG:"host=localhost port=5432 active_schema=test_schema dbname=test_db user=test_role password=test_password" test_shape.shp

cleanup remove test_role

REASSIGN OWNED BY test_role TO postgres;
DROP OWNED BY test_role;
DROP USER test_role;

Versions and provenance

Windows 10 PostgreSQL 16.3 PostGIS 3.4.1 GDAL 3.9.0, released 2024/05/07 installed via OSGeo4W in qgis-ltr

Additional context

No response

Meibes avatar May 24 '24 08:05 Meibes

My workaround is to add -nomd to all my scripts where a privileged user would import files into the db so that ogr_system_tables.metadata is never created in the first place, but I'm not sure if this is enough.

that should be fine. You could also specify --config OGR_PG_ENABLE_METADATA NO

rouault avatar May 24 '24 14:05 rouault

There is a related problem in gis.stackexchange https://gis.stackexchange.com/questions/478078/import-of-geojson-multi-polygon-into-postgresql-does-not-work-with-gdal-ogr2ogr.

Does this change perhaps resolve also that? The issue seems to arise from this CREATE FUNCTION... https://github.com/OSGeo/gdal/blob/fe3cf27139f56067b31b2e200ffd4cfa8407760b/ogr/ogrsf_frmts/pg/ogrpgtablelayer.cpp#L359 that requires superuser rights.

jratike80 avatar May 28 '24 11:05 jratike80

Does this change perhaps resolve also that?

I believe the test https://github.com/OSGeo/gdal/pull/10004/files#diff-7dbd738af5dc94c81fdbfda0c672f9f3f7a63382b22771fa0a679b8917513ef8R3185 added in PR #10004 should also resolve that case

rouault avatar May 28 '24 11:05 rouault

Today we experienced a problem in Postgres that might be related: after a Shapefile had been imported in PostGIS using ogr2ogr with superuser credentials, a non-superuser couldn't DROP a completely unrelated table, with an error stating that they didn't have permissions to access the ogr_system_tables schema:

ERROR:  permission denied for schema ogr_system_tables 
LINE 1: DELETE FROM ogr_system_tables.metadata m WHERE m.schema_name... 
                    ^ 
QUERY:  DELETE FROM ogr_system_tables.metadata m WHERE m.schema_name = obj.schema_name AND m.table_name = obj.object_name 
CONTEXT:  PL/pgSQL function ogr_system_tables.event_trigger_function_for_metadata() line 8 at SQL statement  

The ogr_system_tables schema was owned by the same superuser that ran ogr2ogr. We solved the issues by changing owner and permission for ogr_system and ogr_system_tables.metadata.

Here's the postgis_full_version() output:

POSTGIS="3.4.2 c19ce56" [EXTENSION] PGSQL="160" GEOS="3.11.1-CAPI-1.17.1" (compiled against GEOS 3.10.2) SFCGAL="SFCGAL 1.4.1, CGAL 5.3.1, BOOST 1.74.0" PROJ="8.2.1 NETWORK_ENABLED=OFF URL_ENDPOINT=https://cdn.proj.org USER_WRITABLE_DIRECTORY=/tmp/proj DATABASE_PATH=/usr/share/proj/proj.db" GDAL="GDAL 3.4.3, released 2022/04/22" LIBXML="2.9.13" LIBJSON="0.15" LIBPROTOBUF="1.3.3" WAGYU="0.5.0 (Internal)" TOPOLOGY RASTER

albertodeluca avatar May 29 '24 09:05 albertodeluca

@albertodeluca, I also encountered this semi-related issue after having used ogr2ogr 3.9.0 last week to load a shapefile into PostgreSQL as a superuser. Subsequently, other users on the database couldn't perform DROPs (even on TEMPORARY TABLEs they owned).

Another possible solution to fix this issue if you're not interested in this extra metadata tracking after loading data is to drop the EVENT TRIGGER which is listening for every DROP across the entire database (which is why DROPs are suddenly problematic):

DROP EVENT TRIGGER ogr_system_tables_event_trigger_for_metadata;

Or if you want to remove this metadata table and function completely after it's already been created, then you could also drop the entire schema. I'm admittedly not super familiar with the driver behind this new metadata schema and tracking (from https://github.com/OSGeo/gdal/pull/8965), but based on my limited understanding, I don't believe it's essential (since it wouldn't have been created to start with if the -nomd option were specified during load). So if you want to drop it entirely (but obviously, be careful with drops):

DROP SCHEMA ogr_system_tables CASCADE;

For anyone extra curious and trying to understand what's going on here and how using ogr2ogr is impacting unrelated unrelated DROPs, here's what I think is happening if it helps (but I'm not super familiar with all of this, so someone else feel free to correct me or take all of this with a grain of salt):

  • ogr2ogr as of v3.9.0 will create this ogr_system_tables schema and metadata table by default in PostgreSQL (added in https://github.com/OSGeo/gdal/pull/8965).
  • Part of this metadata setup is a EVENT TRIGGER and function that will run on every DROP executed on the database that will try to cleanup the dropped table from the ogr metadata table: https://github.com/OSGeo/gdal/blob/bb5ea618e23e498e79e8b8e6bd7215ad94bb2103/ogr/ogrsf_frmts/pg/ogrpgdatasource.cpp#L3226-L3257
  • Since this EVENT TRIGGER will execute for all tables, this is what can lead to permission problems, since users may be authorized to perform DROPs, but they may not have the necessary permissions to execute the ogr cleanup function.
  • I haven't tested it, but it does look like the changes in https://github.com/OSGeo/gdal/pull/10004 will allow the trigger function to still execute even if the user doesn't have the necessary permissions.
  • If you don't necessarily need or want this extra metadata tracking, then you can specify the -nomd flag on ogr2ogr to skip all of this.
    • @rouault, I know you also mentioned setting --config OGR_PG_ENABLE_METADATA NO and I see where support for that was added in https://github.com/OSGeo/gdal/pull/9865, but I can't get this setting to work. Am I just passing it in wrong? I've tried passing it in a variety of ways to ogr2ogr, but each time I still get the metadata triggers and schemas created. Here's an example I've tried (with the ghcr.io/osgeo/gdal:ubuntu-small-3.9.0 docker image):

      ogr2ogr \
        -f PostgreSQL "PG:host=example.com dbname=example user=postgres" \
        -nln "public.example_table" \
        -nlt MULTILINESTRING \
        -lco PRECISION=NO \
        --config OGR_PG_ENABLE_METADATA=NO \
        example.shp
      

GUI avatar May 31 '24 18:05 GUI

  • I know you also mentioned setting --config OGR_PG_ENABLE_METADATA NO and I see where support for that was added in ... but I can't get this setting to work.

doh, there was a stupid logic error, and there was no regression test for it, thus it was broken. Added an extra fix in #10004 . I'd appreciate if you could give it a try regarding permissions issues.

rouault avatar May 31 '24 18:05 rouault