Skip to content

Revert 2ea214cf SQL permission verification from release/4.5.2 [AUTOMATED] #145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cluedin-bot wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Revert 2ea214cf SQL permission verification from release/4.5.2 [AUTOMATED] #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions docs/4.5.2-release-notes.md
View file
Open in desktop

This file was deleted.

152 changes: 0 additions & 152 deletions docs/minimum-sql-permissions.md
View file
Open in desktop

This file was deleted.

190 changes: 3 additions & 187 deletions src/Connector.SqlServer/Connector/SqlServerConnector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,28 +328,17 @@ public override async Task<ConnectionVerificationResult> VerifyConnection(Execut
schema = SqlTableName.DefaultSchema;
}


var schemaExists = await _client.VerifySchemaExists(connectionAndTransaction.Transaction, schema);

await connectionAndTransaction.DisposeAsync();

if (!schemaExists)
{
_logger.LogWarning("SqlServerConnector connection verification failed, schema '{schema}' does not exist", schema);
return new ConnectionVerificationResult(false, "Schema does not exist");
}

// Only perform detailed permission checks when explicitly invoked by a user
// This avoids running heavy checks every 30 seconds during automatic health checks
// additionally existing installations may be working fine without all permissions, so we don't want to break them by enforcing permission checks in health checks
if (executionContext.Principal != null)
{
var permissionResult = await VerifyRequiredPermissions(connectionAndTransaction.Transaction, schema);
if (!permissionResult.Success)
{
return permissionResult;
}
}

await connectionAndTransaction.DisposeAsync();

return new ConnectionVerificationResult(true);
}
catch (Exception e)
Expand All @@ -359,179 +348,6 @@ public override async Task<ConnectionVerificationResult> VerifyConnection(Execut
}
}

private async Task<ConnectionVerificationResult> VerifyRequiredPermissions(SqlTransaction transaction, string schema)
{
var missingPermissions = new List<string>();

// Check SELECT permission on INFORMATION_SCHEMA.TABLES (required for table existence checks and GetContainers)
if (!await CheckPermission(transaction, "SELECT", "INFORMATION_SCHEMA", "TABLES"))
{
missingPermissions.Add("SELECT on INFORMATION_SCHEMA.TABLES");
}

// Check SELECT permission on INFORMATION_SCHEMA.COLUMNS (required for schema upgrades and GetContainers)
if (!await CheckPermission(transaction, "SELECT", "INFORMATION_SCHEMA", "COLUMNS"))
{
missingPermissions.Add("SELECT on INFORMATION_SCHEMA.COLUMNS");
}

// To create a table in a schema, user needs:
// 1. CREATE TABLE permission at the database level
// 2. ALTER permission on the schema
if (!await CheckDatabasePermission(transaction, "CREATE TABLE"))
{
missingPermissions.Add("CREATE TABLE on database");
}

// Check CREATE TYPE permission at the database level (required for table-valued parameters used in bulk operations)
if (!await CheckDatabasePermission(transaction, "CREATE TYPE"))
{
missingPermissions.Add("CREATE TYPE on database");
}

// Check ALTER permission on the schema (required for CREATE TABLE in schema and ALTER TABLE during upgrades)
if (!await CheckSchemaPermission(transaction, "ALTER", schema))
{
missingPermissions.Add($"ALTER on schema [{schema}]");
}

// Check EXECUTE permission on the schema (required to execute table-valued types created in the schema)
if (!await CheckSchemaPermission(transaction, "EXECUTE", schema))
{
missingPermissions.Add($"EXECUTE on schema [{schema}]");
}

// Check SELECT permission on the schema
if (!await CheckSchemaPermission(transaction, "SELECT", schema))
{
missingPermissions.Add($"SELECT on schema [{schema}]");
}

// Check INSERT permission on the schema
if (!await CheckSchemaPermission(transaction, "INSERT", schema))
{
missingPermissions.Add($"INSERT on schema [{schema}]");
}

// Check UPDATE permission on the schema
if (!await CheckSchemaPermission(transaction, "UPDATE", schema))
{
missingPermissions.Add($"UPDATE on schema [{schema}]");
}

// Check DELETE permission on the schema
if (!await CheckSchemaPermission(transaction, "DELETE", schema))
{
missingPermissions.Add($"DELETE on schema [{schema}]");
}

// Check EXECUTE permission on sp_rename (required for Archive and Rename operations)
if (!await CheckExecutePermission(transaction, "sp_rename"))
{
missingPermissions.Add("EXECUTE on sp_rename");
}

if (missingPermissions.Any())
{
var errorMessage = $"Missing required SQL permissions: {string.Join(", ", missingPermissions)}";
_logger.LogError("SqlServerConnector connection verification failed: {ErrorMessage}", errorMessage);
return new ConnectionVerificationResult(false, errorMessage);
}

return new ConnectionVerificationResult(true);
}

