Repository: ansible/ansible
Pull request: add UpdateUserName and UpdatePasswordPolicy
##### SUMMARY
add UpdateUserName in redfish_command module to update existed user account's UserName.
add UpdatePasswordPolicy in redfish_command module to set account password policy under AccountService.

Fixes #62937

##### ISSUE TYPE
- Feature Pull Request

##### COMPONENT NAME
redfish_command

##### ADDITIONAL INFORMATION
none

Problem statement:
redfish_command - Accounts - UpdateUserName / UpdatePasswordPolicy
#### SUMMARY
This feature would implement a UpdateUserName command for the Accounts category of redfish_command, to update user name. And also implement a UpdatePasswordPolicy command for the Accounts category of redfish_command to change password policy.

##### ISSUE TYPE
- Feature Idea

##### COMPONENT NAME
redfish_command

##### ADDITIONAL INFORMATION
These 2 commands would help user to update user name or update password policy

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/remote_management/redfish/redfish_command.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/remote_management/redfish/redfish_command.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @billdodd @jose-delarosa @mraineri @tomasg2012
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: fix error detected by ansible-test

Patch to review:
diff --git a/lib/ansible/module_utils/redfish_utils.py b/lib/ansible/module_utils/redfish_utils.py
index 35eda9aa2d5735..1ade50024400ff 100644
--- a/lib/ansible/module_utils/redfish_utils.py
+++ b/lib/ansible/module_utils/redfish_utils.py
@@ -987,6 +987,66 @@ def update_user_password(self, user):
             return response
         return {'ret': True}
 
