Fixing some parsing issues in authorized_key module

Also adds an integration test for authorized_key for future validation.

Fixes #6700

library/system/authorized_key

@@ -199,33 +199,19 @@ def parseoptions(module, options):
     '''
     options_dict = keydict() #ordered dict
     if options:
-        token_exp = [
-            # matches separator
-            (r',+', False),
-            # matches option with value, e.g. from="x,y"
-            (r'([a-z0-9-]+)="((?:[^"\\]|\\.)*)"', True),
-            # matches single option, e.g. no-agent-forwarding 
-            (r'[a-z0-9-]+', True)
-        ]
-
-        pos = 0
-        while pos < len(options):
-            match = None
-            for pattern, is_valid_option in token_exp:
-                regex = re.compile(pattern, re.IGNORECASE)
-                match = regex.match(options, pos)
-                if match:
-                    text = match.group(0)
-                    if is_valid_option:
-                        if len(match.groups()) == 2:
-                            options_dict[match.group(1)] = match.group(2)
-                        else:
-                            options_dict[text] = None
-                    break
-            if not match:
-                module.fail_json(msg="invalid option string: %s" % options)
-            else:
-                pos = match.end(0)
+        try:
+            # the following regex will split on commas while
+            # ignoring those commas that fall within quotes
+            regex = re.compile(r'''((?:[^,"']|"[^"]*"|'[^']*')+)''')
+            parts = regex.split(options)[1:-1]
+            for part in parts:
+                if "=" in part:
+                    (key, value) = part.split("=", 1)
+                    options_dict[key] = value
+                elif part != ",":
+                    options_dict[part] = None
+        except:
+            module.fail_json(msg="invalid option string: %s" % options)
 
     return options_dict
 
@@ -254,7 +240,7 @@ def parsekey(module, raw_key):
 
     # split key safely
     lex = shlex.shlex(raw_key)
-    lex.quotes = ["'", '"']
+    lex.quotes = []
     lex.commenters = '' #keep comment hashes
     lex.whitespace_split = True
     key_parts = list(lex)
@@ -315,7 +301,7 @@ def writekeys(module, filename, keys):
                     option_strings = []
                     for option_key in options.keys():
                         if options[option_key]:
-                            option_strings.append("%s=\"%s\"" % (option_key, options[option_key]))
+                            option_strings.append("%s=%s" % (option_key, options[option_key]))
                         else:
                             option_strings.append("%s" % option_key)
 

test/integration/non_destructive.yml

@@ -36,3 +36,4 @@
     - { role: test_command_shell, tags: test_command_shell }
     - { role: test_failed_when, tags: test_failed_when }
     - { role: test_script, tags: test_script }
+    - { role: test_authorized_key, tags: test_authorized_key }

test/integration/roles/test_authorized_key/defaults/main.yml

@@ -0,0 +1,15 @@
+---
+dss_key_basic: >
+  ssh-dss DATA_BASIC root@testing
+dss_key_unquoted_option: >
+  idle-timeout=5m ssh-dss DATA_UNQUOTED_OPTION root@testing
+dss_key_command: >
+  command="/bin/true" ssh-dss DATA_COMMAND root@testing
+dss_key_complex_command: >
+  command="echo foo 'bar baz'" ssh-dss DATA_COMPLEX_COMMAND root@testing
+dss_key_command_single_option: >
+  no-port-forwarding,command="/bin/true" ssh-dss DATA_COMMAND_SINGLE_OPTIONS root@testing
+dss_key_command_multiple_options: >
+  no-port-forwarding,idle-timeout=5m,command="/bin/true" ssh-dss DATA_COMMAND_MULTIPLE_OPTIONS root@testing
+dss_key_trailing: >
+  ssh-dss DATA_TRAILING root@testing foo bar baz

test/integration/roles/test_authorized_key/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies: 
+  - prepare_tests

test/integration/roles/test_authorized_key/tasks/main.yml

@@ -0,0 +1,244 @@
+# test code for the authorized_key module
+# (c) 2014, James Cammarata <jcammarata@ansible.com>
+
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+
+# -------------------------------------------------------------
+# Setup steps
+
+- name: touch the authorized_keys file
+  file: dest="{{output_dir}}/authorized_keys" state=touch
+  register: result
+
+- name: assert that the authorized_keys file was created
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.state == "file"']
+
+# -------------------------------------------------------------
+# basic ssh-dss key
+
+- name: add basic ssh-dss key
+  authorized_key: user=root key="{{ dss_key_basic }}" state=present path="{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_basic']
+    - ['result.key_options == None']
+
+- name: re-add basic ssh-dss key
+  authorized_key: user=root key="{{ dss_key_basic }}" state=present path="{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+
+# -------------------------------------------------------------
+# ssh-dss key with an unquoted option
+
+- name: add ssh-dss key with an unquoted option
+  authorized_key: 
+    user: root
+    key: "{{ dss_key_unquoted_option }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_unquoted_option']
+    - ['result.key_options == None']
+
+- name: re-add ssh-dss key with an unquoted option
+  authorized_key:
+    user: root
+    key: "{{ dss_key_unquoted_option }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+
+# -------------------------------------------------------------
+# ssh-dss key with a leading command="/bin/foo"
+
+- name: add ssh-dss key with a leading command
+  authorized_key: 
+    user: root
+    key: "{{ dss_key_command }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_command']
+    - ['result.key_options == None']
+
+- name: re-add ssh-dss key with a leading command
+  authorized_key:
+    user: root
+    key: "{{ dss_key_command }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+
+# -------------------------------------------------------------
+# ssh-dss key with a complex quoted leading command
+# ie. command="/bin/echo foo 'bar baz'"
+
+- name: add ssh-dss key with a complex quoted leading command
+  authorized_key:
+    user: root
+    key: "{{ dss_key_complex_command }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_complex_command']
+    - ['result.key_options == None']
+
+- name: re-add ssh-dss key with a complex quoted leading command
+  authorized_key:
+    user: root
+    key: "{{ dss_key_complex_command }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+
+# -------------------------------------------------------------
+# ssh-dss key with a command and a single option, which are
+# in a comma-separated list
+
+- name: add ssh-dss key with a command and a single option
+  authorized_key:
+    user: root
+    key: "{{ dss_key_command_single_option }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_command_single_option']
+    - ['result.key_options == None']
+
+- name: re-add ssh-dss key with a command and a single option
+  authorized_key:
+    user: root
+    key: "{{ dss_key_command_single_option }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+
+# -------------------------------------------------------------
+# ssh-dss key with a command and multiple other options
+
+- name: add ssh-dss key with a command and multiple options
+  authorized_key:
+    user: root
+    key: "{{ dss_key_command_multiple_options }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_command_multiple_options']
+    - ['result.key_options == None']
+
+- name: re-add ssh-dss key with a command and multiple options
+  authorized_key:
+    user: root
+    key: "{{ dss_key_command_multiple_options }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+
+# -------------------------------------------------------------
+# ssh-dss key with multiple trailing parts, which are space-
+# separated and not quoted in any way
+
+- name: add ssh-dss key with trailing parts
+  authorized_key:
+    user: root
+    key: "{{ dss_key_trailing }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that the key was added
+  assert:
+    that:
+    - ['result.changed == True']
+    - ['result.key == dss_key_trailing']
+    - ['result.key_options == None']
+
+- name: re-add ssh-dss key with trailing parts
+  authorized_key:
+    user: root
+    key: "{{ dss_key_trailing }}"
+    state: present
+    path: "{{output_dir|expanduser}}/authorized_keys"
+  register: result
+
+- name: assert that nothing changed
+  assert:
+    that:
+    - ['result.changed == False']
+