make password an optional parameter/field in createAccount - #13955
make password an optional parameter/field in createAccount#13955DaanHoogland wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13955 +/- ##
============================================
+ Coverage 19.74% 19.80% +0.06%
- Complexity 19960 20022 +62
============================================
Files 6371 6371
Lines 575784 575958 +174
Branches 70478 70524 +46
============================================
+ Hits 113665 114049 +384
+ Misses 449765 449474 -291
- Partials 12354 12435 +81
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🔴 Test Coverage Grade:
|
| Metric | Value |
|---|---|
| Line coverage | 24.69% |
| Branch coverage | 18.90% |
Grade Scale
| Grade | Line Coverage | Meaning |
|---|---|---|
| 🟢 A | ≥ 80% | Excellent - this code sleeps well at night 😴 |
| 🟡 B | 60-79% | Good - almost there, don't stop now 😉 |
| 🟠 C | 40-59% | Acceptable - your code is wearing a seatbelt, but no airbags 😬 |
| 🔴 D | 20-39% | Marginal - boldly shipping where no test has gone before 🖖 |
| ⛔ F | < 20% | Failing - tests? what tests? 🔥 |
Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run
…unt/CreateAccountCmd.java Co-authored-by: GaOrtiga <49285692+GaOrtiga@users.noreply.github.com>
86b71c7 to
7fba665
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18980 |
@GaOrtiga did you happen to test? |
|
@blueorangutan test |
vishesh92
left a comment
vishesh92
left a comment
There was a problem hiding this comment.
just one comment. rest looks good.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Updates account creation to allow omitting a password (notably for externally authenticated accounts like SAML/LDAP), and reflects that behavior in the UI and API validation.
Changes:
- UI: Hide password inputs when SAML is enabled, show informational message, and relax validation rules accordingly.
- API: Make
passwordoptional forcreateAccountand auto-generate a random password when absent/blank. - Tests: Update
CreateAccountCmdTestexpectations around null/empty passwords.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ui/src/views/iam/AddAccount.vue | Conditionally hides password fields for SAML and updates client-side validation + request params. |
| ui/public/locales/en.json | Adds i18n string explaining that no password is needed for SAML accounts. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java | Makes password optional and generates a random password when blank. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java | Adjusts unit tests for the new password generation behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (StringUtils.isBlank(getPassword())) { | ||
| password = PasswordGenerator.generateRandomPassword(12); | ||
| } |
| '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 } | ||
| ] |
| if(StringUtils.isEmpty(getPassword())) { | ||
| throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed"); | ||
| if (StringUtils.isBlank(getPassword())) { | ||
| password = PasswordGenerator.generateRandomPassword(12); |
| try { | ||
| createAccountCmd.execute(); | ||
| Assert.fail("should throw exception for a null password"); | ||
| } catch (ServerApiException e) { | ||
| Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); | ||
| Assert.assertEquals("Empty passwords are not allowed", e.getMessage()); | ||
| Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true); | ||
| } | ||
| Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd); | ||
| Assert.assertNotNull("a password should be generated for accounts that authenticate externally", createAccountCmd.getPassword()); | ||
| Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd); |
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19184 |
Description
This PR...
Fixes: #12039
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?