+    def update_user_name(self, user):
+        if not user.get('account_updatename'):
+            return {'ret': False, 'msg':
+                    'Must provide account_updatename for UpdateUserName command'}
+
+        response = self._find_account_uri(username=user.get('account_username'),
+                                          acct_id=user.get('account_id'))
+        if not response['ret']:
+            return response
+        uri = response['uri']
+        payload = {'UserName': user['account_updatename']}
+        response = self.patch_request(self.root_uri + uri, payload)
+        if response['ret'] is False:
+            return response
+        return {'ret': True}
+
+    def update_password_policy(self, user):
+        if user.get('account_policy_name') is None or user.get('account_policy_value') is None:
+            return {'ret': False, 'msg':
+                    'Must provide account_policy_name and account_policy_value for UpdatePasswordPolicy command'}
+
+        policy_name = user.get('account_policy_name')
+        policy_value = user.get('account_policy_value')
+
+        # Find AccountService
+        response = self.get_request(self.root_uri + self.service_root)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if 'AccountService' not in data:
+            return {'ret': False, 'msg': "AccountService resource not found"}
+        accountservice_uri = data["AccountService"]["@odata.id"]
+
+        # Check support or not
+        response = self.get_request(self.root_uri + accountservice_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if policy_name not in data:
+            return {'ret': False, 'msg':
+                    'account_policy_name %s not support' % policy_name}
+
+        # if value is already matched, nothing to do
+        property_value = data.get(policy_name)
+        new_value = None
+        if isinstance(property_value, int):
+            new_value = int(policy_value)
+        elif isinstance(property_value, bool):
+            new_value = bool(policy_value)
+        else:
+            new_value = policy_value
+        if property_value == new_value:
+            return {'ret': True, 'changed': False}
+
+        payload = {policy_name: new_value}
+        response = self.patch_request(self.root_uri + self.service_root + 'AccountService', payload)
+        if response['ret'] is False:
+            return response
+        return {'ret': True}
+
     def get_sessions(self):
         result = {}
         # listing all users has always been slower than other operations, why?
diff --git a/lib/ansible/modules/remote_management/redfish/redfish_command.py b/lib/ansible/modules/remote_management/redfish/redfish_command.py
index f16e486edb0e26..4610a1bec52360 100644
--- a/lib/ansible/modules/remote_management/redfish/redfish_command.py
+++ b/lib/ansible/modules/remote_management/redfish/redfish_command.py
@@ -100,6 +100,25 @@
       - BootNext target when bootdevice is "UefiBootNext"
     type: str
     version_added: "2.9"
+  update_username:
+    required: false
+    aliases: [ account_updatename ]
+    description:
+      - new update user name for account_username
+    type: str
+    version_added: "2.10"
+  account_policy_name:
+    required: false
+    description:
+      - name of password policy to update
+    type: str
+    version_added: "2.10"
+  account_policy_value:
+    required: false
+    description:
+      - value of password policy to update
+    type: str
+    version_added: "2.10"
 
 author: "Jose Delarosa (@jose-delarosa)"
 '''
@@ -230,6 +249,36 @@
       account_username: "{{ account_username }}"
       roleid: "{{ roleid }}"
 
+  - name: Update user name
+    redfish_command:
+      category: Accounts
+      command: UpdateUserName
+      baseuri: "{{ baseuri }}"
+      username: "{{ username }}"
+      password: "{{ password }}"
+      account_username: "{{ account_username }}"
+      account_updatename: "{{ account_updatename }}"
+
+  - name: Update user name
+    redfish_command:
+      category: Accounts
+      command: UpdateUserName
+      baseuri: "{{ baseuri }}"
+      username: "{{ username }}"
+      password: "{{ password }}"
+      account_username: "{{ account_username }}"
+      update_username: "{{ update_username }}"
+
+  - name: Update password policy
+    redfish_command:
+      category: Accounts
+      command: UpdatePasswordPolicy
+      baseuri: "{{ baseuri }}"
+      username: "{{ username }}"
+      password: "{{ password }}"
+      account_policy_name: AccountLockoutThreshold
+      account_policy_value: 5
+
   - name: Clear Manager Logs with a timeout of 20 seconds
     redfish_command:
       category: Manager
@@ -259,7 +308,8 @@
                 "PowerGracefulShutdown", "PowerReboot", "SetOneTimeBoot"],
     "Chassis": ["IndicatorLedOn", "IndicatorLedOff", "IndicatorLedBlink"],
     "Accounts": ["AddUser", "EnableUser", "DeleteUser", "DisableUser",
-                 "UpdateUserRole", "UpdateUserPassword"],
+                 "UpdateUserRole", "UpdateUserPassword", "UpdateUserName",
+                 "UpdatePasswordPolicy"],
     "Manager": ["GracefulRestart", "ClearLogs"],
 }
 
@@ -277,6 +327,9 @@ def main():
             new_username=dict(aliases=["account_username"]),
             new_password=dict(aliases=["account_password"], no_log=True),
             roleid=dict(aliases=["account_roleid"]),
+            update_username=dict(type='str', aliases=["account_updatename"]),
+            account_policy_name=dict(),
+            account_policy_value=dict(),
             bootdevice=dict(),
             timeout=dict(type='int', default=10),
             uefi_target=dict(),
@@ -296,7 +349,10 @@ def main():
     user = {'account_id': module.params['id'],
             'account_username': module.params['new_username'],
             'account_password': module.params['new_password'],
-            'account_roleid': module.params['roleid']}
+            'account_roleid': module.params['roleid'],
+            'account_updatename': module.params['update_username'],
+            'account_policy_name': module.params['account_policy_name'],
+            'account_policy_value': module.params['account_policy_value']}
 
     # timeout
     timeout = module.params['timeout']
@@ -323,7 +379,9 @@ def main():
             "DeleteUser": rf_utils.delete_user,
             "DisableUser": rf_utils.disable_user,
             "UpdateUserRole": rf_utils.update_user_role,
-            "UpdateUserPassword": rf_utils.update_user_password
+            "UpdateUserPassword": rf_utils.update_user_password,
+            "UpdateUserName": rf_utils.update_user_name,
+            "UpdatePasswordPolicy": rf_utils.update_password_policy
         }
 
         # execute only if we find an Account service resource


Code review comments:
diff_hunk: @@ -259,7 +308,8 @@
                 "PowerGracefulShutdown", "PowerReboot", "SetOneTimeBoot"],
     "Chassis": ["IndicatorLedOn", "IndicatorLedOff", "IndicatorLedBlink"],
     "Accounts": ["AddUser", "EnableUser", "DeleteUser", "DisableUser",
-                 "UpdateUserRole", "UpdateUserPassword"],
+                 "UpdateUserRole", "UpdateUserPassword", "UpdateUserName",
+                 "UpdatePasswordPolicy"],
path: lib/ansible/modules/remote_management/redfish/redfish_command.py
text: @user1:
For the command name, since there are several AccountService properties that users may want to set and not all of them may be considered password policies, I think it would be better to use a more general name. Something like `UpdateAccountServiceProperties`.

@author:
Good suggestion. I have updated command name to UpdateAccountServiceProperties.

diff_hunk: @@ -987,6 +987,66 @@ def update_user_password(self, user):
             return response
         return {'ret': True}
 
+    def update_user_name(self, user):
+        if not user.get('account_updatename'):
+            return {'ret': False, 'msg':
+                    'Must provide account_updatename for UpdateUserName command'}
+
+        response = self._find_account_uri(username=user.get('account_username'),
+                                          acct_id=user.get('account_id'))
+        if not response['ret']:
+            return response
+        uri = response['uri']
+        payload = {'UserName': user['account_updatename']}
+        response = self.patch_request(self.root_uri + uri, payload)
+        if response['ret'] is False:
+            return response
+        return {'ret': True}
+
+    def update_password_policy(self, user):
+        if user.get('account_policy_name') is None or user.get('account_policy_value') is None:
+            return {'ret': False, 'msg':
+                    'Must provide account_policy_name and account_policy_value for UpdatePasswordPolicy command'}
+
+        policy_name = user.get('account_policy_name')
+        policy_value = user.get('account_policy_value')
+
+        # Find AccountService
+        response = self.get_request(self.root_uri + self.service_root)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if 'AccountService' not in data:
+            return {'ret': False, 'msg': "AccountService resource not found"}
+        accountservice_uri = data["AccountService"]["@odata.id"]
+
+        # Check support or not
+        response = self.get_request(self.root_uri + accountservice_uri)
+        if response['ret'] is False:
+            return response
+        data = response['data']
+        if policy_name not in data:
+            return {'ret': False, 'msg':
+                    'account_policy_name %s not support' % policy_name}
+
+        # if value is already matched, nothing to do
+        property_value = data.get(policy_name)
+        new_value = None
+        if isinstance(property_value, int):
+            new_value = int(policy_value)
+        elif isinstance(property_value, bool):
+            new_value = bool(policy_value)
+        else:
+            new_value = policy_value
+        if property_value == new_value:
+            return {'ret': True, 'changed': False}
+
+        payload = {policy_name: new_value}
+        response = self.patch_request(self.root_uri + self.service_root + 'AccountService', payload)
path: lib/ansible/module_utils/redfish_utils.py
text: @user1:
```suggestion
        response = self.patch_request(self.root_uri + self.accounts_uri, payload)
```

@author:
accounts_uri is /redfish/v1/AccountService/Accounts, not target uri. I have updated uri to self.root_uri + accountservice_uri

@user1:
Ah, yes. My mistake. We should probably update _find_accountservice_resource() to save the AccountService URI. But that can be done in a separate PR.

diff_hunk: @@ -296,7 +349,10 @@ def main():
     user = {'account_id': module.params['id'],
             'account_username': module.params['new_username'],
             'account_password': module.params['new_password'],
-            'account_roleid': module.params['roleid']}
+            'account_roleid': module.params['roleid'],
+            'account_updatename': module.params['update_username'],
+            'account_policy_name': module.params['account_policy_name'],
+            'account_policy_value': module.params['account_policy_value']}
path: lib/ansible/modules/remote_management/redfish/redfish_command.py
text: @user1:
See previous naming comment. Suggest something like:
```
        'account_properties': module.params['account_properties']
```

@author:
Suggestion has been adopted in code.

diff_hunk: @@ -277,6 +327,9 @@ def main():
             new_username=dict(aliases=["account_username"]),
             new_password=dict(aliases=["account_password"], no_log=True),
             roleid=dict(aliases=["account_roleid"]),
+            update_username=dict(type='str', aliases=["account_updatename"]),
+            account_policy_name=dict(),
+            account_policy_value=dict(),
path: lib/ansible/modules/remote_management/redfish/redfish_command.py
text: @user1:
Allowing a dictionary of properties here would make a cleaner and more usable interface, allowing the user to set multiple properties at one time:

```
        account_properties=dict(type='dict', default={}),
```

This would allow plays that look something like:

```
  - name: Update account properties
    redfish_command:
      category: Accounts
      command: UpdateAccountServiceProperties
      baseuri: "{{ baseuri }}"
      username: "{{ username }}"
      password: "{{ password }}"
      account_properties:
        AccountLockoutThreshold: 5
        AccountLockoutDuration: 600
```

@author:
Suggestion has been adopted in code. @user1 Can you check the code again?

@user1:
Will do. Thanks for the updates.