stag7824 opened a new pull request, #14152:
URL: https://github.com/apache/cloudstack/pull/14152
### Description
Fixes #13912
A role that denies `listLdapConfigurations` cannot use the web console at
all: the UI never
renders and the user is redirected back to `/user/login`.
`GetInfo` in `ui/src/store/modules/user.js` runs about a dozen independent
bootstrap calls
inside a single `new Promise`, and several of them are wired to that one
shared `reject`.
`listLdapConfigurations` is one of them, and all it does with the answer is
record a flag:
```js
getAPI('listLdapConfigurations').then(response => {
const ldapEnable = (response.ldapconfigurationresponse.count > 0)
commit('SET_LDAP', ldapEnable)
}).catch(error => {
reject(error)
})
```
When the role denies it, the 432 rejects the shared promise. That is a race
against
`listApis` resolving it, and the tiny denied query almost always settles
first, so `GetInfo`
rejects. The router guard in `ui/src/permission.js` treats a rejected
`GetInfo` as a failed
session and logs the user out instead of building routes — so one denied
flag read takes down
the whole console.
The tolerant pattern is already in the same function, a few lines away.
`listNetworkServiceProviders` and `cloudianIsEnabled` both swallow their own
error and the
console loads fine without them. This change gives `listLdapConfigurations`
the same
treatment, defaulting the flag rather than rejecting:
```js
}).catch(ignored => {
// A role is allowed to deny this read. It only records whether LDAP is
configured, so
// treat it as not configured rather than rejecting the promise the whole
console waits on.
commit('SET_LDAP', false)
})
```
Defaulting to `false` matches what the flag already means elsewhere —
`Logout` resets it, and
a denied read is not evidence that LDAP is configured.
#### Scope
I have deliberately changed only the call named in the issue.
`listCapabilities`, `listZones`
and the second `listUsers` are wired to the same shared `reject` and carry
the same hazard,
but unlike this one they are not pure flag reads, so whether each is
"essential" is a call for
you rather than for me. Happy to widen this PR if you'd like them handled
together, or to
leave that to a follow-up.
### Types of changes
- [x] Bug fix (non-breaking change which fixes an issue)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [x] Major
### How Has This Been Tested?
I could not add a unit test for this proportionately: `user.js` imports
`@/router`, `@/store`
and `@/vue-app`, there are no existing tests for any store module to build
on, and standing
that scaffolding up for a three-line change seemed like more maintenance
burden than it is
worth. Happy to add one if you disagree.
What I did instead was reduce `GetInfo`'s promise structure to a runnable
model — a late
`listApis` that resolves, a tolerant denied call, and the denied
`listLdapConfigurations` —
and run it both ways:
```
without the fix -> LOGGED OUT (432 denied by role)
with the fix -> console loads (apis=1, ldap=false)
```
which reproduces the reported behaviour and the race described in the issue:
the small denied
query settles before `listApis` can resolve the shared promise.
I also ran what CI runs, on Node 16 to match `.github/workflows/ui.yml`:
```
npm run lint DONE No lint errors found!
npm run test:unit Test Suites: 9 passed, 9 total
Tests: 191 passed, 191 total
```
`vue-cli-service lint` autofixes by default, so I checked separately that it
left the file
untouched rather than quietly reformatting the change into shape.
I do not have a deployment with custom roles to confirm the console
rendering end to end, so
that part is unverified by me. The change itself is the same shape as the
two tolerant calls
either side of it.
The same construction is present on 4.20 (it uses `api(` rather than
`getAPI(` there). I have
targeted 4.22 since that is what the issue names, but happy to retarget.
--
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]
