winrm: source user from options than remote_user (#40467) (#40516)

* winrm: source user from options than remote_user (#40467)

* winrm: source user from options than remote_user

* fixed up mock for kerberos import

* Added changelog fragment

* get hostname from option as well

(cherry picked from commit 1ac180c74d)

* Also removed uneeded method that wasn't backported

changelogs/fragments/winrm-get_correct_user.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- winrm - allow `ansible_user` or `ansible_winrm_user` to override `ansible_ssh_user` when both are defined in an inventory - https://github.com/ansible/ansible/issues/39844

lib/ansible/plugins/connection/winrm.py

@@ -100,7 +100,6 @@ import base64
 import inspect
 import os
 import re
-import shlex
 import traceback
 import json
 import tempfile
@@ -188,8 +187,8 @@ class Connection(ConnectionBase):
 
         super(Connection, self).set_options(task_keys=None, var_options=var_options, direct=direct)
 
-        self._winrm_host = self._play_context.remote_addr
+        self._winrm_host = self.get_option('remote_addr')
-        self._winrm_user = self._play_context.remote_user
+        self._winrm_user = self.get_option('remote_user')
         self._winrm_pass = self._play_context.password
 
         self._become_method = self._play_context.become_method
@@ -235,7 +234,7 @@ class Connection(ConnectionBase):
         kinit_mode = self.get_option('kerberos_mode')
         if kinit_mode is None:
             # HACK: ideally, remove multi-transport stuff
-            self._kerb_managed = "kerberos" in self._winrm_transport and self._winrm_pass
+            self._kerb_managed = "kerberos" in self._winrm_transport and (self._winrm_pass is not None and self._winrm_pass != "")
         elif kinit_mode == "managed":
             self._kerb_managed = True
         elif kinit_mode == "manual":
@@ -505,47 +504,6 @@ class Connection(ConnectionBase):
 
         return (result.status_code, result.std_out, result.std_err)
 
-    def exec_command_old(self, cmd, in_data=None, sudoable=True):
-        super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)
-        cmd_parts = shlex.split(to_bytes(cmd), posix=False)
-        cmd_parts = map(to_text, cmd_parts)
-        script = None
-        cmd_ext = cmd_parts and self._shell._unquote(cmd_parts[0]).lower()[-4:] or ''
-        # Support running .ps1 files (via script/raw).
-        if cmd_ext == '.ps1':
-            script = '& %s' % cmd
-        # Support running .bat/.cmd files; change back to the default system encoding instead of UTF-8.
-        elif cmd_ext in ('.bat', '.cmd'):
-            script = '[System.Console]::OutputEncoding = [System.Text.Encoding]::Default; & %s' % cmd
-        # Encode the command if not already encoded; supports running simple PowerShell commands via raw.
-        elif '-EncodedCommand' not in cmd_parts:
-            script = cmd
-        if script:
-            cmd_parts = self._shell._encode_script(script, as_list=True, strict_mode=False)
-        if '-EncodedCommand' in cmd_parts:
-            encoded_cmd = cmd_parts[cmd_parts.index('-EncodedCommand') + 1]
-            decoded_cmd = to_text(base64.b64decode(encoded_cmd).decode('utf-16-le'))
-            display.vvv("EXEC %s" % decoded_cmd, host=self._winrm_host)
-        else:
-            display.vvv("EXEC %s" % cmd, host=self._winrm_host)
-        try:
-            result = self._winrm_exec(cmd_parts[0], cmd_parts[1:], from_exec=True)
-        except Exception:
-            traceback.print_exc()
-            raise AnsibleConnectionFailure("failed to exec cmd %s" % to_native(cmd))
-        result.std_out = to_bytes(result.std_out)
-        result.std_err = to_bytes(result.std_err)
-
-        # parse just stderr from CLIXML output
-        if self.is_clixml(result.std_err):
-            try:
-                result.std_err = self.parse_clixml_stream(result.std_err)
-            except Exception:
-                # unsure if we're guaranteed a valid xml doc- use raw output in case of error
-                pass
-
-        return (result.status_code, result.std_out, result.std_err)
-
     def is_clixml(self, value):
         return value.startswith(b"#< CLIXML")
 

test/units/plugins/connection/test_winrm.py

@@ -0,0 +1,221 @@
+# -*- coding: utf-8 -*-
+# (c) 2018, Jordan Borean <jborean@redhat.com>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import sys
+
+import pytest
+
+from io import StringIO
+
+from ansible.compat.tests.mock import patch, MagicMock
+from ansible.playbook.play_context import PlayContext
+from ansible.plugins.loader import connection_loader
+
+
+class TestConnectionWinRM(object):
+
+    OPTIONS_DATA = (
+        # default options
+        (
+            {},
+            {'_extras': {}},
+            {},
+            {
+                '_kerb_managed': False,
+                '_kinit_cmd': 'kinit',
+                '_winrm_connection_timeout': None,
+                '_winrm_host': 'inventory_hostname',
+                '_winrm_kwargs': {'username': None, 'password': ''},
+                '_winrm_pass': '',
+                '_winrm_path': '/wsman',
+                '_winrm_port': 5986,
+                '_winrm_scheme': 'https',
+                '_winrm_transport': ['ssl'],
+                '_winrm_user': None
+            },
+            False
+        ),
+        # http through port
+        (
+            {},
+            {'_extras': {}, 'ansible_port': 5985},
+            {},
+            {
+                '_winrm_kwargs': {'username': None, 'password': ''},
+                '_winrm_port': 5985,
+                '_winrm_scheme': 'http',
+                '_winrm_transport': ['plaintext'],
+            },
+            False
+        ),
+        # kerberos user with kerb present
+        (
+            {},
+            {'_extras': {}, 'ansible_user': 'user@domain.com'},
+            {},
+            {
+                '_kerb_managed': False,
+                '_kinit_cmd': 'kinit',
+                '_winrm_kwargs': {'username': 'user@domain.com',
+                                  'password': ''},
+                '_winrm_pass': '',
+                '_winrm_transport': ['kerberos', 'ssl'],
+                '_winrm_user': 'user@domain.com'
+            },
+            True
+        ),
+        # kerberos user without kerb present
+        (
+            {},
+            {'_extras': {}, 'ansible_user': 'user@domain.com'},
+            {},
+            {
+                '_kerb_managed': False,
+                '_kinit_cmd': 'kinit',
+                '_winrm_kwargs': {'username': 'user@domain.com',
+                                  'password': ''},
+                '_winrm_pass': '',
+                '_winrm_transport': ['ssl'],
+                '_winrm_user': 'user@domain.com'
+            },
+            False
+        ),
+        # kerberos user with managed ticket (implicit)
+        (
+            {'password': 'pass'},
+            {'_extras': {}, 'ansible_user': 'user@domain.com'},
+            {},
+            {
+                '_kerb_managed': True,
+                '_kinit_cmd': 'kinit',
+                '_winrm_kwargs': {'username': 'user@domain.com',
+                                  'password': 'pass'},
+                '_winrm_pass': 'pass',
+                '_winrm_transport': ['kerberos', 'ssl'],
+                '_winrm_user': 'user@domain.com'
+            },
+            True
+        ),
+        # kerb with managed ticket (explicit)
+        (
+            {'password': 'pass'},
+            {'_extras': {}, 'ansible_user': 'user@domain.com',
+             'ansible_winrm_kinit_mode': 'managed'},
+            {},
+            {
+                '_kerb_managed': True,
+            },
+            True
+        ),
+        # kerb with unmanaged ticket (explicit))
+        (
+            {'password': 'pass'},
+            {'_extras': {}, 'ansible_user': 'user@domain.com',
+             'ansible_winrm_kinit_mode': 'manual'},
+            {},
+            {
+                '_kerb_managed': False,
+            },
+            True
+        ),
+        # transport override (single)
+        (
+            {},
+            {'_extras': {}, 'ansible_user': 'user@domain.com',
+             'ansible_winrm_transport': 'ntlm'},
+            {},
+            {
+                '_winrm_kwargs': {'username': 'user@domain.com',
+                                  'password': ''},
+                '_winrm_pass': '',
+                '_winrm_transport': ['ntlm'],
+            },
+            False
+        ),
+        # transport override (list)
+        (
+            {},
+            {'_extras': {}, 'ansible_user': 'user@domain.com',
+             'ansible_winrm_transport': ['ntlm', 'certificate']},
+            {},
+            {
+                '_winrm_kwargs': {'username': 'user@domain.com',
+                                  'password': ''},
+                '_winrm_pass': '',
+                '_winrm_transport': ['ntlm', 'certificate'],
+            },
+            False
+        ),
+        # winrm extras
+        (
+            {},
+            {'_extras': {'ansible_winrm_server_cert_validation': 'ignore',
+                         'ansible_winrm_service': 'WSMAN'}},
+            {},
+            {
+                '_winrm_kwargs': {'username': None, 'password': '',
+                                  'server_cert_validation': 'ignore',
+                                  'service': 'WSMAN'},
+            },
+            False
+        ),
+        # direct override
+        (
+            {},
+            {'_extras': {}, 'ansible_winrm_connection_timeout': 5},
+            {'connection_timeout': 10},
+            {
+                '_winrm_connection_timeout': 10,
+            },
+            False
+        ),
+        # user comes from option not play context
+        (
+            {'username': 'user1'},
+            {'_extras': {}, 'ansible_user': 'user2'},
+            {},
+            {
+                '_winrm_user': 'user2',
+                '_winrm_kwargs': {'username': 'user2', 'password': ''}
+            },
+            False
+        )
+    )
+
+    # pylint bug: https://github.com/PyCQA/pylint/issues/511
+    # pylint: disable=undefined-variable
+    @pytest.mark.parametrize('play, options, direct, expected, kerb',
+                             ((p, o, d, e, k) for p, o, d, e, k in OPTIONS_DATA))
+    def test_set_options(self, play, options, direct, expected, kerb):
+        if kerb:
+            kerberos_mock = MagicMock()
+            modules = {'kerberos': kerberos_mock}
+        else:
+            modules = {'kerberos': None}
+
+        module_patcher = patch.dict(sys.modules, modules)
+        module_patcher.start()
+
+        pc = PlayContext()
+        for attr, value in play.items():
+            setattr(pc, attr, value)
+
+        new_stdin = StringIO()
+
+        # ensure we get a fresh connection plugin by clearing the cache
+        connection_loader._module_cache = {}
+        conn = connection_loader.get('winrm', pc, new_stdin)
+        conn.set_options(var_options=options, direct=direct)
+
+        for attr, expected in expected.items():
+            actual = getattr(conn, attr)
+            assert actual == expected, \
+                "winrm attr '%s', actual '%s' != expected '%s'"\
+                % (attr, actual, expected)
+
+        module_patcher.stop()