Copilot commented on code in PR #13955:
URL: https://github.com/apache/cloudstack/pull/13955#discussion_r4523774101


##########
ui/src/views/iam/AddAccount.vue:
##########
@@ -408,12 +422,16 @@ export default {
         const params = {
           roleid: values.roleid,
           username: values.username,
-          password: values.password,
           email: values.email,
           firstname: values.firstname,
           lastname: values.lastname,
           domainid: values.domainid
         }
+        if (!values.samlenable) {
+          // SAML-authenticated accounts never log in with a native password; 
let the API
+          // generate one rather than asking the admin to set one that will 
never be used
+          params.password = values.password
+        }

Review Comment:
   The UI logic toggles behavior based on `values.samlenable`, but `samlenable` 
is not included in `params`. If the backend relies on a `samlenable` flag to 
mark the account as SAML-enabled, this can create an account that is *not* 
SAML-enabled while also omitting the password (causing an unknown autogenerated 
password). Include the appropriate API parameter (e.g., `params.samlenable = 
values.samlenable`) when building `params`, if supported by `createAccount`.



##########
api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java:
##########
@@ -75,8 +77,8 @@ public class CreateAccountCmd extends BaseCmd {
 
     @Parameter(name = ApiConstants.PASSWORD,
                type = CommandType.STRING,
-               required = true,
-               description = "Clear text password (Default hashed to 
SHA256SALT). If you wish to use any other hashing algorithm, you would need to 
write a custom authentication adapter See Docs section.")
+               description = "Clear text password (Default hashed to 
SHA256SALT). If you wish to use any other hashing algorithm, you will need to 
write a custom authentication adapter. See Docs section."
+                       + "If omitted, a random password is generated, e.g. for 
an account that will only ever authenticate externally via SAML/LDAP.")

Review Comment:
   The concatenated description string is missing a space between sentences, 
resulting in `section.If omitted...` in generated API docs/help. Add a leading 
space to the second string (or include the space at the end of the first) so 
the description renders correctly.



##########
ui/src/views/iam/AddAccount.vue:
##########
@@ -250,6 +256,14 @@ export default {
         }
       },
       immediate: false
+    },
+    'form.samlenable' (samlEnabled) {
+      // a SAML-authenticated account never logs in with a native password
+      this.rules.password = samlEnabled ? [] : [{ required: true, message: 
this.$t('message.error.required.input') }]
+      this.rules.confirmpassword = samlEnabled ? [] : [
+        { required: true, message: this.$t('message.error.required.input') },
+        { validator: this.validateConfirmPassword }
+      ]
     }

Review Comment:
   This watcher won’t run on initial render, so `rules.password` / 
`rules.confirmpassword` may remain in the wrong state until `form.samlenable` 
changes. If `form.samlenable` can be pre-set (e.g., from defaults or prefilled 
data), the form can end up requiring hidden fields. Convert this watcher to the 
object form with `immediate: true` (or initialize the rules from the initial 
`form.samlenable` value in `data()/created()`).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to