Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ public void getInstances(FullHttpRequest request, HttpResponder responder,

ApplicationSpecification spec = appSpecs.get(appId);
ProgramId programId = appId.program(runnable.getProgramType(), runnable.getProgramId());
accessEnforcer.enforce(programId, authenticationContext.getPrincipal(), StandardPermission.GET);
output.add(getProgramInstances(runnable, spec, programId));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The /instances endpoint is a batch API designed to process multiple runnables and return individual status codes (e.g., 200, 400, 404) for each item in the batch. By calling accessEnforcer.enforce(...) directly inside the loop without catching SecurityException (or UnauthorizedException), any single unauthorized program will throw an exception that propagates out of the method, failing the entire batch request with a 401/403.

To preserve the batch semantics, wrap the enforcement check in a try-catch block, catch SecurityException, and add a BatchRunnableInstances with HttpResponseStatus.UNAUTHORIZED.code() to the output list so that authorized programs can still be successfully processed and returned.

      try {
        accessEnforcer.enforce(programId, authenticationContext.getPrincipal(), StandardPermission.GET);
        output.add(getProgramInstances(runnable, spec, programId));
      } catch (SecurityException e) {
        output.add(new BatchRunnableInstances(runnable, HttpResponseStatus.UNAUTHORIZED.code(), e.getMessage()));
      }

}
responder.sendJson(HttpResponseStatus.OK, ProgramHandlerUtil.toJson(output));
Expand Down Expand Up @@ -305,6 +306,7 @@ public void liveInfo(HttpRequest request, HttpResponder responder,
ProgramType type = ProgramType.valueOfCategoryName(programCategory, BadRequestException::new);
ProgramId program = store.getLatestApp(new ApplicationReference(namespaceId, appId))
.program(type, programId);
accessEnforcer.enforce(program, authenticationContext.getPrincipal(), StandardPermission.GET);
getLiveInfo(responder, program, runtimeService);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* Copyright © 2026 Cask Data, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package io.cdap.cdap.gateway.handlers;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import io.cdap.cdap.app.runtime.ProgramRuntimeService;
import io.cdap.cdap.app.store.Store;
import io.cdap.cdap.common.ApplicationNotFoundException;
import io.cdap.cdap.common.BadRequestException;
import io.cdap.cdap.common.namespace.NamespaceQueryAdmin;
import io.cdap.cdap.internal.app.services.ProgramLifecycleService;
import io.cdap.cdap.proto.NotRunningProgramLiveInfo;
import io.cdap.cdap.proto.ProgramType;
import io.cdap.cdap.proto.id.ApplicationId;
import io.cdap.cdap.proto.id.ApplicationReference;
import io.cdap.cdap.proto.id.NamespaceId;
import io.cdap.cdap.proto.id.ProgramId;
import io.cdap.cdap.proto.security.Authorizable;
import io.cdap.cdap.proto.security.Principal;
import io.cdap.cdap.proto.security.StandardPermission;
import io.cdap.cdap.security.auth.context.AuthenticationTestContext;
import io.cdap.cdap.security.authorization.InMemoryAccessController;
import io.cdap.cdap.security.spi.authentication.AuthenticationContext;
import io.cdap.cdap.security.spi.authorization.UnauthorizedException;
import io.cdap.http.HttpResponder;
import io.netty.handler.codec.http.HttpRequest;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.Matchers;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

org.mockito.Matchers is deprecated in Mockito 2.x and removed in newer versions. It should be replaced with org.mockito.ArgumentMatchers to avoid using deprecated APIs and ensure compatibility with future Mockito upgrades.

Suggested change
import org.mockito.Matchers;
import org.mockito.ArgumentMatchers;


public class ProgramRuntimeHttpHandlerAuthorizationTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test class only covers the /live-info endpoint (liveInfo). Since this pull request also adds access enforcement to the batch /instances endpoint (getInstances), please add corresponding unit tests to verify both authorized and unauthorized access scenarios for the batch endpoint.

private static final Principal MASTER_PRINCIPAL = new Principal("master", Principal.PrincipalType.USER);
private static final Principal UNPRIVILEGED_PRINCIPAL = new Principal("unprivileged",
Principal.PrincipalType.USER);
private static final NamespaceId NAMESPACE_ID = new NamespaceId("ns");
private static final ApplicationId APP_ID = NAMESPACE_ID.app("app");
private static final ProgramId PROGRAM_ID = APP_ID.service("service");

private static ProgramRuntimeHttpHandler programRuntimeHttpHandler;
private static ProgramRuntimeService runtimeService;

HttpRequest request;
HttpResponder responder;
Exception exceptionThrown;

@BeforeClass
public static void setup() throws ApplicationNotFoundException {
StandardPermission[] requiredPermissions = new StandardPermission[] {StandardPermission.GET};

InMemoryAccessController inMemoryAccessController = new InMemoryAccessController();
inMemoryAccessController.grant(Authorizable.fromEntityId(PROGRAM_ID), MASTER_PRINCIPAL,
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(requiredPermissions))));
AuthenticationContext authenticationContext = new AuthenticationTestContext();

ProgramLifecycleService lifecycleService = mock(ProgramLifecycleService.class);
Store store = mock(Store.class);
runtimeService = mock(ProgramRuntimeService.class);
NamespaceQueryAdmin namespaceQueryAdmin = mock(NamespaceQueryAdmin.class);

when(store.getLatestApp(Matchers.any(ApplicationReference.class))).thenReturn(APP_ID);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Replace the deprecated Matchers.any with ArgumentMatchers.any.

Suggested change
when(store.getLatestApp(Matchers.any(ApplicationReference.class))).thenReturn(APP_ID);
when(store.getLatestApp(ArgumentMatchers.any(ApplicationReference.class))).thenReturn(APP_ID);

when(runtimeService.getLiveInfo(PROGRAM_ID)).thenReturn(new NotRunningProgramLiveInfo(PROGRAM_ID));

programRuntimeHttpHandler = new ProgramRuntimeHttpHandler(lifecycleService, store, runtimeService,
namespaceQueryAdmin, inMemoryAccessController,
authenticationContext);
}

@Before
public void initializeVariables() {
request = mock(HttpRequest.class);
responder = mock(HttpResponder.class);
exceptionThrown = null;
reset(runtimeService);
when(runtimeService.getLiveInfo(PROGRAM_ID)).thenReturn(new NotRunningProgramLiveInfo(PROGRAM_ID));
}

@Test
public void testLiveInfoUnauthorized() throws BadRequestException, ApplicationNotFoundException {
AuthenticationTestContext.actAsPrincipal(UNPRIVILEGED_PRINCIPAL);
try {
programRuntimeHttpHandler.liveInfo(request, responder, NAMESPACE_ID.getNamespace(), APP_ID.getApplication(),
ProgramType.SERVICE.getCategoryName(), PROGRAM_ID.getProgram());
} catch (UnauthorizedException e) {
exceptionThrown = e;
}
Assert.assertNotNull(exceptionThrown);
verify(runtimeService, never()).getLiveInfo(Matchers.any(ProgramId.class));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Replace the deprecated Matchers.any with ArgumentMatchers.any.

Suggested change
verify(runtimeService, never()).getLiveInfo(Matchers.any(ProgramId.class));
verify(runtimeService, never()).getLiveInfo(ArgumentMatchers.any(ProgramId.class));

}

@Test
public void testLiveInfoAuthorized() throws BadRequestException, ApplicationNotFoundException {
AuthenticationTestContext.actAsPrincipal(MASTER_PRINCIPAL);
try {
programRuntimeHttpHandler.liveInfo(request, responder, NAMESPACE_ID.getNamespace(), APP_ID.getApplication(),
ProgramType.SERVICE.getCategoryName(), PROGRAM_ID.getProgram());
} catch (UnauthorizedException e) {
exceptionThrown = e;
}
Assert.assertNull(exceptionThrown);
verify(runtimeService).getLiveInfo(PROGRAM_ID);
}
}