Skip to content

Commit

Permalink
T6988: fix: remove role/level, fix tests (#371)
Browse files Browse the repository at this point in the history
* T6988: fix: remove role/level, fix tests

* feature: add support for SSH keys

* tests: add integration tests for public_keys

* feat: add encrypted password support

* tests: add unit for encrypted

* tests: fix wrapping in YAML

* tests: fix smoke tests
  • Loading branch information
gaige authored Jan 2, 2025
1 parent dbd87e3 commit 9e15999
Show file tree
Hide file tree
Showing 11 changed files with 769 additions and 87 deletions.
15 changes: 15 additions & 0 deletions changelogs/fragments/T6988-fix-user.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
breaking_changes:
- removal of role/level as it was removed in 1.3

major_changes:
- add support for public-key authentication
- add support for encrypted password specification

minor_changes:
- fix sending of `full-name` to use quotes
- fix parsing of `full-name` to ignore quotes
- fix integration tests for smoke

known_issues:
- ssh login tests are brittle
191 changes: 172 additions & 19 deletions docs/vyos.vyos.vyos_user_module.rst

Large diffs are not rendered by default.

195 changes: 158 additions & 37 deletions plugins/modules/vyos_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@
- The C(full_name) argument provides the full name of the user account to be created
on the remote device. This argument accepts any text string value.
type: str
encrypted_password:
description:
- The encrypted password of the user account on the remote device. Note that unlike
the C(configured_password) argument, this argument ignores the C(update_password)
and updates if the value is different from the one in the device running config.
type: str
configured_password:
description:
- The password to be configured on the VyOS device. The password needs to be provided
Expand All @@ -77,13 +83,6 @@
choices:
- on_create
- always
level:
description:
- The C(level) argument configures the level of the user when logged into the
system. This argument accepts string values admin or operator.
type: str
aliases:
- role
state:
description:
- Configures the state of the username definition as it relates to the device
Expand All @@ -94,6 +93,32 @@
choices:
- present
- absent
public_keys: &public_keys
description:
- Public keys for authentiction over SSH.
type: list
elements: dict
suboptions:
name:
description: Name of the key (usually in the form of user@hostname)
required: true
type: str
key:
description: Public key string (base64 encoded)
required: true
type: str
type:
description: Type of the key
required: true
type: str
choices:
- ssh-dss
- ssh-rsa
- ecdsa-sha2-nistp256
- ecdsa-sha2-nistp384
- ssh-ed25519
- ecdsa-sha2-nistp521
name:
description:
- The username to be configured on the VyOS device. This argument accepts a string
Expand All @@ -104,6 +129,12 @@
- The C(full_name) argument provides the full name of the user account to be created
on the remote device. This argument accepts any text string value.
type: str
encrypted_password:
description:
- The encrypted password of the user account on the remote device. Note that unlike
the C(configured_password) argument, this argument ignores the C(update_password)
and updates if the value is different from the one in the device running config.
type: str
configured_password:
description:
- The password to be configured on the VyOS device. The password needs to be provided
Expand All @@ -120,13 +151,7 @@
choices:
- on_create
- always
level:
description:
- The C(level) argument configures the level of the user when logged into the
system. This argument accepts string values admin or operator.
type: str
aliases:
- role
public_keys: *public_keys
purge:
description:
- Instructs the module to consider the resource definition absolute. It will remove
Expand Down Expand Up @@ -161,7 +186,6 @@
aggregate:
- name: netop
- name: netend
level: operator
state: present
- name: Change Password for User netop
vyos.vyos.vyos_user:
Expand All @@ -177,7 +201,6 @@
returned: always
type: list
sample:
- set system login user test level operator
- set system login user authentication plaintext-password password
"""

Expand All @@ -198,11 +221,6 @@
)


def validate_level(value, module):
if value not in ("admin", "operator"):
module.fail_json(msg="level must be either admin or operator, got %s" % value)


def spec_to_commands(updates, module):
commands = list()
update_password = module.params["update_password"]
Expand All @@ -220,11 +238,39 @@ def add(command, want, x):
commands.append("delete system login user %s" % want["name"])
continue

if needs_update(want, have, "level"):
add(commands, want, "level %s" % want["level"])

if needs_update(want, have, "full_name"):
add(commands, want, "full-name %s" % want["full_name"])
add(commands, want, "full-name '%s'" % want["full_name"])

# look both ways for public_keys to handle replacement
want_keys = want.get("public_keys") or dict()
have_keys = have.get("public_keys") or dict()
for key_name in want_keys:
key = want_keys[key_name]
if key_name not in have_keys or key != have_keys[key_name]:
add(
commands,
want,
"authentication public-keys %s key '%s'" % (key["name"], key["key"]),
)
add(
commands,
want,
"authentication public-keys %s type '%s'" % (key["name"], key["type"]),
)

for key_name in have_keys:
if key_name not in want_keys:
commands.append(
"delete system login user %s authentication public-keys %s"
% (want["name"], key_name),
)

if needs_update(want, have, "encrypted_password"):
add(
commands,
want,
"authentication encrypted-password '%s'" % want["encrypted_password"],
)

if needs_update(want, have, "configured_password"):
if update_password == "always" or not have:
Expand All @@ -237,20 +283,57 @@ def add(command, want, x):
return commands


def parse_level(data):
match = re.search(r"level (\S+)", data, re.M)
if match:
level = match.group(1)[1:-1]
return level


def parse_full_name(data):
match = re.search(r"full-name (\S+)", data, re.M)
match = re.search(r"full-name '(\S+)'", data, re.M)
if match:
full_name = match.group(1)[1:-1]
return full_name


def parse_key(data):
match = re.search(r"key '(\S+)'", data, re.M)
if match:
key = match.group(1)
return key


def parse_key_type(data):
match = re.search(r"type '(\S+)'", data, re.M)
if match:
key_type = match.group(1)
return key_type


def parse_public_keys(data):
"""
Parse public keys from the configuration
returning dictionary of dictionaries indexed by key name
"""
match = re.findall(r"public-keys (\S+)", data, re.M)
if not match:
return dict()

keys = dict()
for key in set(match):
regex = r" %s .+$" % key
cfg = re.findall(regex, data, re.M)
cfg = "\n".join(cfg)
obj = {
"name": key,
"key": parse_key(cfg),
"type": parse_key_type(cfg),
}
keys[key] = obj
return keys


def parse_encrypted_password(data):
match = re.search(r"authentication encrypted-password '(\S+)'", data, re.M)
if match:
encrypted_password = match.group(1)
return encrypted_password


def config_to_dict(module):
data = get_config(module)

Expand All @@ -268,8 +351,9 @@ def config_to_dict(module):
"name": user,
"state": "present",
"configured_password": None,
"level": parse_level(cfg),
"full_name": parse_full_name(cfg),
"encrypted_password": parse_encrypted_password(cfg),
"public_keys": parse_public_keys(cfg),
}
instances.append(obj)

Expand All @@ -289,6 +373,21 @@ def get_param_value(key, item, module):
return value


def map_key_params_to_dict(keys):
"""
Map the list of keys to a dictionary of dictionaries
indexed by key name
"""
all_keys = dict()
if keys is None:
return all_keys

for key in keys:
key_name = key["name"]
all_keys[key_name] = key
return all_keys


def map_params_to_obj(module):
aggregate = module.params["aggregate"]
if not aggregate:
Expand All @@ -309,9 +408,10 @@ def map_params_to_obj(module):
for item in users:
get_value = partial(get_param_value, item=item, module=module)
item["configured_password"] = get_value("configured_password")
item["encrypted_password"] = get_value("encrypted_password")
item["full_name"] = get_value("full_name")
item["level"] = get_value("level")
item["state"] = get_value("state")
item["public_keys"] = map_key_params_to_dict(get_value("public_keys"))
objects.append(item)

return objects
Expand All @@ -332,13 +432,30 @@ def update_objects(want, have):

def main():
"""main entry point for module execution"""
public_key_spec = dict(
name=dict(required=True, type="str"),
key=dict(required=True, type="str", no_log=False),
type=dict(
required=True,
type="str",
choices=[
"ssh-dss",
"ssh-rsa",
"ecdsa-sha2-nistp256",
"ecdsa-sha2-nistp384",
"ssh-ed25519",
"ecdsa-sha2-nistp521",
],
),
)
element_spec = dict(
name=dict(),
full_name=dict(),
level=dict(aliases=["role"]),
configured_password=dict(no_log=True),
encrypted_password=dict(no_log=False),
update_password=dict(default="always", choices=["on_create", "always"]),
state=dict(default="present", choices=["present", "absent"]),
public_keys=dict(type="list", elements="dict", options=public_key_spec),
)

aggregate_spec = deepcopy(element_spec)
Expand All @@ -359,7 +476,11 @@ def main():

argument_spec.update(element_spec)

mutually_exclusive = [("name", "aggregate")]
mutually_exclusive = [
("name", "aggregate"),
("encrypted_password", "configured_password"),
]

module = AnsibleModule(
argument_spec=argument_spec,
mutually_exclusive=mutually_exclusive,
Expand Down
8 changes: 2 additions & 6 deletions tests/integration/targets/vyos_smoke/tests/cli/caching.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@
interface_cmds:
- set interfaces ethernet eth1 description 'Configured by Ansible - Interface 1'
- set interfaces ethernet eth1 mtu '1500'
- set interfaces ethernet eth1 duplex 'auto'
- set interfaces ethernet eth1 speed 'auto'
- set interfaces ethernet eth1 vif 101 description 'Eth1 - VIF 101'
- set interfaces ethernet eth2 description 'Configured by Ansible - Interface 2 (ADMIN DOWN)'
- set interfaces ethernet eth2 mtu '600'
- set interfaces ethernet eth2 mtu '1280'
l3_interface_cmds:
- set interfaces ethernet eth1 address '192.0.2.10/24'
- set interfaces ethernet eth1 address '2001:db8::10/32'
Expand All @@ -31,15 +29,13 @@
- name: eth1
description: Configured by Ansible - Interface 1
mtu: 1500
speed: auto
duplex: auto
vifs:
- vlan_id: 101
description: Eth1 - VIF 101

- name: eth2
description: Configured by Ansible - Interface 2 (ADMIN DOWN)
mtu: 600
mtu: 1280
state: merged

- assert:
Expand Down
13 changes: 6 additions & 7 deletions tests/integration/targets/vyos_user/tests/cli/auth.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,24 @@
- name: Create user with password
vyos.vyos.vyos_user:
name: auth_user
role: admin
state: present
configured_password: pass123

- name: test login via ssh with new user
expect:
command: >-
ssh auth_user@{{ ansible_ssh_host }} -p {{ ansible_port | default(22) }} \
-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no '/opt/vyatta/sbin/vyatta-cfg-cmd-wrapper
show version'
ssh auth_user@{{ ansible_ssh_host }} -p {{ ansible_port | default(22) }}
-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no
'/opt/vyatta/sbin/vyatta-cfg-cmd-wrapper show version'
responses:
(?i)password: pass123

- name: test login via ssh with invalid password (should fail)
expect:
command: >-
ssh auth_user@{{ ansible_ssh_host }} -p {{ ansible_port | default(22) }} \
-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no '/opt/vyatta/sbin/vyatta-cfg-cmd-wrapper
show version'
ssh auth_user@{{ ansible_ssh_host }} -p {{ ansible_port | default(22) }}
-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no
'/opt/vyatta/sbin/vyatta-cfg-cmd-wrapper show version'
responses:
(?i)password: badpass
ignore_errors: true
Expand Down
Loading

0 comments on commit 9e15999

Please sign in to comment.