diff --git a/splitgraph/resources/static/splitgraph_api.sql b/splitgraph/resources/static/splitgraph_api.sql index 4a890612..f1739975 100644 --- a/splitgraph/resources/static/splitgraph_api.sql +++ b/splitgraph/resources/static/splitgraph_api.sql @@ -51,21 +51,38 @@ $$ LANGUAGE plpgsql SECURITY DEFINER; +CREATE OR REPLACE FUNCTION splitgraph_api.check_objects_privilege ( + _object_ids varchar[], + _action varchar DEFAULT 'repository.read', + _raise_error boolean DEFAULT false +) + RETURNS varchar[] + AS $$ +BEGIN + -- A no-op privilege check that can be overridden + RETURN _object_ids; +END; +$$ +LANGUAGE plpgsql +SECURITY DEFINER; + CREATE OR REPLACE FUNCTION splitgraph_api.check_privilege ( - namespace varchar + _namespace varchar, + _repository varchar DEFAULT NULL, + _action varchar DEFAULT 'repository.read' ) RETURNS void AS $$ BEGIN - IF splitgraph_api.bypass_privilege () THEN + IF splitgraph_api.bypass_privilege () OR _action = 'repository.read' THEN RETURN; END IF; -- Here "current_user" is the definer, "session_user" is the caller and we can access another session variable -- to establish identity. -- Use IS DISTINCT FROM rather than != to catch namespace=NULL - IF splitgraph_api.get_current_username () IS DISTINCT FROM namespace THEN + IF splitgraph_api.get_current_username () IS DISTINCT FROM _namespace THEN RAISE insufficient_privilege - USING MESSAGE = 'You do not have access to this namespace!'; + USING MESSAGE = 'You do not have sufficient permissions on this namespace!'; END IF; END; $$ @@ -89,6 +106,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_images ( ) AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN QUERY SELECT i.image_hash, i.parent_id, @@ -102,7 +120,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_repository_size(namespace, repository): get repository size in bytes (counting tables @@ -114,6 +132,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_repository_size ( RETURNS BIGINT AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN (WITH iob AS ( SELECT DISTINCT unnest(object_ids) AS object_id FROM splitgraph_meta.tables t @@ -126,7 +145,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- Search for an image by its hash prefix. Currently the prefix feature -- is only used for better API/cmdline UX -- pushes/pulls to the registry @@ -145,6 +164,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_image ( ) AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN QUERY SELECT i.image_hash, i.parent_id, @@ -159,7 +179,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_tagged_images(namespace, repository): get hashes of all images with a tag. CREATE OR REPLACE FUNCTION splitgraph_api.get_tagged_images ( @@ -172,6 +192,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_tagged_images ( ) AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN QUERY SELECT t.image_hash, t.tag @@ -181,7 +202,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_image_size(namespace, repository, image_hash): get image size in bytes (counting tables -- that share objects only once) @@ -193,6 +214,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_image_size ( RETURNS BIGINT AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN (WITH iob AS ( SELECT DISTINCT image_hash, unnest(object_ids) AS object_id @@ -207,7 +229,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_image_dependencies(namespace, repository, image_hash): get all images -- that this Splitfile-built image depends on. @@ -223,6 +245,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_image_dependencies ( ) AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN QUERY WITH provenance_data AS ( SELECT jsonb_array_elements(i.provenance_data) AS d FROM splitgraph_meta.images i @@ -248,7 +271,7 @@ WHERE d ->> 'source_namespace' IS NOT NULL END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_image_dependents(namespace, repository, image_hash): get all images in this -- repository that were built by a Splitfile and used this image through a FROM command. @@ -266,6 +289,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_image_dependents ( DECLARE FILTER JSONB; BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); FILTER = jsonb_build_array (jsonb_build_object('source_namespace', _namespace, 'source', _repository, 'source_hash', _image_hash)); RETURN QUERY @@ -278,7 +302,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- Consider merging writes to all tables into one big routine (e.g. also include a list of tables here, which -- will get added to the tables table) @@ -295,7 +319,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.add_image ( RETURNS void AS $$ BEGIN - PERFORM splitgraph_api.check_privilege (namespace); + PERFORM splitgraph_api.check_privilege (namespace, repository, 'repository.push_image'); INSERT INTO splitgraph_meta.images (namespace, repository, image_hash, parent_id, created, comment, provenance_data) VALUES (namespace, repository, image_hash, parent_id, created, comment, @@ -318,7 +342,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.delete_image ( RETURNS void AS $$ BEGIN - PERFORM splitgraph_api.check_privilege (_namespace); + PERFORM splitgraph_api.check_privilege (_namespace, _repository, 'repository.delete'); DELETE FROM splitgraph_meta.tags WHERE namespace = _namespace AND repository = _repository @@ -346,7 +370,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.tag_image ( RETURNS void AS $$ BEGIN - PERFORM splitgraph_api.check_privilege (_namespace); + PERFORM splitgraph_api.check_privilege (_namespace, _repository, 'repository.push_image'); INSERT INTO splitgraph_meta.tags (namespace, repository, image_hash, tag) VALUES (_namespace, _repository, _image_hash, _tag) ON CONFLICT (namespace, repository, tag) @@ -355,7 +379,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; CREATE OR REPLACE FUNCTION splitgraph_api.delete_tag ( _namespace varchar, @@ -365,7 +389,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.delete_tag ( RETURNS void AS $$ BEGIN - PERFORM splitgraph_api.check_privilege (_namespace); + PERFORM splitgraph_api.check_privilege (_namespace, _repository, 'repository.push_image'); DELETE FROM splitgraph_meta.tags WHERE namespace = _namespace AND repository = _repository @@ -373,7 +397,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- delete_repository(namespace, repository) CREATE OR REPLACE FUNCTION splitgraph_api.delete_repository ( @@ -383,7 +407,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.delete_repository ( RETURNS void AS $$ BEGIN - PERFORM splitgraph_api.check_privilege (_namespace); + PERFORM splitgraph_api.check_privilege (_namespace, _repository, 'repository.delete'); DELETE FROM splitgraph_meta.tables WHERE namespace = _namespace AND repository = _repository; @@ -417,7 +441,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_object_meta(object_ids): get metadata for objects CREATE OR REPLACE FUNCTION splitgraph_api.get_object_meta ( @@ -436,7 +460,10 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_object_meta ( rows_deleted integer ) AS $$ +DECLARE + _allowed_object_ids varchar[]; BEGIN + SELECT splitgraph_api.check_objects_privilege (object_ids) INTO _allowed_object_ids; RETURN QUERY SELECT o.object_id, o.format, @@ -449,11 +476,11 @@ BEGIN o.rows_inserted, o.rows_deleted FROM splitgraph_meta.objects o - WHERE o.object_id = ANY (object_ids); + WHERE o.object_id = ANY (_allowed_object_ids); END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_object_locations(object_ids): get external locations for objects CREATE OR REPLACE FUNCTION splitgraph_api.get_object_locations ( @@ -465,17 +492,20 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_object_locations ( protocol varchar ) AS $$ +DECLARE + _allowed_object_ids varchar[]; BEGIN + SELECT splitgraph_api.check_objects_privilege (object_ids) INTO _allowed_object_ids; RETURN QUERY SELECT o.object_id, o.location, o.protocol FROM splitgraph_meta.object_locations o - WHERE o.object_id = ANY (object_ids); + WHERE o.object_id = ANY (_allowed_object_ids); END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- add_object(object_id, format, namespace, size, insertion_hash, deletion_hash, index) -- If the object already exists, it gets overwritten (making sure the caller has permissions @@ -497,7 +527,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.add_object ( DECLARE existing record; BEGIN - PERFORM splitgraph_api.check_privilege (_namespace); + PERFORM splitgraph_api.check_privilege (_namespace, _action => 'repository.push_image'); -- Do SELECT FOR UPDATE to lock the row if it actually does exist to avoid two people -- calling this at the same time SELECT * INTO existing @@ -512,7 +542,8 @@ BEGIN _insertion_hash, _deletion_hash, _index, _rows_inserted, _rows_deleted); ELSE - PERFORM splitgraph_api.check_privilege (existing.namespace); + PERFORM splitgraph_api.check_privilege (existing.namespace, _action => 'repository.push_image'); + PERFORM splitgraph_api.check_objects_privilege (ARRAY[_object_id], 'repository.push_image', true); UPDATE splitgraph_meta.objects SET format = _format, @@ -527,7 +558,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- add_object_location(object_id, location, protocol) CREATE OR REPLACE FUNCTION splitgraph_api.add_object_location ( @@ -544,7 +575,8 @@ BEGIN SELECT o.namespace FROM splitgraph_meta.objects o WHERE o.object_id = _object_id); - PERFORM splitgraph_api.check_privilege (namespace); + PERFORM splitgraph_api.check_privilege (namespace, _action => 'repository.push_image'); + PERFORM splitgraph_api.check_objects_privilege (ARRAY[_object_id], 'repository.push_image', true); INSERT INTO splitgraph_meta.object_locations (object_id, LOCATION, protocol) VALUES (_object_id, LOCATION, protocol) ON CONFLICT (object_id) @@ -552,7 +584,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- -- TABLE API @@ -571,6 +603,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_tables ( ) AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN QUERY SELECT t.table_name, t.table_schema, @@ -582,7 +615,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_tables(namespace, repository): list all tables in a given repository together with -- images they belong to. @@ -598,6 +631,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_all_tables ( ) AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN QUERY SELECT t.image_hash, t.table_name, @@ -609,7 +643,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- add_table(namespace, repository, table_name, table_schema, object_ids) -- add a table to an existing image. -- Technically, we shouldn't allow this to be done once the image has been created (so maybe that idea with only having @@ -628,7 +662,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.add_table ( RETURNS void AS $$ BEGIN - PERFORM splitgraph_api.check_privilege (_namespace); + PERFORM splitgraph_api.check_privilege (_namespace, _repository, 'repository.push_image'); INSERT INTO splitgraph_meta.tables (namespace, repository, image_hash, table_name, table_schema, object_ids) VALUES (_namespace, _repository, _image_hash, _table_name, _table_schema, _object_ids) @@ -639,7 +673,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_table_size(namespace, repository, image_hash, table_name): get table size in bytes CREATE OR REPLACE FUNCTION splitgraph_api.get_table_size ( @@ -651,6 +685,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_table_size ( RETURNS BIGINT AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN (WITH iob AS ( SELECT unnest(object_ids) AS object_id FROM splitgraph_meta.tables t @@ -665,7 +700,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- get_table_length: use object metadata to get the number of rows in a table. Will return -- NULL if some objects have an unknown number of rows. @@ -678,6 +713,7 @@ CREATE OR REPLACE FUNCTION splitgraph_api.get_table_length ( RETURNS INTEGER AS $$ BEGIN + PERFORM splitgraph_api.check_privilege (_namespace, _repository); RETURN (WITH iob AS ( SELECT unnest(object_ids) AS object_id FROM splitgraph_meta.tables t @@ -692,7 +728,7 @@ BEGIN END $$ LANGUAGE plpgsql -SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp; +SECURITY DEFINER SET search_path = splitgraph_meta, pg_temp, public; -- -- S3 UPLOAD/DOWNLOAD API diff --git a/test/splitgraph/test_security.py b/test/splitgraph/test_security.py index 8190fdd6..8a67a2e9 100644 --- a/test/splitgraph/test_security.py +++ b/test/splitgraph/test_security.py @@ -91,14 +91,14 @@ def test_push_others(local_engine_empty, readonly_pg_repo): with pytest.raises(ProgrammingError) as e: destination.push(remote_repository=readonly_pg_repo, handler="S3") - assert "You do not have access to this namespace!" in str(e.value) + assert "You do not have sufficient permissions on this namespace!" in str(e.value) @pytest.mark.registry def test_delete_others(readonly_pg_repo): with pytest.raises(ProgrammingError) as e: readonly_pg_repo.delete(uncheckout=False) - assert "You do not have access to this namespace!" in str(e.value) + assert "You do not have sufficient permissions on this namespace!" in str(e.value) with pytest.raises(ProgrammingError) as e: readonly_pg_repo.images.delete([readonly_pg_repo.images["latest"].image_hash]) @@ -128,13 +128,13 @@ def test_overwrite_other_object_meta(readonly_pg_repo): with pytest.raises(ProgrammingError) as e: readonly_pg_repo.objects.register_objects([object_meta]) - assert "You do not have access to this namespace!" in str(e.value) + assert "You do not have sufficient permissions on this namespace!" in str(e.value) object_meta = readonly_pg_repo.objects.get_object_meta(fruits.objects)[fruits.objects[0]] assert object_meta.size != 12345 with pytest.raises(ProgrammingError) as e: readonly_pg_repo.objects.register_objects([object_meta], namespace=REMOTE_NAMESPACE) - assert "You do not have access to this namespace!" in str(e.value) + assert "You do not have sufficient permissions on this namespace!" in str(e.value) @pytest.mark.registry @@ -146,7 +146,7 @@ def test_impersonate_external_object(readonly_pg_repo): # an object that they own. with pytest.raises(ProgrammingError) as e: readonly_pg_repo.objects.register_object_locations([(sample_object, "fake_location", "S3")]) - assert "You do not have access to this namespace!" in str(e.value) + assert "You do not have sufficient permissions on this namespace!" in str(e.value) @pytest.mark.registry