Skip to content

Commit d9f2a1a

Browse files
ayush3797seantleonard
ayush3797
and
seantleonard
authored
Preventing update/insert of read-only fields in a table by user (#1596)
## Why make this change? There are certain type of columns in databases which cannot be provided a value (not even null value) for by the user. Eg. in MsSql tables, we cannot provide values for columns that are computed base on other columns in the table or have a datatype of timestamp/rowversion. Similarly for PgSql,/MySql we cannot provide a value for generated columns. This PR prevents DAB from allowing user to attempt an update/insert of such columns. The way DAB will do this is by collecting metadata about columns during the startup phase and populate the `IsReadOnly` property of the `ColumnDefinition` of each column in the table. More details about the specifics can be found on the linked issues for MsSql(#1453), PgSql(#1584), MySql(#1583). **NOTE:** The columns which are autogenerated are also considered as read-only columns irrespective of the database flavor (MsSql/PgSql/MySql). ## What is this change? - Added a new boolean property `IsReadOnly` to `ColumnDefinition` class which if true, will indicate that the column is read-only, i.e. it cannot be provided a value for (via INSERT/UPDATE). - Added a new method `GetQueryToGetReadOnlyColumns()` to `IQueryBuilder.cs` class which will have its overridden implementation in Pg/My/MsSql. This method will return the query that will fetch the metadata about the whether a column is read-only. - Added validations to `RequestValidator.cs` class which will ensure that no read-only column is present the request body either for update (via `Upsert`/`UpsertIncremental`/`Update`/`UpdateIncremental`) or insert operations. - Added similar validations to `SqlInsertQueryStructure.cs`/`SqlUpdateQueryStructure.cs`- classes as the request validation stage is not a part of the codeflow for GQL requests. - Via GQL, value for a read-only field cannot be provided. This will be enabled by adding the `AutoGeneratedDirectiveType` when a column is read-only. Previous this directive was only added for autogenerated fields (which is a subset of read-only fields). ## Implemented Behaviors: 1. `When a read-only field is included in the request body for mutation:` This is a bad request as we cannot provide values for read-only fields. We will throw an exception no matter what operation is being executed (`PUT/PATCH/POST via REST). For create/update via GQL`, we would see a warning in the BCP UI for read-only fields as well that the field is not valid for mutation (just like we see currently for autogen fields). 2. `When a read-only field is excluded from the request body for mutation`: This is a valid scenario. In this case (`for PUT/PATCH operations via REST`), we won't NULL out the field's value but let the responsibility on the database to deal with it. For other operations, we really don't need to do anything as the field is not present in the request body. ## How was this tested? - [x] Integration Tests --------- Co-authored-by: Sean Leonard <[email protected]>
1 parent 7b4c8b5 commit d9f2a1a

39 files changed

Lines changed: 1161 additions & 47 deletions

‎config-generators/mssql-commands.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ add InsertAndDisplayAllBooksUnderGivenPublisher --config "dab-config.MsSql.json"
3939
add GQLmappings --config "dab-config.MsSql.json" --source "GQLmappings" --permissions "anonymous:*" --rest true --graphql true
4040
add Bookmarks --config "dab-config.MsSql.json" --source "bookmarks" --permissions "anonymous:*" --rest true --graphql true
4141
add MappedBookmarks --config "dab-config.MsSql.json" --source "mappedbookmarks" --permissions "anonymous:*" --rest true --graphql true
42+
add BooksSold --config "dab-config.MsSql.json" --source "books_sold" --rest true --graphql "books_sold:books_sold" --permissions "anonymous:*"
4243
update GQLmappings --config "dab-config.MsSql.json" --map "__column1:column1,__column2:column2" --permissions "authenticated:*"
4344
update Publisher --config "dab-config.MsSql.json" --permissions "authenticated:create,read,update,delete" --rest true --graphql true --relationship books --target.entity Book --cardinality many
4445
update Publisher --config "dab-config.MsSql.json" --permissions "policy_tester_01:create,delete"

‎config-generators/mysql-commands.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ add Sales --config "dab-config.MySql.json" --source "sales" --permissions "anony
2727
add GQLmappings --config "dab-config.MySql.json" --source "GQLmappings" --permissions "anonymous:*" --rest true --graphql true
2828
add Bookmarks --config "dab-config.MySql.json" --source "bookmarks" --permissions "anonymous:*" --rest true --graphql true
2929
add MappedBookmarks --config "dab-config.MySql.json" --source "mappedbookmarks" --permissions "anonymous:*" --rest true --graphql true
30+
add BooksSold --config "dab-config.MySql.json" --source "books_sold" --rest true --graphql "books_sold:books_sold" --permissions "anonymous:*"
3031
update GQLmappings --config "dab-config.MySql.json" --map "__column1:column1,__column2:column2" --permissions "authenticated:*"
3132
update Publisher --config "dab-config.MySql.json" --permissions "authenticated:create,read,update,delete" --rest true --graphql true --relationship books --target.entity Book --cardinality many
3233
update Publisher --config "dab-config.MySql.json" --permissions "policy_tester_01:create,delete"

‎config-generators/postgresql-commands.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ add Sales --config "dab-config.PostgreSql.json" --source "sales" --permissions "
2929
add GQLmappings --config "dab-config.PostgreSql.json" --source "gqlmappings" --permissions "anonymous:*" --rest true --graphql true
3030
add Bookmarks --config "dab-config.PostgreSql.json" --source "bookmarks" --permissions "anonymous:*" --rest true --graphql true
3131
add MappedBookmarks --config "dab-config.PostgreSql.json" --source "mappedbookmarks" --permissions "anonymous:*" --rest true --graphql true
32+
add BooksSold --config "dab-config.PostgreSql.json" --source "books_sold" --rest true --graphql "books_sold:books_sold" --permissions "anonymous:*"
3233
update GQLmappings --config "dab-config.PostgreSql.json" --map "__column1:column1,__column2:column2" --permissions "authenticated:*"
3334
update Publisher --config "dab-config.PostgreSql.json" --permissions "authenticated:create,read,update,delete" --rest true --graphql true --relationship books --target.entity Book --cardinality many
3435
update Publisher --config "dab-config.PostgreSql.json" --permissions "policy_tester_01:create,delete"

‎src/Config/DatabasePrimitives/DatabaseObject.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ public class ColumnDefinition
215215
public bool HasDefault { get; set; }
216216
public bool IsAutoGenerated { get; set; }
217217
public bool IsNullable { get; set; }
218+
public bool IsReadOnly { get; set; }
218219
public object? DefaultValue { get; set; }
219220

220221
public ColumnDefinition() { }

‎src/Core/Resolvers/IQueryBuilder.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ public interface IQueryBuilder
6464
/// <returns></returns>
6565
public string BuildStoredProcedureResultDetailsQuery(string databaseObjectName);
6666

67+
/// <summary>
68+
/// Returns the query to get the read-only columns present in the table.
69+
/// </summary>
70+
/// <param name="schemaOrDatabaseParamName">Param name of the schema/database.</param>
71+
/// <param name="tableParamName">Param name of the table.</param>
72+
/// <exception cref="NotImplementedException">Thrown when class implementing the interface has not provided
73+
/// an overridden implementation of the method.</exception>
74+
string BuildQueryToGetReadOnlyColumns(string schemaOrDatabaseParamName, string tableParamName);
75+
6776
/// <summary>
6877
/// Adds database specific quotes to string identifier
6978
/// </summary>

‎src/Core/Resolvers/MsSqlQueryBuilder.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,5 +256,26 @@ public string BuildStoredProcedureResultDetailsQuery(string databaseObjectName)
256256
"WHERE is_hidden is not NULL AND is_hidden = 0";
257257
return query;
258258
}
259+
260+
/// <summary>
261+
/// Builds the query to get all the read-only columns in an MsSql table.
262+
/// For MsSql, the columns:
263+
/// 1. That have data_type of 'timestamp', or
264+
/// 2. are computed based on other columns,
265+
/// are considered as read only columns. The query combines both the types of read-only columns and returns the list.
266+
/// </summary>
267+
/// <param name="schemaOrDatabaseParamName">Param name of the schema/database.</param>
268+
/// <param name="tableParamName">Param name of the table.</param>
269+
/// <returns></returns>
270+
public string BuildQueryToGetReadOnlyColumns(string schemaParamName, string tableParamName)
271+
{
272+
// For 'timestamp' columns sc.is_computed = 0.
273+
string query = "SELECT ifsc.column_name from sys.columns as sc INNER JOIN information_schema.columns as ifsc " +
274+
"ON (sc.is_computed = 1 or ifsc.data_type = 'timestamp') " +
275+
$"AND sc.object_id = object_id({schemaParamName}+'.'+{tableParamName}) and ifsc.table_name = {tableParamName} " +
276+
$"AND ifsc.table_schema = {schemaParamName} and ifsc.column_name = sc.name;";
277+
278+
return query;
279+
}
259280
}
260281
}

