Repository navigation
make password an optional parameter/field in createAccount - #13955
DaanHoogland wants to merge 5 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13955 +/- ##
=========================================
Coverage 20.78% 20.78%
- Complexity 21928 21932 +4
=========================================
Files 6496 6496
Lines 591657 591663 +6
Branches 72592 72596 +4
=========================================
+ Hits 122956 122971 +15
+ Misses 455356 455348 -8
+ Partials 13345 13344 -1
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 | 25.89% |
| Branch coverage | 20.23% |
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
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.
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19184 |
…unt/CreateAccountCmd.java Co-authored-by: GaOrtiga <[email protected]>
c5f8b6a to
340145e
Compare
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.
Copilot review overview
6 open issues
form.samlenableis initially true, but this watcher won’t run… · Newform.samlenableis initially true, but this watcher won’t run… · Newpasswordchanges the API semantics for all… · Newpasswordchanges the API semantics for all… · Newpasswordchanges the API semantics for all… · NewINTERNAL_ERRORthat occurs only… · New
4 resolved since last review
form.samlenableis initially true (e.g.,…assertTrue(..., true)always passes), and the test…12is a magic number here. Prefer referencing an existing configured…
Review effort: Lite
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.
Copilot review overview
2 open issues
6 resolved since last review
form.samlenableis initially true, but this watcher won’t run…form.samlenableis initially true, but this watcher won’t run…INTERNAL_ERRORthat occurs only…passwordchanges the API semantics for all…passwordchanges the API semantics for all…passwordchanges the API semantics for all…
Review effort: Lite
| '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 } | ||
| ] | ||
| } |
| 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. " |
|
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?