Skip to content

Commit 2518cd1

Browse files
committed
(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.
1 parent 200b862 commit 2518cd1

File tree

2 files changed

+20
-3
lines changed

2 files changed

+20
-3
lines changed

spec/acceptance/sqlserver_login_spec.rb

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,11 +288,23 @@ def create_login_manifest (testcase,login_name,login_password,options = {})
288288
end
289289

290290
it "should have the specified sysadmin role" do
291-
query = "SELECT 'is_sysadmin' AS result WHERE IS_SRVROLEMEMBER('sysadmin','#{@login_under_test}') = 1"
291+
# Note - IS_SRVROLEMEMBER always returns false for a disabled WINDOWS_LOGIN user login
292+
query = "SELECT pri.name from sys.server_role_members member
293+
JOIN sys.server_principals rol ON member.role_principal_id = rol.principal_id
294+
JOIN sys.server_principals pri ON member.member_principal_id = pri.principal_id
295+
WHERE rol.type_desc = 'SERVER_ROLE'
296+
AND rol.name = 'sysadmin'
297+
AND pri.name = '#{@login_under_test}'"
292298
run_sql_query(host, run_sql_query_opts_as_sa(query, expected_row_count = 1))
293299
end
294300
it "should have the specified serveradmin role" do
295-
query = "SELECT 'is_serveradmin' AS result WHERE IS_SRVROLEMEMBER('serveradmin','#{@login_under_test}') = 1"
301+
# Note - IS_SRVROLEMEMBER always returns false for a disabled WINDOWS_LOGIN user login
302+
query = "SELECT pri.name from sys.server_role_members member
303+
JOIN sys.server_principals rol ON member.role_principal_id = rol.principal_id
304+
JOIN sys.server_principals pri ON member.member_principal_id = pri.principal_id
305+
WHERE rol.type_desc = 'SERVER_ROLE'
306+
AND rol.name = 'serveradmin'
307+
AND pri.name = '#{@login_under_test}'"
296308
run_sql_query(host, run_sql_query_opts_as_sa(query, expected_row_count = 1))
297309
end
298310

templates/create/login.sql.erb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ ALTER LOGIN [<%= @login %>] WITH
4242
DEFAULT_DATABASE = [<%= @default_database %>];
4343

4444
<% @svrroles.each do |role, enable_bit| -%>
45-
IF IS_SRVROLEMEMBER('<%= role %>','<%= @login %>') != <%= enable_bit %>
45+
IF (SELECT COUNT(me.role_principal_id) from sys.server_role_members me
46+
JOIN sys.server_principals rol ON me.role_principal_id = rol.principal_id
47+
JOIN sys.server_principals pri ON me.member_principal_id = pri.principal_id
48+
WHERE rol.type_desc = 'SERVER_ROLE'
49+
AND rol.name = '<%= role %>'
50+
AND pri.name = '<%= @login %>') != <%= enable_bit %>
4651
BEGIN
4752
<% if enable_bit == '1' || enable_bit == 1 -%>
4853
ALTER SERVER ROLE [<%= role %>] ADD MEMBER [<%= @login %>];

0 commit comments

Comments
 (0)