From 2518cd16cb1ca3e57ca4595dd762a24e8844bcdc Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Wed, 17 Aug 2016 13:55:23 -0700 Subject: [PATCH] (MODULES-3752) Fix modifying server_roles for an existing WINDOWS_LOGIN Previously the SQL template for modifying a login and the associated acceptance tests were using the IS_SRVROLEMEMBER function to determine membership. However this function has many caveats e.g. failing for domain principals if a domain controller is not contactable, and always return false for a disabled Windows user login. This commit changes the check to use the `sys.server_role_members` table which holds the underlying membership information. --- spec/acceptance/sqlserver_login_spec.rb | 16 ++++++++++++++-- templates/create/login.sql.erb | 7 ++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/spec/acceptance/sqlserver_login_spec.rb b/spec/acceptance/sqlserver_login_spec.rb index 8e7787f8..4af7e4d5 100644 --- a/spec/acceptance/sqlserver_login_spec.rb +++ b/spec/acceptance/sqlserver_login_spec.rb @@ -288,11 +288,23 @@ def create_login_manifest (testcase,login_name,login_password,options = {}) end it "should have the specified sysadmin role" do - query = "SELECT 'is_sysadmin' AS result WHERE IS_SRVROLEMEMBER('sysadmin','#{@login_under_test}') = 1" + # Note - IS_SRVROLEMEMBER always returns false for a disabled WINDOWS_LOGIN user login + query = "SELECT pri.name from sys.server_role_members member + JOIN sys.server_principals rol ON member.role_principal_id = rol.principal_id + JOIN sys.server_principals pri ON member.member_principal_id = pri.principal_id + WHERE rol.type_desc = 'SERVER_ROLE' + AND rol.name = 'sysadmin' + AND pri.name = '#{@login_under_test}'" run_sql_query(host, run_sql_query_opts_as_sa(query, expected_row_count = 1)) end it "should have the specified serveradmin role" do - query = "SELECT 'is_serveradmin' AS result WHERE IS_SRVROLEMEMBER('serveradmin','#{@login_under_test}') = 1" + # Note - IS_SRVROLEMEMBER always returns false for a disabled WINDOWS_LOGIN user login + query = "SELECT pri.name from sys.server_role_members member + JOIN sys.server_principals rol ON member.role_principal_id = rol.principal_id + JOIN sys.server_principals pri ON member.member_principal_id = pri.principal_id + WHERE rol.type_desc = 'SERVER_ROLE' + AND rol.name = 'serveradmin' + AND pri.name = '#{@login_under_test}'" run_sql_query(host, run_sql_query_opts_as_sa(query, expected_row_count = 1)) end diff --git a/templates/create/login.sql.erb b/templates/create/login.sql.erb index c29f2291..58a3a842 100644 --- a/templates/create/login.sql.erb +++ b/templates/create/login.sql.erb @@ -42,7 +42,12 @@ ALTER LOGIN [<%= @login %>] WITH DEFAULT_DATABASE = [<%= @default_database %>]; <% @svrroles.each do |role, enable_bit| -%> -IF IS_SRVROLEMEMBER('<%= role %>','<%= @login %>') != <%= enable_bit %> +IF (SELECT COUNT(me.role_principal_id) from sys.server_role_members me + JOIN sys.server_principals rol ON me.role_principal_id = rol.principal_id + JOIN sys.server_principals pri ON me.member_principal_id = pri.principal_id + WHERE rol.type_desc = 'SERVER_ROLE' + AND rol.name = '<%= role %>' + AND pri.name = '<%= @login %>') != <%= enable_bit %> BEGIN <% if enable_bit == '1' || enable_bit == 1 -%> ALTER SERVER ROLE [<%= role %>] ADD MEMBER [<%= @login %>];