Skip to content

Commit

Permalink
Fix bugs for GRANT/REVOKE on SCHEMA (babelfish-for-postgresql#2031)
Browse files Browse the repository at this point in the history 
Bug fixes  for GRANT/REVOKE .. ON SCHEMA

1. Server crash when using empty bracketed name.  `schema::[]`
2. Server crash when using empty quoted identifier. `schema::""`
3. When granting permission to yourself, we need a different error message.
4. Error message in not-supported multi-keyword permission should (i) be in uppercase (ii) have a space between keywords
5. Not-supported object type in error message should be in uppercase
6. GRANT ON SCHEMA:: TO PUBLIC does not take effect
7. Specified collation for column names using NAME datatype

Issues Resolved : BABEL-4344

Signed-off-by: Shalini Lohia <[email protected]>
  • Loading branch information
@shalinilohia50 @lohia-shalini
shalinilohia50 authored and lohia-shalini committed Nov 21, 2023
1 parent 67d1dab commit 44c44be
Show file tree
Hide file tree
Showing 14 changed files with 681 additions and 143 deletions.
10 changes: 5 additions & 5 deletions contrib/babelfishpg_tsql/sql/ownership.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ GRANT SELECT on sys.babelfish_sysdatabases TO PUBLIC;
-- BABELFISH_SCHEMA_PERMISSIONS
CREATE TABLE sys.babelfish_schema_permissions (
dbid smallint NOT NULL,
schema_name NAME NOT NULL,
object_name NAME NOT NULL,
permission NAME NOT NULL,
grantee NAME NOT NULL,
object_type NAME,
schema_name NAME NOT NULL COLLATE sys.database_default,
object_name NAME NOT NULL COLLATE sys.database_default,
permission NAME NOT NULL COLLATE sys.database_default,
grantee NAME NOT NULL COLLATE sys.database_default,
object_type NAME COLLATE sys.database_default,
PRIMARY KEY(dbid, schema_name, object_name, permission, grantee)
);

Expand Down
10 changes: 5 additions & 5 deletions contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -892,11 +892,11 @@ LANGUAGE plpgsql STABLE;
-- BABELFISH_SCHEMA_PERMISSIONS
CREATE TABLE IF NOT EXISTS sys.babelfish_schema_permissions (
dbid smallint NOT NULL,
schema_name NAME NOT NULL,
object_name NAME NOT NULL,
permission NAME NOT NULL,
grantee NAME NOT NULL,
object_type NAME,
schema_name NAME NOT NULL COLLATE sys.database_default,
object_name NAME NOT NULL COLLATE sys.database_default,
permission NAME NOT NULL COLLATE sys.database_default,
grantee NAME NOT NULL COLLATE sys.database_default,
object_type NAME COLLATE sys.database_default,
PRIMARY KEY(dbid, schema_name, object_name, permission, grantee)
);

Expand Down
193 changes: 134 additions & 59 deletions contrib/babelfishpg_tsql/src/catalog.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2843,6 +2843,10 @@ add_entry_to_bbf_schema(const char *schema_name,
bool new_record_nulls_bbf_schema[BBF_SCHEMA_PERMS_NUM_OF_COLS];
int16 dbid = get_cur_db_id();

/* Immediately return, if grantee is NULL or PUBLIC. */
if ((grantee == NULL) || (strcmp(grantee, "public") == 0))
return;

/* Fetch the relation */
bbf_schema_rel = table_open(get_bbf_schema_perms_oid(),
RowExclusiveLock);
Expand Down Expand Up @@ -2885,45 +2889,65 @@ check_bbf_schema_for_entry(const char *schema_name,
{
Relation bbf_schema_rel;
HeapTuple tuple_bbf_schema;
ScanKeyData key[5];
TableScanDesc scan;
ScanKeyData scanKey[5];
SysScanDesc scan;
bool catalog_entry_exists = false;
int16 dbid = get_cur_db_id();

/* Immediately return false, if grantee is NULL or PUBLIC. */
if ((grantee == NULL) || (strcmp(grantee, "public") == 0))
return false;

bbf_schema_rel = table_open(get_bbf_schema_perms_oid(),
AccessShareLock);
ScanKeyInit(&key[0],
ScanKeyInit(&scanKey[0],
Anum_bbf_schema_perms_dbid,
BTEqualStrategyNumber, F_INT2EQ,
Int16GetDatum(dbid));
ScanKeyInit(&key[1],
ScanKeyEntryInitialize(&scanKey[1], 0,
Anum_bbf_schema_perms_schema_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(schema_name));
ScanKeyInit(&key[2],
ScanKeyEntryInitialize(&scanKey[2], 0,
Anum_bbf_schema_perms_object_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(object_name));
ScanKeyInit(&key[3],
ScanKeyEntryInitialize(&scanKey[3], 0,
Anum_bbf_schema_perms_permission,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(permission));
ScanKeyInit(&key[4],
ScanKeyEntryInitialize(&scanKey[4], 0,
Anum_bbf_schema_perms_grantee,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(grantee));
scan = systable_beginscan(bbf_schema_rel,
get_bbf_schema_perms_idx_oid(),
true, NULL, 5, scanKey);

scan = table_beginscan_catalog(bbf_schema_rel, 5, key);

tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection);
tuple_bbf_schema = systable_getnext(scan);
if (HeapTupleIsValid(tuple_bbf_schema))
catalog_entry_exists = true;

table_endscan(scan);
systable_endscan(scan);
table_close(bbf_schema_rel, AccessShareLock);
return catalog_entry_exists;
}

/*
* Checks if a particular schema has any SCHEMA level permission granted to any user.
*/
bool
check_bbf_schema_for_schema(const char *schema_name,
const char *object_name,
Expand All @@ -2932,7 +2956,7 @@ check_bbf_schema_for_schema(const char *schema_name,
Relation bbf_schema_rel;
HeapTuple tuple_bbf_schema;
ScanKeyData key[4];
TableScanDesc scan;
SysScanDesc scan;
bool catalog_entry_exists = false;
int16 dbid = get_cur_db_id();

Expand All @@ -2942,26 +2966,37 @@ check_bbf_schema_for_schema(const char *schema_name,
Anum_bbf_schema_perms_dbid,
BTEqualStrategyNumber, F_INT2EQ,
Int16GetDatum(dbid));
ScanKeyInit(&key[1],
ScanKeyEntryInitialize(&key[1], 0,
Anum_bbf_schema_perms_schema_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(schema_name));
ScanKeyInit(&key[2],
ScanKeyEntryInitialize(&key[2], 0,
Anum_bbf_schema_perms_object_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(object_name));
ScanKeyInit(&key[3],
ScanKeyEntryInitialize(&key[3], 0,
Anum_bbf_schema_perms_permission,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(permission));

scan = table_beginscan_catalog(bbf_schema_rel, 4, key);
scan = systable_beginscan(bbf_schema_rel,
get_bbf_schema_perms_idx_oid(),
true, NULL, 4, key);

tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection);
tuple_bbf_schema = systable_getnext(scan);
if (HeapTupleIsValid(tuple_bbf_schema))
catalog_entry_exists = true;

table_endscan(scan);
systable_endscan(scan);
table_close(bbf_schema_rel, AccessShareLock);
return catalog_entry_exists;
}
Expand All @@ -2974,44 +3009,59 @@ del_from_bbf_schema(const char *schema_name,
{
Relation bbf_schema_rel;
HeapTuple tuple_bbf_schema;
ScanKeyData key[5];
TableScanDesc scan;
ScanKeyData scanKey[5];
SysScanDesc scan;
int16 dbid = get_cur_db_id();

/* Immediately return, if grantee is NULL or PUBLIC. */
if ((grantee == NULL) || (strcmp(grantee, "public") == 0))
return;

bbf_schema_rel = table_open(get_bbf_schema_perms_oid(),
RowExclusiveLock);
ScanKeyInit(&key[0],
ScanKeyInit(&scanKey[0],
Anum_bbf_schema_perms_dbid,
BTEqualStrategyNumber, F_INT2EQ,
Int16GetDatum(dbid));
ScanKeyInit(&key[1],
ScanKeyEntryInitialize(&scanKey[1], 0,
Anum_bbf_schema_perms_schema_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(schema_name));
ScanKeyInit(&key[2],
ScanKeyEntryInitialize(&scanKey[2], 0,
Anum_bbf_schema_perms_object_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(object_name));
ScanKeyInit(&key[3],
ScanKeyEntryInitialize(&scanKey[3], 0,
Anum_bbf_schema_perms_permission,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(permission));
ScanKeyInit(&key[4],
ScanKeyEntryInitialize(&scanKey[4], 0,
Anum_bbf_schema_perms_grantee,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(grantee));
scan = systable_beginscan(bbf_schema_rel,
get_bbf_schema_perms_idx_oid(),
true, NULL, 5, scanKey);

scan = table_beginscan_catalog(bbf_schema_rel, 5, key);

tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection);
tuple_bbf_schema = systable_getnext(scan);

if (HeapTupleIsValid(tuple_bbf_schema))
CatalogTupleDelete(bbf_schema_rel, &tuple_bbf_schema->t_self);

table_endscan(scan);
systable_endscan(scan);
table_close(bbf_schema_rel, RowExclusiveLock);

CommandCounterIncrement();
}

void
Expand All @@ -3035,9 +3085,12 @@ clean_up_bbf_schema(const char *schema_name,
Anum_bbf_schema_perms_dbid,
BTEqualStrategyNumber, F_INT2EQ,
Int16GetDatum(dbid));
ScanKeyInit(&scanKey[1],
ScanKeyEntryInitialize(&scanKey[1], 0,
Anum_bbf_schema_perms_schema_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(schema_name));
scan = systable_beginscan(bbf_schema_rel,
get_bbf_schema_perms_idx_oid(),
Expand All @@ -3050,13 +3103,19 @@ clean_up_bbf_schema(const char *schema_name,
Anum_bbf_schema_perms_dbid,
BTEqualStrategyNumber, F_INT2EQ,
Int16GetDatum(dbid));
ScanKeyInit(&scanKey[1],
ScanKeyEntryInitialize(&scanKey[1], 0,
Anum_bbf_schema_perms_schema_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(schema_name));
ScanKeyInit(&scanKey[2],
ScanKeyEntryInitialize(&scanKey[2], 0,
Anum_bbf_schema_perms_object_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(object_name));
scan = systable_beginscan(bbf_schema_rel,
get_bbf_schema_perms_idx_oid(),
Expand All @@ -3074,12 +3133,18 @@ clean_up_bbf_schema(const char *schema_name,
table_close(bbf_schema_rel, RowExclusiveLock);
}

/*
* For all objects belonging to a schema which has OBJECT level permission,
* It grants the permission explicitly when REVOKE has been executed on that
* specific schema.
*/

void
grant_perms_to_objects_in_schema(const char *schema_name,
const char *permission,
const char *grantee)
{
TableScanDesc scan;
SysScanDesc scan;
Relation bbf_schema_rel;
HeapTuple tuple_bbf_schema;
const char *object_name;
Expand All @@ -3095,21 +3160,31 @@ grant_perms_to_objects_in_schema(const char *schema_name,
Anum_bbf_schema_perms_dbid,
BTEqualStrategyNumber, F_INT2EQ,
Int16GetDatum(dbid));
ScanKeyInit(&scanKey[1],
ScanKeyEntryInitialize(&scanKey[1], 0,
Anum_bbf_schema_perms_schema_name,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(schema_name));
ScanKeyInit(&scanKey[2],
ScanKeyEntryInitialize(&scanKey[2], 0,
Anum_bbf_schema_perms_permission,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(permission));
ScanKeyInit(&scanKey[3],
ScanKeyEntryInitialize(&scanKey[3], 0,
Anum_bbf_schema_perms_grantee,
BTEqualStrategyNumber, F_NAMEEQ,
BTEqualStrategyNumber,
InvalidOid,
tsql_get_server_collation_oid_internal(false),
F_NAMEEQ,
CStringGetDatum(grantee));

scan = table_beginscan_catalog(bbf_schema_rel, 4, scanKey);
tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection);
scan = systable_beginscan(bbf_schema_rel, get_bbf_schema_perms_idx_oid(),
true, NULL, 4, scanKey);
tuple_bbf_schema = systable_getnext(scan);

while (HeapTupleIsValid(tuple_bbf_schema))
{
Expand Down Expand Up @@ -3162,9 +3237,9 @@ grant_perms_to_objects_in_schema(const char *schema_name,
/* make sure later steps can see the object created here */
CommandCounterIncrement();
}
tuple_bbf_schema = heap_getnext(scan, ForwardScanDirection);
tuple_bbf_schema = systable_getnext(scan);
}
table_endscan(scan);
systable_endscan(scan);
table_close(bbf_schema_rel, AccessShareLock);
}

Expand Down
2 changes: 2 additions & 0 deletions contrib/babelfishpg_tsql/src/catalog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ typedef FormData_bbf_function_ext *Form_bbf_function_ext;
#define Anum_bbf_schema_perms_grantee 5
#define Anum_bbf_schema_perms_object_type 6

#define PERMISSIONS_FOR_ALL_OBJECTS_IN_SCHEMA "ALL"

extern Oid bbf_schema_perms_oid;
extern Oid bbf_schema_perms_idx_oid;

Expand Down
Loading

0 comments on commit 44c44be

Please sign in to comment.