diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternal.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternal.java index ea967b1d2bfa..e905edb1a85f 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternal.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternal.java @@ -27,8 +27,12 @@ import io.cdap.cdap.proto.PreferencesDetail; import io.cdap.cdap.proto.ProgramType; import io.cdap.cdap.proto.id.ApplicationId; +import io.cdap.cdap.proto.id.InstanceId; import io.cdap.cdap.proto.id.NamespaceId; import io.cdap.cdap.proto.id.ProgramId; +import io.cdap.cdap.proto.security.StandardPermission; +import io.cdap.cdap.security.spi.authentication.AuthenticationContext; +import io.cdap.cdap.security.spi.authorization.AccessEnforcer; import io.cdap.http.HttpResponder; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponseStatus; @@ -48,14 +52,20 @@ public class PreferencesHttpHandlerInternal extends AbstractAppFabricHttpHandler private final PreferencesService preferencesService; private final ApplicationLifecycleService applicationLifecycleService; private final NamespaceQueryAdmin namespaceQueryAdmin; + private final AccessEnforcer accessEnforcer; + private final AuthenticationContext authenticationContext; @Inject PreferencesHttpHandlerInternal(PreferencesService preferencesService, ApplicationLifecycleService applicationLifecycleService, - NamespaceQueryAdmin namespaceQueryAdmin) { + NamespaceQueryAdmin namespaceQueryAdmin, + AccessEnforcer accessEnforcer, + AuthenticationContext authenticationContext) { this.preferencesService = preferencesService; this.applicationLifecycleService = applicationLifecycleService; this.namespaceQueryAdmin = namespaceQueryAdmin; + this.accessEnforcer = accessEnforcer; + this.authenticationContext = authenticationContext; } /** @@ -66,7 +76,9 @@ public class PreferencesHttpHandlerInternal extends AbstractAppFabricHttpHandler */ @Path("/preferences") @GET - public void getInstancePreferences(HttpRequest request, HttpResponder responder) { + public void getInstancePreferences(HttpRequest request, HttpResponder responder) throws Exception { + accessEnforcer.enforce(new InstanceId(""), authenticationContext.getPrincipal(), + StandardPermission.GET); PreferencesDetail detail = preferencesService.getPreferences(); responder.sendJson(HttpResponseStatus.OK, GSON.toJson(detail, PreferencesDetail.class)); } @@ -88,8 +100,10 @@ public void getInstancePreferences(HttpRequest request, HttpResponder responder) @GET public void getNamespacePreferences(HttpRequest request, HttpResponder responder, @PathParam("namespace-id") String namespace, - @QueryParam("resolved") boolean resolved) { + @QueryParam("resolved") boolean resolved) throws Exception { NamespaceId namespaceId = new NamespaceId(namespace); + accessEnforcer.enforce(namespaceId, authenticationContext.getPrincipal(), + StandardPermission.GET); // No need to check if namespace exists. PreferencesService returns an empty PreferencesDetail when that happens. PreferencesDetail detail; if (resolved) { @@ -119,8 +133,10 @@ public void getNamespacePreferences(HttpRequest request, HttpResponder responder public void getApplicationPreferences(HttpRequest request, HttpResponder responder, @PathParam("namespace-id") String namespace, @PathParam("application-id") String appId, - @QueryParam("resolved") boolean resolved) { + @QueryParam("resolved") boolean resolved) throws Exception { ApplicationId applicationId = new ApplicationId(namespace, appId); + accessEnforcer.enforce(applicationId, authenticationContext.getPrincipal(), + StandardPermission.GET); // No need to check if application exists. PreferencesService returns an empty PreferencesDetail when that happens. PreferencesDetail detail; if (resolved) { @@ -156,6 +172,7 @@ public void getProgramPreferences(HttpRequest request, HttpResponder responder, @PathParam("program-id") String programId, @QueryParam("resolved") boolean resolved) throws Exception { ProgramId program = new ProgramId(namespace, appId, getProgramType(programType), programId); + accessEnforcer.enforce(program, authenticationContext.getPrincipal(), StandardPermission.GET); // No need to check if program exists. PreferencesService returns an empty PreferencesDetail when that happens. PreferencesDetail detail; if (resolved) { diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternalAuthorizationTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternalAuthorizationTest.java new file mode 100644 index 000000000000..2abdd01c6946 --- /dev/null +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/gateway/handlers/PreferencesHttpHandlerInternalAuthorizationTest.java @@ -0,0 +1,207 @@ +/* + * 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.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.cdap.cdap.common.namespace.NamespaceQueryAdmin; +import io.cdap.cdap.config.PreferencesService; +import io.cdap.cdap.internal.app.services.ApplicationLifecycleService; +import io.cdap.cdap.proto.PreferencesDetail; +import io.cdap.cdap.proto.ProgramType; +import io.cdap.cdap.proto.id.ApplicationId; +import io.cdap.cdap.proto.id.InstanceId; +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; + +/** + * FileFetcherHttpHandlerInternal's sibling gap: PreferencesHttpHandlerInternal exposed + * instance/namespace/application preferences with no authorization check at all, + * unlike PreferencesHttpHandler (the public equivalent), which calls + * accessEnforcer.enforce(..., StandardPermission.GET) at every one of those same scopes. + * Mirrors ConfigHandlerAuthorizationTest. InMemoryAccessController implements + * AccessController, which itself extends AccessEnforcer, so it can be passed directly + * as the handler's AccessEnforcer dependency (this handler, unlike ConfigHandler, takes + * the plain AccessEnforcer interface rather than ContextAccessEnforcer). + */ +public class PreferencesHttpHandlerInternalAuthorizationTest { + 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 OTHER_NAMESPACE = new NamespaceId("other-tenant-namespace"); + private static final ApplicationId OTHER_APP = new ApplicationId("other-tenant-namespace", "some-app"); + private static final ProgramId OTHER_PROGRAM = new ProgramId("other-tenant-namespace", "some-app", + ProgramType.WORKFLOW, "some-program"); + + private static PreferencesHttpHandlerInternal preferencesHandler; + + HttpRequest request; + HttpResponder responder; + Exception exceptionThrown; + + @BeforeClass + public static void setup() { + StandardPermission[] requiredPermissions = new StandardPermission[] {StandardPermission.GET}; + + InMemoryAccessController inMemoryAccessController = new InMemoryAccessController(); + inMemoryAccessController.grant(Authorizable.fromEntityId(InstanceId.SELF), MASTER_PRINCIPAL, + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(requiredPermissions)))); + inMemoryAccessController.grant(Authorizable.fromEntityId(OTHER_NAMESPACE), MASTER_PRINCIPAL, + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(requiredPermissions)))); + inMemoryAccessController.grant(Authorizable.fromEntityId(OTHER_APP), MASTER_PRINCIPAL, + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(requiredPermissions)))); + inMemoryAccessController.grant(Authorizable.fromEntityId(OTHER_PROGRAM), MASTER_PRINCIPAL, + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(requiredPermissions)))); + AuthenticationContext authenticationContext = new AuthenticationTestContext(); + + PreferencesService mockPreferencesService = mock(PreferencesService.class); + when(mockPreferencesService.getPreferences()) + .thenReturn(new PreferencesDetail(Collections.emptyMap(), 0, false)); + when(mockPreferencesService.getPreferences(any(NamespaceId.class))) + .thenReturn(new PreferencesDetail(Collections.emptyMap(), 0, false)); + when(mockPreferencesService.getPreferences(any(ApplicationId.class))) + .thenReturn(new PreferencesDetail(Collections.emptyMap(), 0, false)); + when(mockPreferencesService.getPreferences(any(ProgramId.class))) + .thenReturn(new PreferencesDetail(Collections.emptyMap(), 0, false)); + ApplicationLifecycleService mockAppLifecycleService = mock(ApplicationLifecycleService.class); + NamespaceQueryAdmin mockNamespaceQueryAdmin = mock(NamespaceQueryAdmin.class); + + preferencesHandler = new PreferencesHttpHandlerInternal(mockPreferencesService, mockAppLifecycleService, + mockNamespaceQueryAdmin, inMemoryAccessController, authenticationContext); + } + + @Before + public void initializeVariables() { + request = mock(HttpRequest.class); + responder = mock(HttpResponder.class); + exceptionThrown = null; + } + + @Test + public void testGetInstancePreferencesUnauthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(UNPRIVILEGED_PRINCIPAL); + try { + preferencesHandler.getInstancePreferences(request, responder); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNotNull("an unprivileged caller must not be able to read instance-level " + + "preferences via the internal endpoint", exceptionThrown); + } + + @Test + public void testGetInstancePreferencesAuthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(MASTER_PRINCIPAL); + try { + preferencesHandler.getInstancePreferences(request, responder); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNull("a caller with instance-level access must not be rejected", exceptionThrown); + } + + @Test + public void testGetNamespacePreferencesUnauthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(UNPRIVILEGED_PRINCIPAL); + try { + preferencesHandler.getNamespacePreferences(request, responder, OTHER_NAMESPACE.getNamespace(), false); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNotNull("an unprivileged caller must not be able to read another " + + "namespace's preferences via the internal endpoint", exceptionThrown); + } + + @Test + public void testGetNamespacePreferencesAuthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(MASTER_PRINCIPAL); + try { + preferencesHandler.getNamespacePreferences(request, responder, OTHER_NAMESPACE.getNamespace(), false); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNull("a caller with access to the namespace must not be rejected", exceptionThrown); + } + + @Test + public void testGetApplicationPreferencesUnauthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(UNPRIVILEGED_PRINCIPAL); + try { + preferencesHandler.getApplicationPreferences(request, responder, OTHER_NAMESPACE.getNamespace(), + "some-app", false); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNotNull("an unprivileged caller must not be able to read another " + + "namespace's application preferences via the internal endpoint", exceptionThrown); + } + + @Test + public void testGetApplicationPreferencesAuthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(MASTER_PRINCIPAL); + try { + preferencesHandler.getApplicationPreferences(request, responder, OTHER_NAMESPACE.getNamespace(), + "some-app", false); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNull("a caller with access to the application must not be rejected", exceptionThrown); + } + + @Test + public void testGetProgramPreferencesUnauthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(UNPRIVILEGED_PRINCIPAL); + try { + preferencesHandler.getProgramPreferences(request, responder, OTHER_NAMESPACE.getNamespace(), + "some-app", "workflows", "some-program", false); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNotNull("an unprivileged caller must not be able to read another " + + "namespace's program preferences via the internal endpoint", exceptionThrown); + } + + @Test + public void testGetProgramPreferencesAuthorized() throws Exception { + AuthenticationTestContext.actAsPrincipal(MASTER_PRINCIPAL); + try { + preferencesHandler.getProgramPreferences(request, responder, OTHER_NAMESPACE.getNamespace(), + "some-app", "workflows", "some-program", false); + } catch (UnauthorizedException e) { + exceptionThrown = e; + } + Assert.assertNull("a caller with access to the program must not be rejected", exceptionThrown); + } +}