From 2a9ec6bdbf58c830680cd961f89972acf99ec1e8 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Thu, 25 Jan 2018 02:56:58 -0800 Subject: [PATCH] fix Windows tests with hardcoded Administrator account (#35339) * Admin account is not always called Administrator (eg Azure) * this fixes some, but not all issues related to the Administrator account on non-English Windows as well (still numerous references to "Administrators" and other en-US Windows group names) --- .../win_group_membership/tasks/tests.yml | 32 +++++---- .../library/sid_utils_test.ps1 | 25 +++++-- .../win_scheduled_task_stat/tasks/main.yml | 12 +++- .../targets/win_shortcut/tasks/tests.yml | 20 ++++-- .../targets/win_user_right/tasks/tests.yml | 72 ++++++++++--------- 5 files changed, 103 insertions(+), 58 deletions(-) diff --git a/test/integration/targets/win_group_membership/tasks/tests.yml b/test/integration/targets/win_group_membership/tasks/tests.yml index b3d15bd3c3..f0f2729b5d 100644 --- a/test/integration/targets/win_group_membership/tasks/tests.yml +++ b/test/integration/targets/win_group_membership/tasks/tests.yml @@ -17,11 +17,19 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . +- name: Look up built-in Administrator account name (-500 user whose domain == computer name) + raw: $machine_sid = (Get-CimInstance Win32_UserAccount -Filter "Domain='$env:COMPUTERNAME'")[0].SID -replace '(S-1-5-21-\d+-\d+-\d+)-\d+', '$1'; (Get-CimInstance Win32_UserAccount -Filter "SID='$machine_sid-500'").Name + check_mode: no + register: admin_account_result + +- set_fact: + admin_account_name: "{{ admin_account_result.stdout_lines[0] }}" + - name: Remove potentially leftover group members win_group_membership: name: "{{ win_local_group }}" members: - - Administrator + - "{{ admin_account_name }}" - Guest - NT AUTHORITY\SYSTEM - NT AUTHORITY\NETWORK SERVICE @@ -32,7 +40,7 @@ win_group_membership: name: FakeGroup members: - - Administrator + - "{{ admin_account_name }}" state: present register: add_user_to_fake_group failed_when: add_user_to_fake_group.changed != false or add_user_to_fake_group.msg != "Could not find local group FakeGroup" @@ -62,7 +70,7 @@ win_group_membership: &wgm_present name: "{{ win_local_group }}" members: - - Administrator + - "{{ admin_account_name }}" - Guest - NT AUTHORITY\SYSTEM state: present @@ -72,8 +80,8 @@ assert: that: - add_users_to_group.changed == true - - add_users_to_group.added == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM"] - - add_users_to_group.members == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM"] + - add_users_to_group.added == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM"] + - add_users_to_group.members == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM"] when: not in_check_mode - name: Test add_users_to_group (check-mode) @@ -94,7 +102,7 @@ that: - add_users_to_group_again.changed == false - add_users_to_group_again.added == [] - - add_users_to_group_again.members == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM"] + - add_users_to_group_again.members == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM"] when: not in_check_mode @@ -102,7 +110,7 @@ win_group_membership: <<: *wgm_present members: - - "{{ ansible_hostname }}\\Administrator" + - '{{ ansible_hostname }}\{{ admin_account_name }}' - .\Guest register: add_different_syntax_users_to_group_again @@ -111,7 +119,7 @@ that: - add_different_syntax_users_to_group_again.changed == false - add_different_syntax_users_to_group_again.added == [] - - add_different_syntax_users_to_group_again.members == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM"] + - add_different_syntax_users_to_group_again.members == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM"] when: not in_check_mode - name: Test add_different_syntax_users_to_group_again (check-mode) @@ -135,7 +143,7 @@ that: - add_another_user_to_group.changed == true - add_another_user_to_group.added == ["NT AUTHORITY\\NETWORK SERVICE"] - - add_another_user_to_group.members == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM", "NT AUTHORITY\\NETWORK SERVICE"] + - add_another_user_to_group.members == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM", "NT AUTHORITY\\NETWORK SERVICE"] when: not in_check_mode - name: Test add_another_user_to_group (check-mode) @@ -156,7 +164,7 @@ that: - add_another_user_to_group_again.changed == false - add_another_user_to_group_again.added == [] - - add_another_user_to_group_again.members == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM", "NT AUTHORITY\\NETWORK SERVICE"] + - add_another_user_to_group_again.members == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM", "NT AUTHORITY\\NETWORK SERVICE"] when: not in_check_mode @@ -170,7 +178,7 @@ assert: that: - remove_users_from_group.changed == true - - remove_users_from_group.removed == ["Administrator", "Guest", "NT AUTHORITY\\SYSTEM"] + - remove_users_from_group.removed == [admin_account_name, "Guest", "NT AUTHORITY\\SYSTEM"] - remove_users_from_group.members == ["NT AUTHORITY\\NETWORK SERVICE"] when: not in_check_mode @@ -200,7 +208,7 @@ win_group_membership: <<: *wgm_absent members: - - "{{ ansible_hostname }}\\Administrator" + - '{{ ansible_hostname }}\{{ admin_account_name }}' - .\Guest register: remove_different_syntax_users_from_group_again diff --git a/test/integration/targets/win_module_utils/library/sid_utils_test.ps1 b/test/integration/targets/win_module_utils/library/sid_utils_test.ps1 index 1b9bffd2f7..e64f5cf454 100644 --- a/test/integration/targets/win_module_utils/library/sid_utils_test.ps1 +++ b/test/integration/targets/win_module_utils/library/sid_utils_test.ps1 @@ -10,14 +10,24 @@ Function Assert-Equals($actual, $expected) { } Function Get-ComputerSID() { - # this is sort off cheating but I can't see any better way of getting this SID - $admin_sid = Convert-ToSID -account_name "$env:COMPUTERNAME\Administrator" + # find any local user and trim off the final UID + $luser_sid = (Get-CimInstance Win32_UserAccount -Filter "Domain='$env:COMPUTERNAME'")[0].SID - return $admin_sid.Substring(0, $admin_sid.Length - 4) + return $luser_sid -replace '(S-1-5-21-\d+-\d+-\d+)-\d+', '$1' } $local_sid = Get-ComputerSID +# most machines should have a -500 Administrator account, but it may have been renamed. Look it up by SID +$default_admin = Get-CimInstance Win32_UserAccount -Filter "SID='$local_sid-500'" + +# this group is called Administrators by default on English Windows, but could named something else. Look it up by SID +$default_admin_group = Get-CimInstance Win32_Group -Filter "SID='S-1-5-32-544'" + +if (@($default_admin).Length -ne 1) { + Fail-Json @{} "could not find a local admin account with SID ending in -500" +} + ### Set this to the NETBIOS name of the domain you wish to test, not set for shippable ### $test_domain = $null @@ -26,10 +36,10 @@ $tests = @( @{ sid = "S-1-1-0"; full_name = "Everyone"; names = @("Everyone") }, @{ sid = "S-1-5-18"; full_name = "NT AUTHORITY\SYSTEM"; names = @("NT AUTHORITY\SYSTEM", "SYSTEM") }, @{ sid = "S-1-5-20"; full_name = "NT AUTHORITY\NETWORK SERVICE"; names = @("NT AUTHORITY\NETWORK SERVICE", "NETWORK SERVICE") }, - @{ sid = "$local_sid-500"; full_name = "$env:COMPUTERNAME\Administrator"; names = @("$env:COMPUTERNAME\Administrator", "Administrator", ".\Administrator") }, + @{ sid = "$($default_admin.SID)"; full_name = "$($default_admin.FullName)"; names = @("$env:COMPUTERNAME\$($default_admin.Name)", "$($default_admin.Name)", ".\$($default_admin.Name)") }, # Local Groups - @{ sid = "S-1-5-32-544"; full_name = "BUILTIN\Administrators"; names = @("BUILTIN\Administrators", "Administrators", ".\Administrators") } + @{ sid = "$($default_admin_group.SID)"; full_name = "BUILTIN\$($default_admin_group.Name)"; names = @("BUILTIN\$($default_admin_group.Name)", "$($default_admin_group.Name)", ".\$($default_admin_group.Name)") } ) # Add domain tests if the domain name has been set @@ -55,7 +65,10 @@ if ($test_domain -ne $null) { foreach ($test in $tests) { $actual_account_name = Convert-FromSID -sid $test.sid - Assert-Equals -actual $actual_account_name -expected $test.full_name + # renamed admins may have an empty FullName; skip comparison in that case + if ($test.full_name) { + Assert-Equals -actual $actual_account_name -expected $test.full_name + } foreach ($test_name in $test.names) { $actual_sid = Convert-ToSID -account_name $test_name diff --git a/test/integration/targets/win_scheduled_task_stat/tasks/main.yml b/test/integration/targets/win_scheduled_task_stat/tasks/main.yml index fe0b02084f..a9bda9b81e 100644 --- a/test/integration/targets/win_scheduled_task_stat/tasks/main.yml +++ b/test/integration/targets/win_scheduled_task_stat/tasks/main.yml @@ -1,4 +1,12 @@ --- +- name: Look up built-in Administrator account name (-500 user whose domain == computer name) + raw: $machine_sid = (Get-CimInstance Win32_UserAccount -Filter "Domain='$env:COMPUTERNAME'")[0].SID -replace '(S-1-5-21-\d+-\d+-\d+)-\d+', '$1'; (Get-CimInstance Win32_UserAccount -Filter "SID='$machine_sid-500'").Name + check_mode: no + register: admin_account_result + +- set_fact: + admin_account_name: "{{ admin_account_result.stdout_lines[0] }}" + - name: ensure task is deleted before test win_scheduled_task: name: '{{test_scheduled_task_stat_name}}' @@ -34,7 +42,7 @@ name: '{{test_scheduled_task_stat_name}}' state: present logon_type: interactive_token - username: Administrator + username: '{{ admin_account_name }}' author: Ansible Author description: Fake description execution_time_limit: PT23H @@ -102,7 +110,7 @@ - stat_task_present.principal.group_id == None - stat_task_present.principal.logon_type == "TASK_LOGON_INTERACTIVE_TOKEN" - stat_task_present.principal.run_level == "TASK_RUNLEVEL_LUA" - - stat_task_present.principal.user_id.endswith("Administrator") + - stat_task_present.principal.user_id.endswith(admin_account_name) - stat_task_present.registration_info.author == "Ansible Author" - stat_task_present.registration_info.date is defined - stat_task_present.registration_info.description == "Fake description" diff --git a/test/integration/targets/win_shortcut/tasks/tests.yml b/test/integration/targets/win_shortcut/tasks/tests.yml index c0534d933f..61809b9e5b 100644 --- a/test/integration/targets/win_shortcut/tasks/tests.yml +++ b/test/integration/targets/win_shortcut/tasks/tests.yml @@ -16,6 +16,14 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . +- name: get current user profile location + raw: $env:USERPROFILE + check_mode: no + register: profile_result + +- set_fact: + profile_dir: '{{ profile_result.stdout_lines[0] }}' + - name: Add Ansible website link on the desktop win_shortcut: src: https://ansible.com/ @@ -27,7 +35,7 @@ assert: that: - ansible_website_link_add.changed == true - - ansible_website_link_add.dest == 'C:\\Users\\Administrator\\Desktop\\Ansible website.url' + - ansible_website_link_add.dest == profile_dir + '\Desktop\Ansible website.url' - ansible_website_link_add.src == 'https://ansible.com/' - name: Add Ansible website link on the desktop again @@ -41,7 +49,7 @@ assert: that: - ansible_website_link_add_again.changed == false - - ansible_website_link_add_again.dest == 'C:\\Users\\Administrator\\Desktop\\Ansible website.url' + - ansible_website_link_add_again.dest == profile_dir + '\Desktop\Ansible website.url' - ansible_website_link_add_again.src == 'https://ansible.com/' when: not in_check_mode @@ -49,7 +57,7 @@ assert: that: - ansible_website_link_add_again.changed == true - - ansible_website_link_add_again.dest == 'C:\\Users\\Administrator\\Desktop\\Ansible website.url' + - ansible_website_link_add_again.dest == profile_dir + '\Desktop\Ansible website.url' - ansible_website_link_add_again.src == 'https://ansible.com/' when: in_check_mode @@ -63,14 +71,14 @@ assert: that: - ansible_website_link_remove.changed == true - - ansible_website_link_remove.dest == 'C:\\Users\\Administrator\\Desktop\\Ansible website.url' + - ansible_website_link_remove.dest == profile_dir + '\Desktop\Ansible website.url' when: not in_check_mode - name: Check there was no change (check-mode) assert: that: - ansible_website_link_remove.changed == false - - ansible_website_link_remove.dest == 'C:\\Users\\Administrator\\Desktop\\Ansible website.url' + - ansible_website_link_remove.dest == profile_dir + '\Desktop\Ansible website.url' when: in_check_mode - name: Remove link again @@ -83,7 +91,7 @@ assert: that: - ansible_website_link_remove_again.changed == false - - ansible_website_link_remove_again.dest == 'C:\\Users\\Administrator\\Desktop\\Ansible website.url' + - ansible_website_link_remove_again.dest == profile_dir + '\Desktop\Ansible website.url' - name: Add a regedit shortcut on the desktop win_shortcut: diff --git a/test/integration/targets/win_user_right/tasks/tests.yml b/test/integration/targets/win_user_right/tasks/tests.yml index c7b65c6399..7d12048082 100644 --- a/test/integration/targets/win_user_right/tasks/tests.yml +++ b/test/integration/targets/win_user_right/tasks/tests.yml @@ -1,8 +1,16 @@ --- +- name: Look up built-in Administrator account name (-500 user whose domain == computer name) + raw: $machine_sid = (Get-CimInstance Win32_UserAccount -Filter "Domain='$env:COMPUTERNAME'")[0].SID -replace '(S-1-5-21-\d+-\d+-\d+)-\d+', '$1'; (Get-CimInstance Win32_UserAccount -Filter "SID='$machine_sid-500'").Name + check_mode: no + register: admin_account_result + +- set_fact: + admin_account_name: "{{ admin_account_result.stdout_lines[0] }}" + - name: fail to set invalid right win_user_right: name: FailRight - users: Administrator + users: '{{ admin_account_name }}' register: fail_invalid_right failed_when: fail_invalid_right.msg != 'the specified right FailRight is not a valid right' @@ -16,7 +24,7 @@ - name: remove from empty right check win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Administrators'] + users: ['{{ admin_account_name }}', 'Administrators'] action: remove register: remove_empty_right_check check_mode: yes @@ -31,7 +39,7 @@ - name: remove from empty right win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Administrators'] + users: ['{{ admin_account_name }}', 'Administrators'] action: remove register: remove_empty_right check_mode: yes @@ -46,7 +54,7 @@ - name: set administrator check win_user_right: name: '{{test_win_user_right_name}}' - users: Administrator + users: '{{ admin_account_name }}' action: set register: set_administrator_check check_mode: yes @@ -60,14 +68,14 @@ assert: that: - set_administrator_check is changed - - set_administrator_check.added == ["{{ansible_hostname}}\\Administrator"] + - set_administrator_check.added == ['{{ansible_hostname}}\{{ admin_account_name }}'] - set_administrator_check.removed == [] - set_administrator_actual_check.users == [] - name: set administrator win_user_right: name: '{{test_win_user_right_name}}' - users: Administrator + users: '{{ admin_account_name }}' action: set register: set_administrator @@ -80,14 +88,14 @@ assert: that: - set_administrator is changed - - set_administrator.added == ["{{ansible_hostname}}\\Administrator"] + - set_administrator.added == ['{{ansible_hostname}}\{{ admin_account_name }}'] - set_administrator.removed == [] - - set_administrator_actual.users == ['Administrator'] + - set_administrator_actual.users == ['{{ admin_account_name }}'] - name: set administrator again win_user_right: name: '{{test_win_user_right_name}}' - users: Administrator + users: '{{ admin_account_name }}' action: set register: set_administrator_again @@ -101,7 +109,7 @@ - name: remove from right check win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Guests', '{{ansible_hostname}}\Users', '.\Backup Operators'] + users: ['{{ admin_account_name }}', 'Guests', '{{ansible_hostname}}\Users', '.\Backup Operators'] action: remove register: remove_right_check check_mode: yes @@ -115,14 +123,14 @@ assert: that: - remove_right_check is changed - - remove_right_check.removed == ["{{ansible_hostname}}\\Administrator"] + - remove_right_check.removed == ['{{ansible_hostname}}\{{ admin_account_name }}'] - remove_right_check.added == [] - - remove_right_actual_check.users == ['Administrator'] + - remove_right_actual_check.users == ['{{ admin_account_name }}'] - name: remove from right win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Guests', '{{ansible_hostname}}\Users', '.\Backup Operators'] + users: ['{{ admin_account_name }}', 'Guests', '{{ansible_hostname}}\Users', '.\Backup Operators'] action: remove register: remove_right @@ -135,14 +143,14 @@ assert: that: - remove_right is changed - - remove_right.removed == ["{{ansible_hostname}}\\Administrator"] + - remove_right.removed == ['{{ansible_hostname}}\{{ admin_account_name }}'] - remove_right.added == [] - remove_right_actual.users == [] - name: remove from right again win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Guests', '{{ansible_hostname}}\Users', '.\Backup Operators'] + users: ['{{ admin_account_name }}', 'Guests', '{{ansible_hostname}}\Users', '.\Backup Operators'] action: remove register: remove_right_again @@ -156,7 +164,7 @@ - name: add to empty right check win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Administrators'] + users: ['{{ admin_account_name }}', 'Administrators'] action: add register: add_right_on_empty_check check_mode: yes @@ -171,13 +179,13 @@ that: - add_right_on_empty_check is changed - add_right_on_empty_check.removed == [] - - add_right_on_empty_check.added == ["{{ansible_hostname}}\\Administrator", "BUILTIN\\Administrators"] + - add_right_on_empty_check.added == ['{{ansible_hostname}}\{{ admin_account_name }}', "BUILTIN\\Administrators"] - add_right_on_empty_actual_check.users == [] - name: add to empty right win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Administrators'] + users: ['{{ admin_account_name }}', 'Administrators'] action: add register: add_right_on_empty @@ -191,13 +199,13 @@ that: - add_right_on_empty is changed - add_right_on_empty.removed == [] - - add_right_on_empty.added == ["{{ansible_hostname}}\\Administrator", "BUILTIN\\Administrators"] - - add_right_on_empty_actual.users == ["Administrator", "BUILTIN\\Administrators"] + - add_right_on_empty.added == ["{{ansible_hostname}}\\{{ admin_account_name }}", "BUILTIN\\Administrators"] + - add_right_on_empty_actual.users == ["{{ admin_account_name }}", "BUILTIN\\Administrators"] - name: add to empty right again win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Administrators'] + users: ['{{ admin_account_name }}', 'Administrators'] action: add register: add_right_on_empty_again @@ -211,7 +219,7 @@ - name: add to existing right check win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Guests', '{{ansible_hostname}}\Users'] + users: ['{{ admin_account_name }}', 'Guests', '{{ansible_hostname}}\Users'] action: add register: add_right_on_existing_check check_mode: yes @@ -227,12 +235,12 @@ - add_right_on_existing_check is changed - add_right_on_existing_check.removed == [] - add_right_on_existing_check.added == ["BUILTIN\\Guests", "BUILTIN\\Users"] - - add_right_on_existing_actual_check.users == ["Administrator", "BUILTIN\\Administrators"] + - add_right_on_existing_actual_check.users == ["{{ admin_account_name }}", "BUILTIN\\Administrators"] - name: add to existing right win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Guests', '{{ansible_hostname}}\Users'] + users: ['{{ admin_account_name }}', 'Guests', '{{ansible_hostname}}\Users'] action: add register: add_right_on_existing @@ -247,12 +255,12 @@ - add_right_on_existing is changed - add_right_on_existing.removed == [] - add_right_on_existing.added == ["BUILTIN\\Guests", "BUILTIN\\Users"] - - add_right_on_existing_actual.users == ["Administrator", "BUILTIN\\Administrators", "BUILTIN\\Users", "BUILTIN\\Guests"] + - add_right_on_existing_actual.users == ["{{ admin_account_name }}", "BUILTIN\\Administrators", "BUILTIN\\Users", "BUILTIN\\Guests"] - name: add to existing right again win_user_right: name: '{{test_win_user_right_name}}' - users: ['Administrator', 'Guests', '{{ansible_hostname}}\Users'] + users: ['{{ admin_account_name }}', 'Guests', '{{ansible_hostname}}\Users'] action: add register: add_right_on_existing_again @@ -266,7 +274,7 @@ - name: remove from existing check win_user_right: name: '{{test_win_user_right_name}}' - users: ['Guests', 'Administrator'] + users: ['Guests', '{{ admin_account_name }}'] action: remove register: remove_on_existing_check check_mode: yes @@ -280,14 +288,14 @@ assert: that: - remove_on_existing_check is changed - - remove_on_existing_check.removed == ["BUILTIN\\Guests", "{{ansible_hostname}}\\Administrator"] + - remove_on_existing_check.removed == ["BUILTIN\\Guests", "{{ansible_hostname}}\\{{ admin_account_name }}"] - remove_on_existing_check.added == [] - - remove_on_existing_actual_check.users == ["Administrator", "BUILTIN\\Administrators", "BUILTIN\\Users", "BUILTIN\\Guests"] + - remove_on_existing_actual_check.users == ["{{ admin_account_name }}", "BUILTIN\\Administrators", "BUILTIN\\Users", "BUILTIN\\Guests"] - name: remove from existing win_user_right: name: '{{test_win_user_right_name}}' - users: ['Guests', 'Administrator'] + users: ['Guests', '{{ admin_account_name }}'] action: remove register: remove_on_existing @@ -300,14 +308,14 @@ assert: that: - remove_on_existing is changed - - remove_on_existing.removed == ["BUILTIN\\Guests", "{{ansible_hostname}}\\Administrator"] + - remove_on_existing.removed == ["BUILTIN\\Guests", "{{ansible_hostname}}\\{{ admin_account_name }}"] - remove_on_existing.added == [] - remove_on_existing_actual.users == ["BUILTIN\\Administrators", "BUILTIN\\Users"] - name: remove from existing again win_user_right: name: '{{test_win_user_right_name}}' - users: ['Guests', 'Administrator'] + users: ['Guests', '{{ admin_account_name }}'] action: remove register: remove_on_existing_again