ingox opened a new issue, #14455:
URL: https://github.com/apache/cloudstack/issues/14455
### problem
### COMPONENT NAME
UI, SAML2 plugin
### CLOUDSTACK VERSION
4.22.1.1 (the code paths below are verified in tag `4.22.1.1`)
### CONFIGURATION
- KVM, Advanced zone
- SAML2 plugin enabled, IdP: Keycloak (SP-initiated, HTTP-POST binding)
- User authorized for SAML SSO via `authorizeSamlSso`; role type
**DomainAdmin** (same username present in two domains)
### SUMMARY
After a successful SAML2 SSO login, the UI never finishes loading (blank
page or endless spinner) for any user who is **not a Root Admin**. Native login
for the same user works. All APIs called with the SAML session return HTTP 200
(`listCapabilities`, `listApis`, `listUsers`, `listZones`,
`listAndSwitchSamlAccount`), so the server-side session is valid. The failure
is entirely in the UI.
### versions
### CLOUDSTACK VERSION
4.22.1.1 (the code paths below are verified in tag `4.22.1.1`)ons of ACS,
hypervisors, storage, network etc..
### The steps to reproduce the bug
### STEPS TO REPRODUCE
1. Enable SAML2 (`saml2.enabled=true`) and configure an IdP.
2. Authorize a **Domain Admin** or **User** for SAML SSO.
3. Log in through the IdP.
4. After the redirect back to `/client`, the UI stays blank.
### EXPECTED RESULTS
The UI loads the dashboard.
### ACTUAL RESULTS
Browser console:
```
SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON
data
get webStorage.js:20
GetInfo user.js:319
... permission.js:104
Uncaught (in promise) TypeError: can't convert undefined to object
GenerateRoutes permission.js:74
```
`localStorage.getItem('primate__MS_ID')` returns the literal string
`"undefined"`, and no `managementserverid` cookie is set after `samlSso`.
### What to do about it?
### ROOT CAUSE
1. **Server:** `ApiServer.loginUser()` only puts `managementserverid` into
the session for **root admins**:
```java
if (accountMgr.isRootAdmin(userAcct.getAccountId())) {
... session.setAttribute(ApiConstants.MANAGEMENT_SERVER_ID,
msHost.getUuid());
}
```
For any other user, `SAMLUtils.addBaseCookies()` therefore sets no
`managementserverid` cookie. That part is correct and intended.
2. **UI, SAML branch** (`ui/src/permission.js`): the UI commits the cookie
value without checking it:
```js
store.commit('SET_MS_ID', Cookies.get('managementserverid')) //
undefined for non-root users
```
The native login path in `user.js` does guard against this (`if (result
&& result.managementserverid)`).
3. **UI store** (`SET_MS_ID` in `ui/src/store/modules/user.js`) persists the
value as is: `$localStorage.set(MS_ID, undefined)`. This leaves `primate__MS_ID
= "undefined"` in localStorage.
4. On the next `GetInfo`, `$localStorage.get(MS_ID, false)` calls
`JSON.parse("undefined")`, which throws. `GetInfo` rejects, `GenerateRoutes`
receives `undefined`, and no routes are generated. The UI stays blank.
Clearing localStorage does not help, because the `permission.js` SAML branch
writes the bad value again on every page load.
### WORKAROUND
Once the page is blank, run this in the browser console (any non-empty value
works, because only root admins use `msId`):
```js
document.cookie = 'managementserverid=<ms-uuid>; path=/client';
localStorage.setItem('primate__MS_ID', JSON.stringify({ value: '<ms-uuid>',
expire: null }));
location.reload();
```
### PROPOSED FIX
A patch is attached (`saml-msid-fix.patch`):
1. **`ui/src/permission.js`:** commit the management server ID only when the
cookie exists. Also check `path: '/client'`, consistent with the other SAML
cookies.
```js
const msId = Cookies.get('managementserverid') ||
Cookies.get('managementserverid', { path: '/client' })
if (msId) {
store.commit('SET_MS_ID', msId)
}
```
2. **`ui/src/store/modules/user.js`:** make `SET_MS_ID` safe so it never
persists `undefined`, which also protects any other caller:
```js
SET_MS_ID: (state, msId) => {
state.msId = msId || ''
if (msId) {
vueProps.$localStorage.set(MS_ID, msId)
} else {
vueProps.$localStorage.remove(MS_ID)
}
},
```
3. **`SAMLUtils.addBaseCookies()` (consistency):** create the
`managementserverid` cookie with the same domain and path as the other SAML
cookies (`newCookie(domain, path, ...)` instead of `new Cookie(...)`). Then
root admins get the cookie on the redirect target path even when
`saml2.redirect.url` has a different path from the ACS endpoint.
Fixes 1 and 2 on their own resolve the bug. Fix 3 is a hardening change.
### TESTING
- SAML login as Root Admin: `managementserverid` cookie set,
`primate__MS_ID` holds the UUID, shutdown and maintenance banner checks work.
- SAML login as Domain Admin or User: no cookie, no `primate__MS_ID` entry,
UI loads.
- Native login and logout: unchanged (logout calls `SET_MS_ID('')`, which
now removes the key).
- Users whose browser already holds a stale `primate__MS_ID = "undefined"`:
optionally make `GetInfo` tolerate unparsable values. With fix 2 in place, the
next SAML login removes the key, but `GetInfo` runs before that, so a one-off
cleanup in `permission.js`, or a `try/catch` around the storage `get`, would
make the upgrade path seamless.
--
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]
