7
\$\begingroup\$

I'm currently faced with a problem in our SQL Server environments. We need to migrate our existing logins, users and roles to a new login, user and role with the same permissions as the old. I've been tasked with writing a stored procedure that will accept 3 parameters:

  1. Database Name
  2. Old Role
  3. New Role

Using these parameters, I need to query the system views to get the list of all the objects assigned to a Role and then execute some process to assign these permissions to the new role.

I've written a stored procedure that works but unfortunately uses a WHILE loop and a row-by-agonizing-row dynamic sql execution to assign the permissions. I'd like to know if there are any improvements that would be made to the code to maybe avoid the RBAR process or any other potential pitfalls that are apparent in the code.

CREATE PROCEDURE dbo.usp_AssignPermissionsToRole
(
    @OldRoleName VARCHAR(50),
    @NewRoleName VARCHAR(50),
    @Database VARCHAR(50)
)
AS
BEGIN
    DECLARE @DataSQL NVARCHAR(MAX);
    DECLARE @PermissionSQL NVARCHAR(MAX);
    DECLARE @Start INT = 1;
    DECLARE @Total INT;
    DECLARE @PermissionState VARCHAR(60);
    DECLARE @PermissionName VARCHAR(500);
    DECLARE @SchemaName VARCHAR(50);
    DECLARE @ObjectName VARCHAR(500);

    IF(OBJECT_ID('tempdb..#OldPermissions') IS NOT NULL)
        DROP TABLE #OldPermissions;

    -- get the list of all permissions for the OldRole
    CREATE TABLE #OldPermissions
    (
        ID INT IDENTITY,
        RoleName VARCHAR(50),
        PermissionState VARCHAR(60),
        PermissionName VARCHAR(500),
        SchemaName VARCHAR(50),
        ObjectName VARCHAR(500)
    );

    SET @DataSQL = 'USE '+QUOTENAME(@Database)+'
                    INSERT INTO #OldPermissions
                    (
                        RoleName,
                        PermissionState,
                        PermissionName,
                        SchemaName,
                        ObjectName
                    )
                    SELECT 
                        RoleName = r.name,
                        PermissionState = dp.state_desc,
                        dp.permission_name,
                        SchemaName = s.name,
                        ObjectName = o.name
                    FROM sys.database_permissions dp
                    INNER JOIN sys.database_principals r
                        ON dp.grantee_principal_id = r.principal_id
                    INNER JOIN sys.objects o
                        ON dp.major_id = o.[object_id]
                    INNER JOIN sys.schemas s
                        ON o.schema_id = s.schema_id
                    WHERE r.name = '+QUOTENAME(@OldRoleName, '''');

    EXEC(@DataSQL);

    -- ugly cursor to execute each of these permission changes
    -- only assign permission if the role already exists
    -- Need to be sure that the role is set up on the Database we are passing int
    DECLARE @RoleExistsSQL nvarchar(max);
    DECLARE @ReturnVal INT;
    SET @RoleExistsSQL = 'USE '+QUOTENAME(@Database)+' SELECT @ReturnVal=1 FROM sys.database_principals WHERE [type] = ''R'' AND [name] = '+QUOTENAME(@NewRoleName, '''');
    EXEC sp_executesql @RoleExistsSQL, N'@ReturnVal INT OUTPUT', @ReturnVal OUTPUT;

    IF (@ReturnVal = 1)
        BEGIN

            SET @Total = (SELECT COUNT(*) FROM #OldPermissions);

            WHILE (@Start <= @Total)
                BEGIN
                    -- get the permission for each row
                    SELECT 
                        @PermissionState = PermissionState,
                        @PermissionName = PermissionName,
                        @SchemaName = SchemaName,
                        @ObjectName = ObjectName
                    FROM #OldPermissions
                    WHERE ID = @Start;

                    -- create the dynamic SQL to assign the new permission to the new role
                    SET @PermissionSQL = 'USE  '+QUOTENAME(@Database)+' '
                                            +@PermissionState + ' ' + @PermissionName + ' on '
                                            +QUOTENAME(@SchemaName)+'.'+QUOTENAME(@ObjectName)+' to '
                                            +QUOTENAME(@NewRoleName);

                    EXEC(@PermissionSQL);

                    SET @Start = @Start + 1;
                END
        END
    ELSE
        BEGIN
            PRINT 'Role: ' + @NewRoleName +' not set up in this database.';
        END
END
GO

Is there any way to avoid the row-by-row processing? Or any other improvements that can be made?

\$\endgroup\$
4
\$\begingroup\$

Alright, here is my take on it:

CREATE PROCEDURE dbo.usp_AssignPermissionsToRole
(
    @OldRoleName VARCHAR(50),
    @NewRoleName VARCHAR(50),
    @Database VARCHAR(50)
)
AS
BEGIN
    DECLARE @DataSQL NVARCHAR(MAX);
    DECLARE @PermissionSQL NVARCHAR(MAX) = '';
    DECLARE @Start INT = 1;
    DECLARE @Total INT;
    DECLARE @PermissionState VARCHAR(60);
    DECLARE @PermissionName VARCHAR(500);
    DECLARE @SchemaName VARCHAR(50);
    DECLARE @ObjectName VARCHAR(500);

    -- This check is unnecessary
    --IF(OBJECT_ID('tempdb..#OldPermissions') IS NOT NULL)
    --    DROP TABLE #OldPermissions;

    -- I would check for the existence of the new role here:
    DECLARE @RoleExistsSQL NVARCHAR(MAX);
    DECLARE @ReturnVal INT;
    SET @RoleExistsSQL = 'USE '+QUOTENAME(@Database)+' SELECT @ReturnVal=1 FROM sys.database_principals WHERE [type] = ''R'' AND [name] = '+QUOTENAME(@NewRoleName, '''');
    EXEC sp_executesql @RoleExistsSQL, N'@ReturnVal INT OUTPUT', @ReturnVal OUTPUT;

    IF (@ReturnVal <> 1)
        BEGIN
            PRINT 'Role: ' + @NewRoleName +' not set up in this database.';
            RETURN;
        END
    ELSE
        BEGIN
            -- get the list of all permissions for the OldRole
            CREATE TABLE #OldPermissions
            (
                ID INT IDENTITY,
                RoleName VARCHAR(50),
                PermissionState VARCHAR(60),
                PermissionName VARCHAR(500),
                SchemaName VARCHAR(50),
                ObjectName VARCHAR(500)
            );

            SET @DataSQL = 'USE '+QUOTENAME(@Database)+'
                            INSERT INTO #OldPermissions
                            (
                                RoleName,
                                PermissionState,
                                PermissionName,
                                SchemaName,
                                ObjectName
                            )
                            SELECT 
                                RoleName = r.name,
                                PermissionState = dp.state_desc,
                                dp.permission_name,
                                SchemaName = s.name,
                                ObjectName = o.name
                            FROM sys.database_permissions dp
                            INNER JOIN sys.database_principals r
                                ON dp.grantee_principal_id = r.principal_id
                            INNER JOIN sys.objects o
                                ON dp.major_id = o.[object_id]
                            INNER JOIN sys.schemas s
                                ON o.schema_id = s.schema_id
                            WHERE r.name = '+QUOTENAME(@OldRoleName, '''');

            EXEC sp_executesql @DataSQL; --Changed the EXEC to sp_executesql for consistency

            -- I concatenated all the commands needed on the same string to execute it once
            SELECT @PermissionSQL = @PermissionSQL + PermissionState + ' ' + PermissionName + ' on ' + 
                                    QUOTENAME(SchemaName)+'.'+QUOTENAME(ObjectName)+' to ' + QUOTENAME(@NewRoleName) + '; '
            FROM #OldPermissions;

            SET @PermissionSQL = 'USE  '+QUOTENAME(@Database)+' '+@PermissionSQL;
            EXEC sp_executesql @PermissionSQL;

        END

END

The main changes are checking existence of the new role at the beginning of the sp to avoid unnecessary creating and populating the temp table with the data of the old role. Then I concatenated all the GRANT or DENY permissions on the same string (using an undocumented behaviour I'm afraid) without using a WHILE so you need to execute it only once.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.