Skip to content

restapi: Restrict access to domain user API for non-admin users - #1019

Open
Artemi10 wants to merge 1 commit into
oVirt:masterfrom
Artemi10:feature/restrict-domain-user-api
Open

restapi: Restrict access to domain user API for non-admin users#1019
Artemi10 wants to merge 1 commit into
oVirt:masterfrom
Artemi10:feature/restrict-domain-user-api

Conversation

@Artemi10

Copy link
Copy Markdown

Non-admin users can access the domain user API endpoints (GET /api/domains/{domainId}/users and GET /api/domains/{domainId}/users/{userId}), which is a security issue. This commit adds permission checks to these endpoints. If a user without admin permissions attempts to access them, a 403 Forbidden response will be returned.

  • Before fix
    before_list
    before_get

  • After fix
    after_list
    after_get

Are you the owner of the code you are sending in, or do you have permission of the owner?

Yes

Users without admin permissions can access the list and get endpoints of the domain user API. This fix ensures that such users now receive a 403 Forbidden response.

Signed-off-by: Liakh Artemii <aliakh@orionsoft.ru>

@dupondje dupondje left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see how it's implemented in backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java
There you also have isFiltered(), so implement with the same logic.

@Artemi10

Artemi10 commented Jun 2, 2025

Copy link
Copy Markdown
Author

Please see how it's implemented in backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/aaa/BackendUsersResource.java There you also have isFiltered(), so implement with the same logic.

@dupondje
Thanks for pointing that out!

I looked into the isFiltered() implementation in BackendUsersResource, but I noticed a potential issue: it respects the filter query parameter even for non-admin users. This means a non-admin could explicitly set filter=false in the request header and potentially bypass data filtering, which might expose sensitive information.

To avoid this, implemented a separate method isAdmin(). This way, I only check user permissions and do not take any request headers into account, ensuring stricter access control.

protected boolean isAdmin() {
DbUser user = getCurrent().getUser();
return user.isAdmin();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants