Skip to content

Commit 470211b

Browse files
fix invalid access validation performed to the domainId attribute
1 parent dae0395 commit 470211b

File tree

3 files changed

+55
-4
lines changed

3 files changed

+55
-4
lines changed

api/src/main/java/org/apache/cloudstack/api/command/user/consoleproxy/ListConsoleSessionsCmd.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ public class ListConsoleSessionsCmd extends BaseListCmd {
5353
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = ConsoleSessionResponse.class, description = "The ID of the console session.")
5454
private Long id;
5555

56-
@ACL
5756
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, entityType = DomainResponse.class, description = "The domain ID of the account that created the console endpoint.")
5857
private Long domainId;
5958

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public ListResponse<ConsoleSessionResponse> listConsoleSessions(ListConsoleSessi
215215

216216
protected Pair<List<ConsoleSessionVO>, Integer> listConsoleSessionsInternal(ListConsoleSessionsCmd cmd) {
217217
CallContext caller = CallContext.current();
218-
long domainId = cmd.getDomainId() != null ? cmd.getDomainId() : caller.getCallingAccount().getDomainId();
218+
long domainId = getBaseDomainIdToListConsoleSessions(cmd.getDomainId());
219219
Long accountId = cmd.getAccountId();
220220
Long userId = cmd.getUserId();
221221
boolean isRecursive = cmd.isRecursive();
@@ -239,6 +239,26 @@ protected Pair<List<ConsoleSessionVO>, Integer> listConsoleSessionsInternal(List
239239
cmd.getPageSizeVal(), cmd.getStartIndex());
240240
}
241241

242+
/**
243+
* Determines the base domain ID for listing console sessions.
244+
*
245+
* If no domain ID is provided, returns the caller's domain ID. Otherwise,
246+
* checks if the caller has access to that domain and returns the provided domain ID.
247+
*
248+
* @param domainId The domain ID to check, can be null
249+
* @return The base domain ID to use for listing console sessions
250+
* @throws PermissionDeniedException if the caller does not have access to the specified domain
251+
*/
252+
protected long getBaseDomainIdToListConsoleSessions(Long domainId) {
253+
Account caller = CallContext.current().getCallingAccount();
254+
if (domainId == null) {
255+
return caller.getDomainId();
256+
}
257+
258+
accountManager.checkAccess(caller, domainDao.findById(domainId));
259+
return domainId;
260+
}
261+
242262
@Override
243263
public ConsoleSession listConsoleSessionById(long id) {
244264
return consoleSessionDao.findById(id);

server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.cloudstack.consoleproxy;
1818

1919
import com.cloud.agent.AgentManager;
20+
import com.cloud.domain.DomainVO;
2021
import com.cloud.domain.dao.DomainDao;
2122
import com.cloud.exception.PermissionDeniedException;
2223
import com.cloud.server.ManagementServer;
@@ -81,6 +82,8 @@ public class ConsoleAccessManagerImplTest {
8182
@Mock
8283
private DomainDao domainDaoMock;
8384
@Mock
85+
private DomainVO domainMock;
86+
@Mock
8487
private ConsoleSessionVO consoleSessionMock;
8588
@Mock
8689
private ConsoleSessionDao consoleSessionDaoMock;
@@ -242,7 +245,9 @@ public void listConsoleSessionsInternalTestShouldFetchConsoleSessionsRecursively
242245

243246
@Test
244247
public void listConsoleSessionsTestShouldCreateResponsesWithFullViewForRootAdmins() {
245-
Mockito.when(consoleAccessManager.listConsoleSessionsInternal(listConsoleSessionsCmdMock)).thenReturn(new Pair<>(List.of(consoleSessionMock), 1));
248+
Mockito.doReturn(new Pair<>(List.of(consoleSessionMock), 1))
249+
.when(consoleAccessManager)
250+
.listConsoleSessionsInternal(listConsoleSessionsCmdMock);
246251

247252
try (MockedStatic<CallContext> callContextStaticMock = Mockito.mockStatic(CallContext.class)) {
248253
callContextStaticMock.when(CallContext::current).thenReturn(callContextMock);
@@ -257,7 +262,9 @@ public void listConsoleSessionsTestShouldCreateResponsesWithFullViewForRootAdmin
257262

258263
@Test
259264
public void listConsoleSessionsTestShouldCreateResponsesWithRestrictedViewForNonRootAdmins() {
260-
Mockito.when(consoleAccessManager.listConsoleSessionsInternal(listConsoleSessionsCmdMock)).thenReturn(new Pair<>(List.of(consoleSessionMock), 1));
265+
Mockito.doReturn(new Pair<>(List.of(consoleSessionMock), 1))
266+
.when(consoleAccessManager)
267+
.listConsoleSessionsInternal(listConsoleSessionsCmdMock);
261268

262269
try (MockedStatic<CallContext> callContextStaticMock = Mockito.mockStatic(CallContext.class)) {
263270
callContextStaticMock.when(CallContext::current).thenReturn(callContextMock);
@@ -271,6 +278,31 @@ public void listConsoleSessionsTestShouldCreateResponsesWithRestrictedViewForNon
271278
Mockito.verify(responseGeneratorMock).createConsoleSessionResponse(consoleSessionMock, ResponseObject.ResponseView.Restricted);
272279
}
273280

281+
@Test
282+
public void getBaseDomainIdToListConsoleSessionsTestIfNoDomainIdIsProvidedReturnCallersDomainId() {
283+
long callerDomainId = 5L;
284+
285+
try (MockedStatic<CallContext> callContextStaticMock = Mockito.mockStatic(CallContext.class)) {
286+
callContextStaticMock.when(CallContext::current).thenReturn(callContextMock);
287+
Mockito.when(callContextMock.getCallingAccount()).thenReturn(account);
288+
Mockito.when(account.getDomainId()).thenReturn(callerDomainId);
289+
Assert.assertEquals(callerDomainId, consoleAccessManager.getBaseDomainIdToListConsoleSessions(null));
290+
}
291+
}
292+
293+
@Test
294+
public void getBaseDomainIdToListConsoleSessionsTestPerformAccessValidationWhenDomainIsProvided() {
295+
long domainId = 5L;
296+
297+
try (MockedStatic<CallContext> callContextStaticMock = Mockito.mockStatic(CallContext.class)) {
298+
callContextStaticMock.when(CallContext::current).thenReturn(callContextMock);
299+
Mockito.when(callContextMock.getCallingAccount()).thenReturn(account);
300+
Mockito.when(domainDaoMock.findById(domainId)).thenReturn(domainMock);
301+
Assert.assertEquals(domainId, consoleAccessManager.getBaseDomainIdToListConsoleSessions(domainId));
302+
Mockito.verify(accountManager).checkAccess(account, domainMock);
303+
}
304+
}
305+
274306
@Test
275307
public void listConsoleSessionByIdTestShouldCallDbLayer() {
276308
consoleAccessManager.listConsoleSessionById(1L);

0 commit comments

Comments
 (0)