stag7824 commented on issue #14014:
URL: https://github.com/apache/cloudstack/issues/14014#issuecomment-5645690378
I went looking for the root cause of this one. It is a regression from the
4.22.1.1 security release, specifically `b550e4c` ("Restrict domain admin
access to project roles within their own domain").
That commit added `checkAccess(projectId)` to the two **lookup** methods in
`ProjectRoleManagerImpl`, alongside the mutating ones it already guarded —
`findProjectRole` (4.22:167), `findProjectRoles` (4.22:187) and
`findProjectRolesByName` (4.22:255).
But `checkAccess` is a project-**admin** guard. It ends at 4.22:118 with:
```java
if (ProjectAccount.Role.Admin != projectAccount.getAccountRole()) {
throw new PermissionDeniedException("User unauthorized to perform
operation in the project");
}
```
and `findProjectRole` is on the API authorization hot path.
`ProjectRoleBasedApiAccessChecker.isPermitted` (4.22:163-166), and the
identical block in `DomainChecker.isPermitted`, resolve the **caller's own**
project role on every API call made in project context:
```java
if (projectUser.getProjectRoleId() != null) {
projectRole =
projectRoleService.findProjectRole(projectUser.getProjectRoleId(),
project.getId());
}
```
So for a project member who is not a project Admin and who has a custom
project role, every API call in project context now throws
`PermissionDeniedException` while merely looking up that member's own role —
before any permission rule is ever evaluated. That is why `allow *` makes no
difference: the rules are never reached.
Two details from the report line up with this exactly:
- **"If user is added with Admin or Regular Type without any project role,
switching to project view works correctly."** With no project role,
`getProjectRoleId()` is null, `findProjectRole` is never called, and nothing
throws. The failure needs a custom project role, which is precisely the `!=
null` branch.
- **"No additional logs available in management-server.log."**
`ApiServer.checkCommandAvailable` catches `PermissionDeniedException` and
rewrites it without logging:
```java
} catch (final PermissionDeniedException ex) {
final String errorMessage = "The given command '" + commandName + "'
either does not exist, is not available" +
" for user.";
throw new ServerApiException(ApiErrorCode.UNAUTHORIZED ,
errorMessage);
}
```
which is where both the wording and the `errorcode: 401` in the browser
response come from. The message is misleading here — the command exists and is
permitted; the lookup of the caller's role is what failed.
### Suggested fix
The hardening in `b550e4c` is about who may **manage or list** project roles
through the API, which is a concern of the command layer. The internal
authorization lookup is a different caller with a different requirement: a
member must always be able to resolve their own role.
The smallest change that keeps the domain-admin restriction intact would be
to split the guard — keep the current admin check for the mutating methods, and
give the read paths a variant that still confines a domain admin to their own
domain hierarchy but permits any member of the project. Alternatively, move the
check up into `ListProjectRolesCmd` / `ListProjectRolePermissionsCmd` and leave
the service lookups uncheck, though that needs care to cover every API entry
point.
I am happy to put up a PR, but since this is walking back part of a security
fix I would rather confirm the intended semantics first — @Pearl1594, which
shape would you prefer? Should a non-admin project member be able to list the
project's roles, or only to have their own resolved?
--
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]
