avoid rejecting user login if listLdapConfiguration is not allowed#13152
avoid rejecting user login if listLdapConfiguration is not allowed#13152sudo87 wants to merge 4 commits into
Conversation
|
@sureshanaparti a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the UI login/feature-discovery flow so that a user without permission to call listLdapConfigurations is not blocked from logging in, while still keeping the UI’s LDAP-enabled flag updated when LDAP config changes.
Changes:
- Stop rejecting the
GetInfoflow whenlistLdapConfigurationsfails (e.g., due to missing API permission). - Rename the Vuex action used for refreshing the LDAP-enabled flag from
UpdateConfigurationtoUpdateLdapConfigurationFlag. - Update
AutogenViewto dispatch the renamed action after LDAP configuration add/delete actions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ui/src/views/AutogenView.vue | Dispatches the renamed Vuex action after LDAP config mutations. |
| ui/src/store/modules/user.js | Makes LDAP config listing non-fatal during GetInfo; renames the LDAP refresh action. |
Comments suppressed due to low confidence (1)
ui/src/store/modules/user.js:561
UpdateLdapConfigurationFlagwraps the API call in anew Promise(...)but never callsresolve(...)on success, so any caller awaitingdispatch('UpdateLdapConfigurationFlag')will hang indefinitely. Either callresolve(ldapEnable)inside thethen(...)(and possiblyresolve(false)on handled errors), or simplify by returning theapi('listLdapConfigurations')promise chain directly.
UpdateLdapConfigurationFlag ({ commit }) {
return new Promise((resolve, reject) => {
api('listLdapConfigurations').then(response => {
const ldapEnable = (response.ldapconfigurationresponse.count > 0)
commit('SET_LDAP', ldapEnable)
}).catch(error => {
reject(error)
})
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13152 +/- ##
============================================
- Coverage 16.27% 16.26% -0.01%
+ Complexity 13440 13431 -9
============================================
Files 5665 5665
Lines 500555 500556 +1
Branches 60789 60789
============================================
- Hits 81445 81396 -49
- Misses 410004 410053 +49
- Partials 9106 9107 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
UI build: ✔️ |
|
I opened a stacked follow-up PR against this PR's source branch with the review hardening from the reproduced 4.22 issue: shapeblue#136 It keeps |
|
Update: I pushed the follow-up hardening directly into this PR branch now that I confirmed write access to The PR now includes commit
The temporary stacked PR is now merged/superseded: shapeblue#136 |
1 similar comment
Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>
Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>
1 similar comment
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Description
This PR fixes the login flow for users who do not have permission to access the listLdapConfigurations API. Currently, login is blocked for these users regardless of whether LDAP is configured.
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?