private async Task<bool> CheckPermission(SqlTransaction transaction, string permission, string schemaName, string objectName)
{
try
{
// Use QUOTENAME to safely escape identifiers and prevent SQL injection
const string commandText = "SELECT HAS_PERMS_BY_NAME(QUOTENAME(@SchemaName) + '.' + QUOTENAME(@ObjectName), 'OBJECT', @Permission)";

var command = transaction.Connection.CreateCommand();
command.Transaction = transaction;
command.CommandText = commandText;
command.Parameters.AddWithValue("@SchemaName", schemaName);
command.Parameters.AddWithValue("@ObjectName", objectName);
command.Parameters.AddWithValue("@Permission", permission);

var result = await command.ExecuteScalarAsync();
return result is 1;
}
catch (Exception ex)
{
_logger.LogWarning(ex, "Failed to check permission {Permission} on [{Schema}].[{Object}]", permission, schemaName, objectName);
return false;
}
}

private async Task<bool> CheckDatabasePermission(SqlTransaction transaction, string permission)
{
try
{
const string commandText = "SELECT HAS_PERMS_BY_NAME(DB_NAME(), 'DATABASE', @Permission)";

var command = transaction.Connection.CreateCommand();
command.Transaction = transaction;
command.CommandText = commandText;
command.Parameters.AddWithValue("@Permission", permission);

var result = await command.ExecuteScalarAsync();
return result is 1;
}
catch (Exception ex)
{
_logger.LogWarning(ex, "Failed to check database permission {Permission}", permission);
return false;
}
}

private async Task<bool> CheckSchemaPermission(SqlTransaction transaction, string permission, string schemaName)
{
try
{
// Use QUOTENAME to safely escape identifiers and prevent SQL injection
const string commandText = "SELECT HAS_PERMS_BY_NAME(QUOTENAME(@SchemaName), 'SCHEMA', @Permission)";

var command = transaction.Connection.CreateCommand();
command.Transaction = transaction;
command.CommandText = commandText;
command.Parameters.AddWithValue("@SchemaName", schemaName);
command.Parameters.AddWithValue("@Permission", permission);

var result = await command.ExecuteScalarAsync();
return result is 1;
}
catch (Exception ex)
{
_logger.LogWarning(ex, "Failed to check schema permission {Permission} on schema [{Schema}]", permission, schemaName);
return false;
}
}

private async Task<bool> CheckExecutePermission(SqlTransaction transaction, string procedureName)
{
try
{
// Use QUOTENAME to safely escape identifiers and prevent SQL injection
// sp_rename is in sys schema
const string commandText = "SELECT HAS_PERMS_BY_NAME(QUOTENAME('sys') + '.' + QUOTENAME(@ProcedureName), 'OBJECT', 'EXECUTE')";

var command = transaction.Connection.CreateCommand();
command.Transaction = transaction;
command.CommandText = commandText;
command.Parameters.AddWithValue("@ProcedureName", procedureName);

var result = await command.ExecuteScalarAsync();
return result is 1;
}
catch (Exception ex)
{
_logger.LogWarning(ex, "Failed to check EXECUTE permission on {Procedure}", procedureName);
return false;
}
}

public override async Task CreateContainer(ExecutionContext executionContext, Guid connectorProviderDefinitionId, IReadOnlyCreateContainerModelV2 model)
{
await ExecuteWithRetryAsync(async () =>
Expand Down
Loading