‎src/Core/Resolvers/MySqlQueryBuilder.cs‎

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ public string Build(SqlInsertStructure structure)
7676
/// <inheritdoc />
7777
public string Build(SqlUpdateStructure structure)
7878
{
79-
(string sets, string updates, string select) = MakeStoreUpdatePK(structure.AllColumns(),
80-
structure.OutputColumns);
79+
(string sets, string updates, string select) = MakeQuerySegmentsForUpdate(structure, structure.OutputColumns);
8180
string predicates = JoinPredicateStrings(
8281
structure.GetDbPolicyForOperation(EntityActionOperation.Update),
8382
Build(structure.Predicates));
@@ -113,8 +112,7 @@ public string Build(SqlExecuteStructure structure)
113112
/// <inheritdoc />
114113
public string Build(SqlUpsertQueryStructure structure)
115114
{
116-
(string sets, string updates, string select) = MakeStoreUpdatePK(structure.AllColumns(),
117-
structure.OutputColumns);
115+
(string sets, string updates, string select) = MakeQuerySegmentsForUpdate(structure, structure.OutputColumns);
118116

119117
if (structure.IsFallbackToUpdate)
120118
{
@@ -177,20 +175,47 @@ AND REFERENCED_TABLE_NAME IS NOT NULL
177175
}
178176

179177
/// <summary>
180-
/// Makes the query segments to store PK during an update
178+
/// Makes the query segments to store PK during an update. For each of the constructed segments, we do not include fields which are
179+
/// read-only because read-only fields cannot be included in an update statement as their value cannot be updated. And consequently,
180+
/// they cannot be included in the subsequent select statement as well.
181181
/// </summary>
182-
private (string, string, string) MakeStoreUpdatePK(List<string> columns,
183-
List<LabelledColumn> outputColumns)
182+
/// <param name="structure">Query structure of the update/upsert query.</param>
183+
/// <param name="outputColumns">List of columns to be returned.</param>
184+
/// <returns>A tuple of 3 strings where:
185+
/// 1. The first string is for the set clause: to create local variables to store the updatable columns.
186+
/// 2. The second string is for the update clause: to fetch the values of the updatable columns to local variables.
187+
/// 3. The third string is for the select clause: to select local variables and mapping to original column name.
188+
/// </returns>
189+
private (string, string, string) MakeQuerySegmentsForUpdate(BaseSqlQueryStructure structure, List<LabelledColumn> outputColumns)
184190
{
185-
// Create local variables to store the columns
186-
string sets = String.Join(";\n", columns.Select((x, index) => $"SET {"@LU_" + index.ToString()} := 0"));
187-
188-
// Fetch the value to local variables
189-
string updates = String.Join(", ", columns.Select((x, index) =>
190-
$"{QuoteIdentifier(x)} = (SELECT {"@LU_" + index.ToString()} := {QuoteIdentifier(x)})"));
191-
192-
// Select local variables and mapping to original column name
193-
string select = String.Join(", ", outputColumns.Select((x, index) => $"{"@LU_" + index.ToString()} AS {QuoteIdentifier(x.Label)}"));
191+
SourceDefinition sourceDefinition = structure.GetUnderlyingSourceDefinition();
192+
List<string> columns = structure.AllColumns();
193+
194+
// Create local variables to store the updatable columns.
195+
string sets = String.Join(";\n",
196+
columns.Where(col => !sourceDefinition.Columns[col].IsReadOnly || sourceDefinition.Columns[col].IsAutoGenerated)
197+
.Select((col, index) => $"SET {"@LU_" + index.ToString()} := 0"));
198+
199+
// Fetch the values of the updatable columns to local variables.
200+
string updates = String.Join(", ",
201+
columns.Where(col => !sourceDefinition.Columns[col].IsReadOnly || sourceDefinition.Columns[col].IsAutoGenerated)
202+
.Select((col, index) => $"{QuoteIdentifier(col)} = (SELECT {"@LU_" + index.ToString()} := {QuoteIdentifier(col)})"));
203+
204+
// Select local variables and mapping to original column name.
205+
string select = String.Join(", ",
206+
outputColumns.Where(col => !sourceDefinition.Columns[col.ColumnName].IsReadOnly || sourceDefinition.Columns[col.ColumnName].IsAutoGenerated)
207+
.Select((col, index) => $"{"@LU_" + index.ToString()} AS {QuoteIdentifier(col.Label)}"));
208+
/*
209+
* An example tuple of sets,updates, and select would look like:
210+
* sets:
211+
* SET @LU_0 := 0
212+
* SET @LU_1 := 0;
213+
* SET @LU_2 := 0
214+
* updates:
215+
* `param0` = (SELECT @LU_0 := `param0`), `param1` = (SELECT @LU_1 := `param1`), `param2` = (SELECT @LU_2 := `param2`)
216+
* select:
217+
* @LU_0 AS `param0`, @LU_1 AS `param1`, @LU_2 AS `param2`
218+
*/
194219

195220
return (sets, updates, select);
196221
}
@@ -289,13 +314,18 @@ private string MakeUpsertSelections(SqlUpsertQueryStructure structure)
289314
{
290315
string quotedColName = QuoteIdentifier(column.Label);
291316
ColumnDefinition columnDefinition = structure.GetColumnDefinition(column.ColumnName);
292-
if (insertColumnsToParamName.TryGetValue(column.ColumnName, out string? paramName))
317+
if (columnDefinition.IsAutoGenerated)
293318
{
294-
selections.Add($"{paramName} AS {quotedColName}");
319+
selections.Add($"LAST_INSERT_ID() AS {quotedColName}");
295320
}
296-
else if (columnDefinition.IsAutoGenerated)
321+
else if (columnDefinition.IsReadOnly)
297322
{
298-
selections.Add($"LAST_INSERT_ID() AS {quotedColName}");
323+
// We cannot update a read-only column and hence cannot include it in the response.
324+
continue;
325+
}
326+
else if (insertColumnsToParamName.TryGetValue(column.ColumnName, out string? paramName))
327+
{
328+
selections.Add($"{paramName} AS {quotedColName}");
299329
}
300330
else if (columnDefinition.HasDefault)
301331
{
@@ -323,6 +353,14 @@ private static string GetMySQLDefaultValue(ColumnDefinition column)
323353
return defaultValue;
324354
}
325355

356+
/// <inheritdoc/>
357+
public string BuildQueryToGetReadOnlyColumns(string schemaParamName, string tableParamName)
358+
{
359+
string query = "select column_name as column_name from information_schema.columns " +
360+
$"where table_schema = {schemaParamName} and table_name = {tableParamName} and generation_expression != '';";
361+
return query;
362+
}
363+
326364
/// <inheritdoc/>
327365
public string BuildStoredProcedureResultDetailsQuery(string databaseObjectName)
328366
{

‎src/Core/Resolvers/PostgresQueryBuilder.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,5 +224,13 @@ public string BuildStoredProcedureResultDetailsQuery(string databaseObjectName)
224224
{
225225
throw new NotImplementedException();
226226
}
227+
228+
/// <inheritdoc/>
229+
public string BuildQueryToGetReadOnlyColumns(string schemaParamName, string tableParamName)
230+
{
231+
string query = "SELECT attname AS column_name FROM pg_attribute " +
232+
$"WHERE attrelid = ({schemaParamName} || '.' || {tableParamName})::regclass AND attgenerated = 's';";
233+
return query;
234+
}
227235
}
228236
}

‎src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,9 @@ public void AddNullifiedUnspecifiedFields(
100100
//result of adding (SourceDefinition.Columns - MutationFields) to UpdateOperations
101101
foreach (string leftoverColumn in leftoverSchemaColumns)
102102
{
103-
// If the left over column is autogenerated
103+
// If the left over column is a read-only column,
104104
// then no need to add it with a null value.
105-
if (sourceDefinition.Columns[leftoverColumn].IsAutoGenerated)
105+
if (sourceDefinition.Columns[leftoverColumn].IsReadOnly)
106106
{
107107
continue;
108108
}

‎src/Core/Resolvers/Sql Query Structures/SqlInsertQueryStructure.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ HttpContext httpContext
7070
InsertColumns = new();
7171
Values = new();
7272
OutputColumns = GenerateOutputColumns();
73-
7473
foreach (KeyValuePair<string, object?> param in mutationParams)
7574
{
7675
MetadataProvider.TryGetBackingColumn(EntityName, param.Key, out string? backingColumn);

0 commit comments

Comments
 (0)